From 0483c5c8435175e6958e57d49048143b4219b175 Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Wed, 11 Mar 2026 14:33:27 +0000 Subject: [PATCH 1/3] fix: remove old FlowResults class The FlowResults class was renamed SimFlowResults a while back to differentiate it from the results from the other flows. However it looks like the old version remained due to commit rebase issues. Signed-off-by: James McCorrie --- src/dvsim/sim/data.py | 50 +++---------------------------------------- 1 file changed, 3 insertions(+), 47 deletions(-) diff --git a/src/dvsim/sim/data.py b/src/dvsim/sim/data.py index 508f91c4..d607fe52 100644 --- a/src/dvsim/sim/data.py +++ b/src/dvsim/sim/data.py @@ -169,47 +169,6 @@ def flattened(self) -> dict[str, float | None]: return items -class FlowResults(BaseModel): - """Flow results data.""" - - model_config = ConfigDict(frozen=True, extra="forbid") - - block: IPMeta - """IP block metadata.""" - tool: ToolMeta - """Tool used in the simulation run.""" - timestamp: datetime - """Timestamp for when the test ran.""" - - stages: Mapping[str, TestStage] - """Results per test stage.""" - coverage: CoverageMetrics | None - """Coverage metrics.""" - - failed_jobs: BucketedFailures - """Bucketed failed job overview.""" - - passed: int - """Number of tests passed.""" - total: int - """Total number of tests run.""" - percent: float - """Percentage test pass rate.""" - - @staticmethod - def load(path: Path) -> "FlowResults": - """Load results from JSON file. - - Transform the fields of the loaded JSON into a more useful schema for - report generation. - - Args: - path: to the json file to load. - - """ - return FlowResults.model_validate_json(path.read_text()) - - class SimFlowResults(BaseModel): """Flow results data.""" @@ -245,17 +204,14 @@ class SimFlowResults(BaseModel): """Percentage test pass rate.""" @staticmethod - def load(path: Path) -> "FlowResults": + def load(path: Path) -> "SimFlowResults": """Load results from JSON file. - Transform the fields of the loaded JSON into a more useful schema for - report generation. - Args: path: to the json file to load. """ - return FlowResults.model_validate_json(path.read_text()) + return SimFlowResults.model_validate_json(path.read_text()) class SimResultsSummary(BaseModel): @@ -276,7 +232,7 @@ class SimResultsSummary(BaseModel): """Build seed.""" flow_results: Mapping[str, SimFlowResults] - """Flow results.""" + """Flow results summary or full results.""" report_path: Path """Path to the report JSON file.""" From 4903498caa6064291bd3ab49de6da5be345ad968 Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Wed, 11 Mar 2026 14:39:37 +0000 Subject: [PATCH 2/3] refactor: separate out summary from the full results At the moment the `SimResultsSummary` contains both the summary and also the full results. This creates duplicated data within the JSON data files. This commit separates out the summary data from the full detailed results. Which means that the block level reports JSON files will now be the primary source of the data for the detailed results. The summary JSON will now no longer replicate the same data. Signed-off-by: James McCorrie --- src/dvsim/cli/admin.py | 13 +++++++-- src/dvsim/sim/data.py | 62 ++++++++++++++++++++++++++++++++++++++--- src/dvsim/sim/flow.py | 7 ++++- src/dvsim/sim/report.py | 59 ++++++++++++++++++++++++++++++--------- 4 files changed, 120 insertions(+), 21 deletions(-) diff --git a/src/dvsim/cli/admin.py b/src/dvsim/cli/admin.py index cbe5fe54..b0f3a503 100644 --- a/src/dvsim/cli/admin.py +++ b/src/dvsim/cli/admin.py @@ -41,9 +41,16 @@ def gen(json_path: Path, output_dir: Path) -> None: from dvsim.sim.data import SimResultsSummary from dvsim.sim.report import gen_reports - results: SimResultsSummary = SimResultsSummary.load(path=json_path) - - gen_reports(summary=results, path=output_dir) + summary: SimResultsSummary = SimResultsSummary.load(path=json_path) + flow_results = summary.load_flow_results( + base_path=json_path.parent, + ) + + gen_reports( + summary=summary, + flow_results=flow_results, + path=output_dir, + ) if __name__ == "__main__": diff --git a/src/dvsim/sim/data.py b/src/dvsim/sim/data.py index d607fe52..f9427da4 100644 --- a/src/dvsim/sim/data.py +++ b/src/dvsim/sim/data.py @@ -203,6 +203,17 @@ class SimFlowResults(BaseModel): percent: float """Percentage test pass rate.""" + def summary(self) -> "SimFlowSummary": + """Load results from JSON file. + + Args: + path: to the json file to load. + + """ + return SimFlowSummary.model_validate_json( + json_data=self.model_dump_json(), + ) + @staticmethod def load(path: Path) -> "SimFlowResults": """Load results from JSON file. @@ -214,6 +225,35 @@ def load(path: Path) -> "SimFlowResults": return SimFlowResults.model_validate_json(path.read_text()) +class SimFlowSummary(BaseModel): + """Flow results summary.""" + + model_config = ConfigDict(frozen=True, extra="ignore") + + block: IPMeta + """IP block metadata.""" + + coverage: CoverageMetrics | None + """Coverage metrics.""" + + passed: int + """Number of tests passed.""" + total: int + """Total number of tests run.""" + percent: float + """Percentage test pass rate.""" + + @staticmethod + def load(path: Path) -> "SimFlowSummary": + """Load results from JSON file. + + Args: + path: to the json file to load. + + """ + return SimFlowSummary.model_validate_json(path.read_text()) + + class SimResultsSummary(BaseModel): """Summary of results.""" @@ -231,19 +271,33 @@ class SimResultsSummary(BaseModel): build_seed: int | None """Build seed.""" - flow_results: Mapping[str, SimFlowResults] + flow_results: Mapping[str, SimFlowSummary] """Flow results summary or full results.""" report_path: Path """Path to the report JSON file.""" + def load_flow_results(self, base_path: Path) -> Mapping[str, SimFlowResults]: + """Load the detailed results for the sim flows from their JSON files. + + Args: + base_path: path to the directory containing the json files to load. + + Returns: + Mapping of flow name to detailed simulation flow results. + + """ + return { + flow: SimFlowResults.load( + path=base_path / f"{flow}.json", + ) + for flow in self.flow_results + } + @staticmethod def load(path: Path) -> "SimResultsSummary": """Load results from JSON file. - Transform the fields of the loaded JSON into a more useful schema for - report generation. - Args: path: to the json file to load. diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index d45957c2..f32dcff0 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -30,6 +30,7 @@ from dvsim.sim.data import ( IPMeta, SimFlowResults, + SimFlowSummary, SimResultsSummary, Testpoint, TestResult, @@ -625,6 +626,7 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None: dvsim_version = None all_flow_results: Mapping[str, SimFlowResults] = {} + flow_summaries: Mapping[str, SimFlowSummary] = {} for item in self.cfgs: item_results = [ @@ -640,7 +642,9 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None: # Convert to lowercase to match filename block_result_index = item.variant_name.lower().replace("/", "_") + all_flow_results[block_result_index] = flow_results + flow_summaries[block_result_index] = flow_results.summary() self.errors_seen |= item.errors_seen @@ -672,13 +676,14 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None: version=dvsim_version, timestamp=timestamp, build_seed=build_seed, - flow_results=all_flow_results, + flow_results=flow_summaries, report_path=reports_dir, ) # Generate all the JSON/HTML reports to the report area. gen_reports( summary=results_summary, + flow_results=all_flow_results, path=reports_dir, ) diff --git a/src/dvsim/sim/report.py b/src/dvsim/sim/report.py index 8d0ebe9e..b94222d8 100644 --- a/src/dvsim/sim/report.py +++ b/src/dvsim/sim/report.py @@ -5,7 +5,7 @@ """Generate reports.""" from collections import defaultdict -from collections.abc import Callable, Collection, Iterable +from collections.abc import Callable, Collection, Iterable, Mapping from datetime import datetime from pathlib import Path from typing import Any, Protocol, TypeAlias @@ -49,7 +49,12 @@ class ReportRenderer(Protocol): format_name: str - def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> ReportArtifacts: + def render( + self, + summary: SimResultsSummary, + flow_results: Mapping[str, SimFlowResults], + outdir: Path | None = None, + ) -> ReportArtifacts: """Render a report of the sim flow results into output artifacts.""" ... @@ -59,14 +64,19 @@ class JsonReportRenderer: format_name = "json" - def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> ReportArtifacts: + def render( + self, + summary: SimResultsSummary, + flow_results: Mapping[str, SimFlowResults], + outdir: Path | None = None, + ) -> ReportArtifacts: """Render a JSON report of the sim flow results into output artifacts.""" if outdir is not None: outdir.mkdir(parents=True, exist_ok=True) artifacts = {} - for results in summary.flow_results.values(): + for results in flow_results.values(): file_name = results.block.variant_name() log.debug("Generating JSON report for '%s'", file_name) block_file = f"{file_name}.json" @@ -88,7 +98,12 @@ class HtmlReportRenderer: format_name = "html" - def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> ReportArtifacts: + def render( + self, + summary: SimResultsSummary, + flow_results: Mapping[str, SimFlowResults], + outdir: Path | None = None, + ) -> ReportArtifacts: """Render a HTML report of the sim flow results into output artifacts.""" if outdir is not None: outdir.mkdir(parents=True, exist_ok=True) @@ -96,7 +111,7 @@ def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> Repo artifacts = {} # Generate block HTML pages - for results in summary.flow_results.values(): + for results in flow_results.values(): file_name = results.block.variant_name() log.debug("Generating HTML report for '%s'", file_name) block_file = f"{file_name}.html" @@ -164,14 +179,19 @@ def __init__(self, html_link_base: Path | None = None, relative_to: Path | None self.html_link_base = html_link_base self.relative_to = relative_to - def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> ReportArtifacts: + def render( + self, + summary: SimResultsSummary, + flow_results: Mapping[str, SimFlowResults], + outdir: Path | None = None, + ) -> ReportArtifacts: """Render a Markdown report of the sim flow results.""" if outdir is not None: outdir.mkdir(parents=True, exist_ok=True) report_md = [ - self.render_block(flow_result)["report.md"] - for flow_result in summary.flow_results.values() + self.render_block(results=flow_result)["report.md"] + for flow_result in flow_results.values() ] report_md.append(self.render_summary(summary)["report.md"]) @@ -184,7 +204,11 @@ def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> Repo def render_block(self, results: SimFlowResults) -> ReportArtifacts: """Render a Markdown report of the sim flow results for a given block/flow.""" # Generate block result metadata information - report_md = self.render_metadata(results.block, results.timestamp, results.build_seed) + report_md = self.render_metadata( + results.block, + results.timestamp, + results.build_seed, + ) testplan_ref = (results.testplan_ref or "").strip() if len(results.stages) > 0 and testplan_ref: report_md += f"\n### [Testplan]({testplan_ref})" @@ -456,7 +480,11 @@ def display_report( sink(header + content + "\n") -def gen_reports(summary: SimResultsSummary, path: Path) -> None: +def gen_reports( + summary: SimResultsSummary, + flow_results: Mapping[str, SimFlowResults], + path: Path, +) -> None: """Generate and display a full set of reports for the given regression run. This helper currently saves JSON and HTML reports to disk (relative to the given path), @@ -464,17 +492,22 @@ def gen_reports(summary: SimResultsSummary, path: Path) -> None: Args: summary: overview of the block results + flow_results: mapping flow names to detailed flow results path: output directory path """ for renderer in (JsonReportRenderer(), HtmlReportRenderer()): - renderer.render(summary, outdir=path) + renderer.render( + summary=summary, + flow_results=flow_results, + outdir=path, + ) renderer = MarkdownReportRenderer(path) # Per-block CLI results are displayed to the `INFO` log if log.isEnabledFor(log.INFO): - for flow_result in summary.flow_results.values(): + for flow_result in flow_results.values(): block_name = flow_result.block.variant_name() log.info("[results]: [%s]", block_name) cli_block = renderer.render_block(flow_result) From 03b374a3b783eb3c276e4bde7e9ec7317ea7dbf0 Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Wed, 11 Mar 2026 18:17:35 +0000 Subject: [PATCH 3/3] fix: links in markdown report when proj-root specified Where the project root is overridden with `--proj-root` and scratch directory is overridden with `--scratch` the HTML report path is not necessarily going to be a subdirectory of the project root. The default `Path.relative_to(...)` raises an exception if the resulting `Path` is `../some/parent/dir`. Signed-off-by: James McCorrie --- src/dvsim/sim/flow.py | 7 ++++++- src/dvsim/sim/report.py | 5 +++-- src/dvsim/utils/fs.py | 40 ++++++++++++++++++++++++++++++++++++++++ tests/utils/test_fs.py | 24 +++++++++++++++++++++++- 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index f32dcff0..909b3798 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -43,6 +43,7 @@ from dvsim.testplan import Testplan from dvsim.tool.utils import get_sim_tool_plugin from dvsim.utils import TS_FORMAT, rm_path +from dvsim.utils.fs import relative_to from dvsim.utils.git import git_commit_hash, git_https_url_with_commit __all__ = ("SimCfg",) @@ -724,7 +725,11 @@ def _gen_json_results( # Build up a reference to the testplan, which might be overridden. if self.testplan_doc_path: - rel_path = Path(self.testplan_doc_path).relative_to(Path(self.proj_root)) + rel_path = relative_to( + Path(self.testplan_doc_path), + Path(self.proj_root), + ) + else: # TODO: testplan variants frequently override `rel_path` for reporting # and build reasons, but do not update the `testplan_doc_path`, meaning diff --git a/src/dvsim/sim/report.py b/src/dvsim/sim/report.py index b94222d8..1cf27a36 100644 --- a/src/dvsim/sim/report.py +++ b/src/dvsim/sim/report.py @@ -17,6 +17,7 @@ from dvsim.sim.data import SimFlowResults, SimResultsSummary from dvsim.templates.render import render_static, render_template from dvsim.utils import TS_FORMAT_LONG +from dvsim.utils.fs import relative_to __all__ = ( "HtmlReportRenderer", @@ -422,9 +423,9 @@ def render_summary(self, summary: SimResultsSummary) -> ReportArtifacts: # Optionally display links to the block HTML reports, relative to the CWD if self.html_link_base is not None: - relative = self.relative_to if self.relative_to is not None else Path(Path.cwd()) + relative = Path(self.relative_to) if self.relative_to is not None else Path.cwd() block_report = self.html_link_base / f"{file_name}.html" - html_report_path = block_report.relative_to(relative) + html_report_path = relative_to(block_report, relative) name_link = f"[{name.upper()}]({html_report_path!s})" else: name_link = name.upper() diff --git a/src/dvsim/utils/fs.py b/src/dvsim/utils/fs.py index d5b6e59b..6b8d398a 100644 --- a/src/dvsim/utils/fs.py +++ b/src/dvsim/utils/fs.py @@ -92,6 +92,46 @@ def mk_symlink(*, path: Path, link: Path) -> None: link.symlink_to(path) +def relative_to(path: Path, other: Path) -> Path: + """Return a relative path from other to path. + + Supports relative paths where path doesn't have to be a sub directory of `other`. + This function is equivalent of path.relative_to(other, walk_up=True) which + is only available from Python 3.12. + + Args: + path: the path to provide a relative path to + other: the base path to take the relative path from + + Returns: + The relative path from other to path. + + """ + path = path.resolve() + other = other.resolve() + + # Find the common ancestor + common = Path( + *[ + p + for p, q in zip( + path.parts, + other.parts, + strict=False, + ) + if p == q + ] + ) + + # How many levels up from `other` to the common ancestor + up_levels = len(other.parts) - len(common.parts) + + # The remaining path down from the common ancestor to `path` + down_path = path.relative_to(common) + + return Path(*([".."] * up_levels), down_path) if up_levels else down_path + + def clean_odirs( odir: Path, max_odirs: int, diff --git a/tests/utils/test_fs.py b/tests/utils/test_fs.py index 34ff11cb..78d60617 100644 --- a/tests/utils/test_fs.py +++ b/tests/utils/test_fs.py @@ -9,7 +9,7 @@ import pytest from hamcrest import assert_that, calling, equal_to, raises -from dvsim.utils.fs import mk_path, mk_symlink, rm_path +from dvsim.utils.fs import mk_path, mk_symlink, relative_to, rm_path def test_symlink_creation(tmp_path: Path) -> None: @@ -155,3 +155,25 @@ def test_path_removal( [p.relative_to(tmp_path) for p in tmp_path.glob("**/*")], equal_to(exp_glob), ) + + +@pytest.mark.parametrize( + ("path", "other", "expected"), + [ + pytest.param(Path("/a/b/c"), Path("/a/b/c"), Path(), id="same_directory"), + pytest.param(Path("/a/b/c/d"), Path("/a/b/c"), Path("d"), id="direct_child"), + pytest.param(Path("/a/b/c/d/e/f"), Path("/a/b/c"), Path("d/e/f"), id="deep_child"), + pytest.param(Path("/a/b/sibling"), Path("/a/b/other"), Path("../sibling"), id="sibling"), + pytest.param(Path("/a/b/c"), Path("/a/b/c/d/e"), Path("../.."), id="multiple_walk_ups"), + pytest.param( + Path("/a/x/y/z"), Path("/a/b/c/d"), Path("../../../x/y/z"), id="walk_up_then_down" + ), + pytest.param( + Path("/x/y/z"), Path("/a/b/c"), Path("../../../x/y/z"), id="only_root_in_common" + ), + pytest.param(Path("/a/b"), Path("/a/b/c/d"), Path("../.."), id="path_is_ancestor"), + ], +) +def test_relative_to(path: Path, other: Path, expected: Path) -> None: + """Test relative_to can create relative paths as expected.""" + assert_that(relative_to(path, other), equal_to(expected))