Refactor HTML/JSON report generation + fix revision information

src/dvsim/job/deploy.py

@@ -117,9 +117,11 @@ def get_job_spec(self) -> "JobSpec":
             block=IPMeta(
                 name=self.sim_cfg.name,
                 variant=self.sim_cfg.variant,
-                commit=self.sim_cfg.revision,
+                commit=self.sim_cfg.commit,
+                commit_short=self.sim_cfg.commit_short,
                 branch=self.sim_cfg.branch,
                 url="",
+                revision_info=self.sim_cfg.revision,
             ),
             tool=ToolMeta(
                 name=self.sim_cfg.tool,

src/dvsim/report/data.py

@@ -24,10 +24,14 @@ class IPMeta(BaseModel):
 
     commit: str
     """Git commit sha of the IP the tests are run against."""
+    commit_short: str
+    """Shortened Git commit sha of the IP the tests are run against."""
     branch: str
     """Git branch"""
     url: str
     """URL to where the IP can be found in git (e.g. github)."""
+    revision_info: str | None
+    """Optional revision info string to use for custom info instead of the above fields."""
 
     def variant_name(self, sep: str = "_") -> str:
         """Name suffixed with the variant for disambiguation."""

src/dvsim/sim/flow.py

@@ -36,7 +36,7 @@
     TestStage,
     ToolMeta,
 )
-from dvsim.sim.report import gen_block_report, gen_summary_report
+from dvsim.sim.report import gen_reports
 from dvsim.sim_results import BucketedFailures, SimResults
 from dvsim.test import Test
 from dvsim.testplan import Testplan
@@ -168,6 +168,11 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None:
 
         super().__init__(flow_cfg_file, hjson_data, args, mk_config)
 
+        # After initialisation & expansion, save some useful revision metadata
+        proj_root = Path(self.proj_root)
+        self.commit = git_commit_hash(path=proj_root, short=False)
+        self.commit_short = git_commit_hash(path=proj_root, short=True)
+
     def _expand(self) -> None:
         # Choose a wave format now. Note that this has to happen after parsing
         # the configuration format because our choice might depend on the
@@ -603,7 +608,6 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
         """
         repo_root = Path(self.proj_root)
         reports_dir = Path(self.scratch_base_path) / "reports"
-        commit = git_commit_hash(path=repo_root)
         url = git_https_url_with_commit(path=repo_root)
         build_seed = self.build_seed if not self.run_only else None
 
@@ -625,21 +629,13 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
 
             flow_results: SimFlowResults = item._gen_json_results(
                 run_results=item_results,
-                commit=commit,
                 url=url,
             )
 
             # Convert to lowercase to match filename
             block_result_index = item.variant_name.lower().replace("/", "_")
             all_flow_results[block_result_index] = flow_results
 
-            # Generate the block's JSON/HTML reports to the report area.
-            gen_block_report(
-                results=flow_results,
-                path=reports_dir,
-                version=dvsim_version,
-            )
-
             self.errors_seen |= item.errors_seen
 
         # The timestamp for this run has been taken with `utcnow()` and is
@@ -658,9 +654,11 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
             top = IPMeta(
                 name=self.name,
                 variant=self.variant,
-                commit=commit,
+                commit=self.commit,
+                commit_short=self.commit_short,
                 branch=self.branch,
                 url=url,
+                revision_info=self.revision,
             )
 
         results_summary = SimResultsSummary(
@@ -673,22 +671,20 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
         )
 
         # Generate all the JSON/HTML reports to the report area.
-        gen_summary_report(
+        gen_reports(
             summary=results_summary,
             path=reports_dir,
         )
 
     def _gen_json_results(
         self,
         run_results: Sequence[CompletedJobStatus],
-        commit: str,
         url: str,
     ) -> SimFlowResults:
         """Generate structured SimFlowResults from simulation run data.
 
         Args:
             run_results: completed job status.
-            commit: git commit Hash
             url: for the IP source
 
         Returns:
@@ -705,9 +701,11 @@ def _gen_json_results(
         block = IPMeta(
             name=self.name.lower(),
             variant=(self.variant or "").lower() or None,
-            commit=commit,
+            commit=self.commit,
+            commit_short=self.commit_short,
             branch=self.branch or "",
             url=url,
+            revision_info=self.revision,
         )
         tool = ToolMeta(name=self.tool.lower(), version="unknown")
 

src/dvsim/sim/report.py

@@ -5,86 +5,136 @@
 """Generate reports."""
 
 from pathlib import Path
+from typing import Protocol, TypeAlias
 
 from dvsim.logging import log
-from dvsim.sim.data import SimFlowResults, SimResultsSummary
+from dvsim.sim.data import SimResultsSummary
 from dvsim.templates.render import render_static, render_template
 
 __all__ = (
-    "gen_block_report",
+    "HtmlReportRenderer",
+    "JsonReportRenderer",
+    "ReportRenderer",
     "gen_reports",
-    "gen_summary_report",
+    "write_report",
 )
 
+# Report rendering returns mappings of relative report paths to (string) contents.
+ReportArtifacts: TypeAlias = dict[str, str]
 
-def gen_block_report(results: SimFlowResults, path: Path, version: str | None = None) -> None:
-    """Generate a block report.
 
-    Args:
-        results: flow results for the block
-        path: output directory path
-        version: dvsim version used to get results for this block
+class ReportRenderer(Protocol):
+    """Renders/formats result reports, returning mappings of relative paths to (string) content."""
 
-    """
-    file_name = results.block.variant_name()
+    format_name: str
 
-    log.debug("generating report '%s'", file_name)
+    def render(self, summary: SimResultsSummary, outdir: Path | None = None) -> ReportArtifacts:
+        """Render a report of the sim flow results into output artifacts."""
+        ...
 
-    path.mkdir(parents=True, exist_ok=True)
 
-    # Save the JSON version
-    (path / f"{file_name}.json").write_text(results.model_dump_json())
+class JsonReportRenderer:
+    """Renders/dumps a JSON report of the sim results."""
 
-    # Generate HTML report
-    (path / f"{file_name}.html").write_text(
-        render_template(
-            path="reports/block_report.html",
-            data={"results": results, "version": version},
-        ),
-    )
+    format_name = "json"
 
+    def render(self, summary: SimResultsSummary, 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)
 
-def make_static_html_report_content(path: Path) -> None:
-    """Generate static style CSS/JS files for HTML reporting."""
-    for name in (
-        "css/style.css",
-        "css/bootstrap.min.css",
-        "js/bootstrap.bundle.min.js",
-        "js/htmx.min.js",
-    ):
-        output = path / name
-        output.parent.mkdir(parents=True, exist_ok=True)
-        output.write_text(render_static(path=name))
+        artifacts = {}
 
+        for results in summary.flow_results.values():
+            file_name = results.block.variant_name()
+            log.debug("Generating JSON report for '%s'", file_name)
+            block_file = f"{file_name}.json"
+            artifacts[block_file] = results.model_dump_json()
+            if outdir is not None:
+                (outdir / block_file).write_text(artifacts[block_file])
 
-def gen_summary_report(summary: SimResultsSummary, path: Path) -> None:
-    """Generate a summary report.
+        top_log_suffix = "" if summary.top is None else f" for {summary.top.name}"
+        log.debug("Generating JSON summary report%s", top_log_suffix)
+        artifacts["index.json"] = summary.model_dump_json()
+        if outdir is not None:
+            (outdir / "index.json").write_text(artifacts["index.json"])
 
-    Args:
-        summary: overview of the block results
-        path: output directory path
+        return artifacts
 
-    """
-    log.debug("generating summary report")
 
-    path.parent.mkdir(parents=True, exist_ok=True)
+class HtmlReportRenderer:
+    """Renders a HTML report of the sim results."""
+
+    format_name = "html"
+
+    def render(self, summary: SimResultsSummary, 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)
 
-    # Save the JSON version
-    (path / "index.json").write_text(summary.model_dump_json())
+        artifacts = {}
 
-    # Generate style CSS
-    make_static_html_report_content(path)
+        # Generate block HTML pages
+        for results in summary.flow_results.values():
+            file_name = results.block.variant_name()
+            log.debug("Generating HTML report for '%s'", file_name)
+            block_file = f"{file_name}.html"
+            artifacts[block_file] = render_template(
+                path="reports/block_report.html",
+                data={"results": results, "version": summary.version},
+            )
+            if outdir is not None:
+                (outdir / block_file).write_text(artifacts[block_file])
 
-    # Generate HTML report. Regardless of whether we have a top or there is only
-    # one block, we always generate a summary page for now.
-    (path / "index.html").write_text(
-        render_template(
+        # Regardless of whether we have a top or there is only one block, we always generate a
+        # summary page for now.
+        top_log_suffix = "" if summary.top is None else f" for {summary.top.name}"
+        log.debug("Generating HTML summary report%s", top_log_suffix)
+        artifacts["index.html"] = render_template(
             path="reports/summary_report.html",
-            data={
-                "summary": summary,
-            },
-        ),
-    )
+            data={"summary": summary},
+        )
+        if outdir is not None:
+            (outdir / "index.html").write_text(artifacts["index.html"])
+
+        # Generate other static site contents
+        artifacts.update(self.render_static_content(outdir))
+
+        return artifacts
+
+    def render_static_content(self, outdir: Path | None = None) -> ReportArtifacts:
+        """Render static CSS / JS artifacts for HTML report generation."""
+        static_files = [
+            "css/style.css",
+            "css/bootstrap.min.css",
+            "js/bootstrap.bundle.min.js",
+            "js/htmx.min.js",
+        ]
+
+        artifacts = {}
+
+        for name in static_files:
+            artifacts[name] = render_static(path=name)
+            if outdir is not None:
+                artifact_path = outdir / name
+                artifact_path.parent.mkdir(parents=True, exist_ok=True)
+                artifact_path.write_text(artifacts[name])
+
+        return artifacts
+
+
+def write_report(files: ReportArtifacts, root: Path) -> None:
+    """Write rendered report artifacts to the file system, relative to a given path.
+
+    Args:
+        files: the output report artifacts from rendering simulation results.
+        root: the path to write the report files relative to.
+
+    """
+    for relative_path, content in files.items():
+        path = root / relative_path
+        path.parent.mkdir(parents=True, exist_ok=True)
+        path.write_text(content)
 
 
 def gen_reports(summary: SimResultsSummary, path: Path) -> None:
@@ -95,7 +145,5 @@ def gen_reports(summary: SimResultsSummary, path: Path) -> None:
         path: output directory path
 
     """
-    gen_summary_report(summary=summary, path=path)
-
-    for flow_result in summary.flow_results.values():
-        gen_block_report(results=flow_result, path=path, version=summary.version)
+    for renderer in (JsonReportRenderer(), HtmlReportRenderer()):
+        renderer.render(summary, outdir=path)

src/dvsim/templates/reports/block_report.html

@@ -51,7 +51,7 @@ <h2>Simulation Results: {{ block.variant_name(sep='/') }}</h2>
             {% endif %}
             <a class="badge text-bg-secondary link-underline link-underline-opacity-0"
                href="{{ block.url }}">
-                sha: {{ block.commit[:7] }}
+                sha: {{ block.commit_short }}
             </a>
             <a class="badge text-bg-secondary link-underline link-underline-opacity-0"
                         href="{{ block.variant_name() }}.json">json</a>

src/dvsim/templates/reports/summary_report.html

@@ -51,7 +51,7 @@ <h2>{{ title }}</h2>
             {% endif %}
             <a class="badge text-bg-secondary link-underline link-underline-opacity-0"
                 href="{{ meta_info.url }}">
-                sha: {{ meta_info.commit[:7] }}
+                sha: {{ meta_info.commit_short }}
             </a>
             <a class="badge text-bg-secondary link-underline link-underline-opacity-0"
                 href="index.json">json</a>

src/dvsim/utils/git.py

@@ -28,7 +28,7 @@ def repo_root(path: Path) -> Path | None:
     return None
 
 
-def git_commit_hash(path: Path | None = None) -> str:
+def git_commit_hash(path: Path | None = None, *, short: bool = False) -> str:
     """Hash of the current git commit."""
     root = repo_root(path=path or Path.cwd())
 
@@ -38,6 +38,9 @@ def git_commit_hash(path: Path | None = None) -> str:
 
     r = Repo(root)
 
+    if short:
+        return r.git.rev_parse(r.head, short=True)
+
     return r.head.commit.hexsha
 
 

tests/test_scheduler.py

@@ -247,8 +247,10 @@ def ip_meta_factory(**overrides: str | None) -> IPMeta:
         "name": "test_ip",
         "variant": None,
         "commit": "test_commit",
+        "commit_short": "test",
         "branch": "test_branch",
         "url": "test_url",
+        "revision_info": None,
     }
     meta.update(overrides)
     return IPMeta(**meta)

tests/utils/test_git.py

@@ -58,6 +58,20 @@ def test_git_commit_hash(tmp_path: Path) -> None:
             equal_to(r.head.commit.hexsha),
         )
 
+    @staticmethod
+    def test_git_short_commit_hash(tmp_path: "Path") -> None:
+        """Test that the expected shortened git commit sha is returned."""
+        r = Repo.init(path=tmp_path)
+
+        file = tmp_path / "a"
+        file.write_text("file to commit")
+        r.index.add([file])
+        r.index.commit("initial commit")
+
+        assert_that(
+            git_commit_hash(tmp_path, short=True), equal_to(r.git.rev_parse(r.head, short=True))
+        )
+
     @staticmethod
     def test_git_origin_url(tmp_path: Path) -> None:
         """Test that the expected git remote origin url is returned."""