diff --git a/doc/design_doc.md b/doc/design_doc.md index 7865918c..71bf8b9f 100644 --- a/doc/design_doc.md +++ b/doc/design_doc.md @@ -149,6 +149,31 @@ Within any of these files, `{file_dir}` names the directory that holds that file `{self_dir}`, by contrast, is the directory of the configuration file that dvsim loaded, at the top of the import tree. `{file_dir}` is replaced as each file is loaded, so it can only be used in Hjson files, not in options given on the command line. +A configuration can also be extended from outside the project tree with `--cfg-extend NAME=PATH`, for example to add tests that need a licensed verification IP that the project itself can't hold: + +```hjson +// ext.hjson, merged with: dvsim top_sim_cfgs.hjson --cfg-extend foo=/path/to/ext.hjson ... +{ + import_cfgs: ["{file_dir}/vip_tests.hjson"] + build_modes: [{name: "vip", build_opts: ["-f {file_dir}/vip.f"]}] + overrides: [{name: "foo_setting", value: "with_vip"}] +} +``` + +The Hjson file at `PATH`, with its imports, is merged into the configuration whose `name` is `NAME` once that configuration has loaded all of its own files; this works whether that configuration is run on its own or is a child of a primary configuration. +`NAME` is matched against the name that the configuration's own files give it, before any `overrides`. +The usual merge rules apply: the extension's list entries follow the configuration's own, and a conflicting scalar is an error, so an extension changes a scalar with `overrides` (but not one that the configuration already overrides). +In an extension, `{self_dir}` is still the directory of the configuration file that dvsim loaded, so the extension names its own files with `{file_dir}`. +Several extensions apply in command-line order. +An extension whose `NAME` matches no loaded configuration is an error, as is naming one extension twice. +With `--on-cfg-extend-error skip`, an extension that can't be loaded or merged, or whose `overrides` don't fit the configuration, is left out with a warning instead: the configuration runs as if the extension had not been named, later extensions still apply, and its reports say which extension was skipped and why. +An extension's `overrides` can only name keys that the configuration or an earlier extension define. +In that mode, an extension whose `NAME` matches nothing only gets a warning, as no report belongs to it. +Errors found after the merge are not covered: for example an unknown wildcard in one of the extension's values, a test that names a build mode that doesn't exist, or a failing build. +The sim reports and JSON results list the extensions that each configuration matched, with the git revision of the checkout that holds each extension file: its `origin` (without credentials), its commit, and whether it was dirty, which includes the extension file itself not being tracked. +Files that an extension imports from elsewhere are not recorded. +The JSON results of a configuration that merged no extension have no `extensions` key, as before extensions were recorded. + ### Parser stage The **parser stage** parses all Hjson files in the tree and constructs a **flow manager object**. diff --git a/src/dvsim/cli/run.py b/src/dvsim/cli/run.py index 60d36877..9592551f 100644 --- a/src/dvsim/cli/run.py +++ b/src/dvsim/cli/run.py @@ -37,6 +37,7 @@ load_config_file, proj_root_from_config, ) +from dvsim.flow.extension import CfgExtension from dvsim.flow.factory import make_cfg from dvsim.fusesoc import CONFIG_SECTION as FUSESOC_CONFIG_SECTION from dvsim.fusesoc import resolve_options as resolve_fusesoc_options @@ -185,6 +186,14 @@ def parse_resource(s: str) -> tuple[str, int | None]: raise argparse.ArgumentTypeError(msg) from e +def parse_cfg_extension(s: str) -> CfgExtension: + """Parse a cfg extension (NAME=PATH) from the argparse CLI.""" + try: + return CfgExtension.parse(s) + except ValueError as e: + raise argparse.ArgumentTypeError(str(e)) from e + + def resolve_branch(branch, cwd=None): """Choose a branch name for output files. @@ -453,6 +462,34 @@ def parse_args(argv: list[str] | None = None): ), ) + whatg.add_argument( + "--cfg-extend", + action="append", + default=[], + type=parse_cfg_extension, + metavar="NAME=PATH", + help=( + "Merge the hjson file PATH (and its imports) into the cfg whose " + "'name' is NAME, after the cfg's own files. NAME may be a child of " + "a primary config. Lists are appended to; scalars are changed " + "with 'overrides'. PATH names its own files with {file_dir}. " + "Repeatable; applied in order. It is an error if NAME matches no cfg." + ), + ) + + whatg.add_argument( + "--on-cfg-extend-error", + type=str.lower, + choices=["error", "skip"], + default="error", + help=( + "What to do when a --cfg-extend file can't be loaded, merged or " + "overridden, or names no cfg: stop the run (error), or leave the " + "extension out, run the cfg without it and say so in its report " + "(skip). Defaults to %(default)s." + ), + ) + disg = parser.add_argument_group("Dispatch options") disg.add_argument( diff --git a/src/dvsim/flow/base.py b/src/dvsim/flow/base.py index c6badf9a..e8e145e6 100644 --- a/src/dvsim/flow/base.py +++ b/src/dvsim/flow/base.py @@ -17,11 +17,18 @@ import hjson import dvsim.instrumentation.runtime as instrumentation +from dvsim.flow.extension import ( + CfgExtension, + check_cfg_extension_overrides, + load_cfg_extension, + override_names, +) from dvsim.flow.hjson import set_target_attribute from dvsim.fusesoc import rewrite_fusesoc_opts from dvsim.job.data import CompletedJobStatus, JobSpec, WorkspaceConfig from dvsim.job.status import JobStatus from dvsim.logging import log +from dvsim.report.data import CfgExtensionMeta from dvsim.scheduler.resources import UnknownResourcePolicy from dvsim.scheduler.runner import ( build_default_scheduler_backend, @@ -33,7 +40,7 @@ rm_path, subst_wildcards, ) -from dvsim.utils.git import git_commit_hash, git_is_dirty +from dvsim.utils.git import git_commit_hash, git_is_dirty, git_revision if TYPE_CHECKING: from dvsim.job.deploy import Deploy @@ -147,7 +154,16 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: # configuration". self.is_primary_cfg = "use_cfgs" in hjson_data + # The cfg extensions (--cfg-extend) that this cfg matched, in order, as + # the reports describe them (merged, or skipped with the reason), and + # the name that they were matched against. + self.cfg_extensions: dict[CfgExtension, CfgExtensionMeta] = {} + self.cfg_extend_name: str | None = None + if not self.is_primary_cfg: + # Extensions merge after everything the cfg loads itself, and + # before overrides and wildcards are processed. + self._merge_cfg_extensions() self.cfgs.append(self) else: for entry in self.use_cfgs: @@ -205,6 +221,63 @@ def _merge_hjson(self, hjson_data: Mapping) -> None: for key, value in hjson_data.items(): set_target_attribute(self.flow_cfg_file, self.__dict__, key, value) + def _merge_cfg_extensions(self) -> None: + """Merge in the cfg extensions (--cfg-extend) that name this cfg. + + They apply in command-line order. Their list values follow the cfg's + own, and their overrides join the cfg's own. + """ + # The name as the cfg's own files give it. Overrides could still change + # it, but those are processed after the extensions merge. + name = subst_wildcards(self.name, self.__dict__, ignore_error=True) + self.cfg_extend_name = name + + # What load_hjson needs to resolve the names of imported files. As in + # the cfg's own files, {self_dir} is the cfg's directory: an extension + # names its own files with {file_dir}. + initial_values = {"proj_root": self.proj_root, "self_dir": self.self_dir} + if self.tool: + initial_values["tool"] = self.tool + # Only a tool given on the command line can't be changed by the files. + arg_keys = {"tool"} if self.args.tool is not None else set() + + for ext in self.args.cfg_extend: + if ext.cfg_name != name: + continue + log.info("[cfg_extend]: %s: %s", name, ext.path) + meta = CfgExtensionMeta(path=str(ext.path), revision=git_revision(ext.path)) + self.cfg_extensions[ext] = meta + # The overrides set before this extension, by the cfg itself or an + # earlier extension. + taken = override_names(self.__dict__.get("overrides", [])) + ext_data = {} + saved = {} + try: + ext_data = load_cfg_extension(ext, initial_values, arg_keys) + # Keep the values that the merge will change, to put them back + # if the extension is skipped. set_target_attribute extends lists + # in place, so the merge goes into copies of them. + saved = {k: self.__dict__[k] for k in ext_data if k in self.__dict__} + for key, value in saved.items(): + if isinstance(value, list): + self.__dict__[key] = list(value) + for key, value in ext_data.items(): + set_target_attribute(str(ext.path), self.__dict__, key, value) + check_cfg_extension_overrides(ext_data.get("overrides", []), self, taken) + except RuntimeError as err: + # Most messages start with the extension's path, which these give already. + reason = str(err).removeprefix(f"{str(ext.path)!r}: ") + if self.args.on_cfg_extend_error != "skip": + log.error("Cannot extend cfg %r with %s: %s", name, ext.path, reason) + sys.exit(1) + log.warning("Skipping cfg extension %s for %r: %s", ext.path, name, reason) + for key in ext_data: + if key in saved: + self.__dict__[key] = saved[key] + else: + self.__dict__.pop(key, None) + self.cfg_extensions[ext] = meta.model_copy(update={"skipped": reason}) + def _expand(self) -> None: """Expand wildcards after merging hjson. diff --git a/src/dvsim/flow/extension.py b/src/dvsim/flow/extension.py new file mode 100644 index 00000000..422edcf5 --- /dev/null +++ b/src/dvsim/flow/extension.py @@ -0,0 +1,168 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Cfg extensions: hjson files merged into a named flow cfg (--cfg-extend). + +See the "DUT configuration" section of doc/design_doc.md. +""" + +import os +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from pathlib import Path + +from dvsim.flow.hjson import load_hjson + +__all__ = ( + "CfgExtension", + "check_cfg_extension_overrides", + "check_cfg_extensions_used", + "load_cfg_extension", + "override_names", +) + + +@dataclass(frozen=True) +class CfgExtension: + """An extension file, and the name of the cfg it extends.""" + + cfg_name: str + path: Path + + @classmethod + def parse(cls, value: str) -> "CfgExtension": + """Parse NAME=PATH, as given to --cfg-extend. + + A relative PATH is taken from the current directory. Symlinks are + kept, as they are in {file_dir}. + """ + name, sep, path = value.partition("=") + name, path = name.strip(), path.strip() + if not sep or not name or not path: + msg = f"Invalid cfg extension {value!r}: expected NAME=PATH." + raise ValueError(msg) + return cls(name, Path(os.path.normpath(Path(path).absolute()))) + + +def load_cfg_extension( + ext: CfgExtension, initial_values: Mapping[str, object], arg_keys: Iterable[str] +) -> dict: + """Load an extension file and its imports. + + initial_values holds what is needed to resolve the names of imported files, + and arg_keys those of its keys that came from the command line (as for + load_hjson). The returned data holds only what the extension sets. + + Raises a RuntimeError if the file can't be loaded or isn't allowed. + """ + path = str(ext.path) + if not ext.path.is_file(): + msg = f"{path!r}: Not a file." + raise RuntimeError(msg) + + try: + data = load_hjson(path, initial_values, set(arg_keys)) + except SystemExit as err: + # parse_hjson logs why a file can't be parsed and exits. Raise instead, + # so that --on-cfg-extend-error skip can leave the extension out. + msg = f"{path!r}: It, or a file it imports, can't be parsed (see above)." + raise RuntimeError(msg) from err + except ValueError as err: + # An import_cfgs entry with a wildcard that can't be resolved. + msg = f"{path!r}: Can't resolve the name of an imported file: {err}" + raise RuntimeError(msg) from err + + if "use_cfgs" in data: + msg = f"{path!r}: An extension can't set use_cfgs." + raise RuntimeError(msg) + + # The initial values are the cfg's own. If the extension set one of them, + # it set it to the same value (or load_hjson reported a conflict), so + # merging it would change nothing. + for key in initial_values: + data.pop(key, None) + + return data + + +def check_cfg_extensions_used( + extensions: Iterable[CfgExtension], + used: Iterable[CfgExtension], + cfg_names: Iterable[str], + primary_name: str | None, +) -> list[str]: + """Check that every extension was used by a loaded cfg. + + used are the extensions that some cfg matched. cfg_names are the names that + the cfgs that can be extended had when they matched them (before any + overrides); a primary cfg can't be extended, so its name is passed + separately to give a clearer message. Returns a list of error messages + (empty if all is well). + """ + used = set(used) + names = set(cfg_names) + errors = [] + for ext in extensions: + if ext in used: + continue + if ext.cfg_name == primary_name: + errors.append( + f"Cfg extension {ext.path} names the primary cfg {ext.cfg_name!r}: " + "only a cfg that runs jobs (a child, or a cfg run on its own) can be extended." + ) + else: + known = ", ".join(sorted(names)) or "none" + errors.append( + f"Cfg extension {ext.path} names {ext.cfg_name!r}, but no loaded cfg has " + f"that name (loaded: {known})." + ) + return errors + + +def override_names(overrides: object) -> set[str]: + """Return the names of the keys that an ``overrides`` list sets.""" + if not isinstance(overrides, list): + return set() + return {o["name"] for o in overrides if isinstance(o, dict) and isinstance(o.get("name"), str)} + + +def check_cfg_extension_overrides(overrides: object, cfg: object, taken: Iterable[str]) -> None: + """Check an extension's overrides as the cfg will apply them. + + The cfg processes overrides only once every file is merged (see + FlowCfg._process_overrides, whose checks these mirror). Checking an + extension's own here, against the cfg it has just been merged into, lets + a broken one (most often, a key that the cfg no longer has) be reported + as that extension's fault. So an extension can only override keys that + the cfg or an earlier extension define. taken are the keys already + overridden before this extension. + + Raises a RuntimeError for the first problem found (its message gives the reason only). + """ + if not isinstance(overrides, list): + msg = "Its 'overrides' must be a list." + raise RuntimeError(msg) + taken = set(taken) + for item in overrides: + if not (isinstance(item, dict) and set(item) == {"name", "value"}): + msg = f"Each of its overrides needs a name and a value, not {item!r}." + raise RuntimeError(msg) + name, value = item["name"], item["value"] + if not isinstance(name, str): + msg = f"Each of its overrides needs a key name, not {name!r}." + raise RuntimeError(msg) + if name in taken: + msg = f"It overrides {name!r}, which is already overridden." + raise RuntimeError(msg) + if not hasattr(cfg, name): + msg = f"It overrides {name!r}, which the cfg doesn't have." + raise RuntimeError(msg) + old = getattr(cfg, name) + if not isinstance(value, type(old)): + msg = ( + f"It overrides {name!r} with {value!r}, " + f"which doesn't match the type of its value {old!r}." + ) + raise RuntimeError(msg) + taken.add(name) diff --git a/src/dvsim/flow/factory.py b/src/dvsim/flow/factory.py index 166db158..ca90edc1 100644 --- a/src/dvsim/flow/factory.py +++ b/src/dvsim/flow/factory.py @@ -7,6 +7,7 @@ from dvsim.flow.base import FlowCfg from dvsim.flow.cdc import CdcCfg +from dvsim.flow.extension import check_cfg_extensions_used from dvsim.flow.formal import FormalCfg from dvsim.flow.hjson import load_hjson from dvsim.flow.lint import LintCfg @@ -113,6 +114,13 @@ def make_cfg(path, args, proj_root) -> FlowCfg: of the project. """ + # An extension named twice would be merged twice. + repeated = [ext for ext in dict.fromkeys(args.cfg_extend) if args.cfg_extend.count(ext) > 1] + for ext in repeated: + log.error("Cfg extension %s=%s is given more than once.", ext.cfg_name, ext.path) + if repeated: + sys.exit(1) + initial_values = { "proj_root": proj_root, "self_dir": pathlib.Path(path).parent, @@ -131,4 +139,24 @@ def factory(child_path): child_ivs["flow"] = hjson_data["flow"] return _make_child_cfg(child_path, args, child_ivs) - return cls(path, hjson_data, args, factory) + cfg = cls(path, hjson_data, args, factory) + + # Every extension must have found its cfg. A name that matches nothing is + # a typo or a cfg that has been renamed, and would otherwise pass silently. + # With --on-cfg-extend-error skip, the rest of the run goes ahead. + errors = check_cfg_extensions_used( + args.cfg_extend, + [ext for child in cfg.cfgs for ext in child.cfg_extensions], + [child.cfg_extend_name for child in cfg.cfgs], + cfg.name if cfg.is_primary_cfg else None, + ) + skip = args.on_cfg_extend_error == "skip" + for error in errors: + if skip: + log.warning("Skipping: %s", error) + else: + log.error("%s", error) + if errors and not skip: + sys.exit(1) + + return cfg diff --git a/src/dvsim/flow/hjson.py b/src/dvsim/flow/hjson.py index 37187667..9d946c0d 100644 --- a/src/dvsim/flow/hjson.py +++ b/src/dvsim/flow/hjson.py @@ -17,7 +17,7 @@ _FILE_DIR_WILDCARD = "{file_dir}" -def load_hjson(path, initial_values): +def load_hjson(path, initial_values, arg_keys=None): """Load an hjson file and any includes. Combines them all into a single dictionary, which is then returned. This @@ -29,6 +29,10 @@ def load_hjson(path, initial_values): is not modified). It needs to contain values for anything needed to resolve include files (typically, this is 'proj_root' and 'tool' (if set)). + arg_keys are the keys of initial_values that came from the command line, + which the files can't change. By default, that is any of them that the + command line can set. + """ worklist = [path] seen = {path} @@ -38,7 +42,8 @@ def load_hjson(path, initial_values): # Figure out a list of fields that had a value from the command line. These # should have been passed in as part of initial_values and we need to know # that we can safely ignore updates. - arg_keys = _CMDLINE_FIELDS & initial_values.keys() + if arg_keys is None: + arg_keys = _CMDLINE_FIELDS & initial_values.keys() while worklist: next_path = worklist.pop() diff --git a/src/dvsim/report/data.py b/src/dvsim/report/data.py index e385d204..a16531c0 100644 --- a/src/dvsim/report/data.py +++ b/src/dvsim/report/data.py @@ -4,9 +4,13 @@ """Report data models.""" +from pathlib import Path + from pydantic import BaseModel, ConfigDict __all__ = ( + "CfgExtensionMeta", + "GitRevisionMeta", "IPMeta", "ToolMeta", ) @@ -50,3 +54,38 @@ class ToolMeta(BaseModel): """Name of the tool.""" version: str """Version of the tool.""" + + +class CfgExtensionMeta(BaseModel): + """A cfg extension (--cfg-extend) that a flow's cfg matched: merged, or skipped.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + path: str + """Path of the extension file.""" + skipped: str | None = None + """Why the extension was left out (--on-cfg-extend-error skip), or None if + it was merged.""" + revision: "GitRevisionMeta | None" = None + """The git revision of the checkout that holds the extension file (not of + any files it imports from elsewhere), or None if it isn't in git.""" + + @property + def name(self) -> str: + """The name of the extension file.""" + return Path(self.path).name + + +class GitRevisionMeta(BaseModel): + """The git revision of a checkout, when the run started.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + origin: str | None + """URL of the ``origin`` remote (credentials stripped), or None.""" + commit: str + """Git commit.""" + commit_short: str + """Shortened git commit.""" + dirty: bool + """Whether the checkout had uncommitted changes, or didn't track the file.""" diff --git a/src/dvsim/sim/data.py b/src/dvsim/sim/data.py index 888dfad1..2f61874b 100644 --- a/src/dvsim/sim/data.py +++ b/src/dvsim/sim/data.py @@ -7,10 +7,11 @@ from collections.abc import Mapping from datetime import datetime from pathlib import Path +from typing import Any -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, SerializerFunctionWrapHandler, model_serializer -from dvsim.report.data import IPMeta, ToolMeta +from dvsim.report.data import CfgExtensionMeta, IPMeta, ToolMeta from dvsim.sim_results import BucketedFailures __all__ = ( @@ -169,6 +170,17 @@ def flattened(self) -> dict[str, float | None]: return items +def _without_empty_extensions(data: dict[str, Any]) -> dict[str, Any]: + """Leave out the extensions of a flow that has none. + + A flow run without --cfg-extend then has the keys it had before extensions were recorded, so a + reader built for those (whose models forbid unknown keys) can still load it. + """ + if not data.get("extensions"): + data.pop("extensions", None) + return data + + class SimFlowResults(BaseModel): """Flow results data.""" @@ -207,6 +219,13 @@ class SimFlowResults(BaseModel): percent: float """Percentage test pass rate.""" + extensions: tuple[CfgExtensionMeta, ...] = () + """The cfg extensions (--cfg-extend) named for this flow.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_empty_extensions(handler(self)) + def summary(self) -> "SimFlowSummary": """Load results from JSON file. @@ -247,6 +266,13 @@ class SimFlowSummary(BaseModel): percent: float """Percentage test pass rate.""" + extensions: tuple[CfgExtensionMeta, ...] = () + """The cfg extensions (--cfg-extend) named for this flow.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_empty_extensions(handler(self)) + @staticmethod def load(path: Path) -> "SimFlowSummary": """Load results from JSON file. diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index a907a832..9a0fc75a 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -877,6 +877,8 @@ def make_test_result(tr) -> TestResult | None: if failures.buckets: self.errors_seen = True + extensions = self.cfg_extensions.values() + # --- Final result --- return SimFlowResults( block=block, @@ -893,6 +895,7 @@ def make_test_result(tr) -> TestResult | None: passed=total_passed, total=total_runs, percent=100.0 * total_passed / total_runs if total_runs else 0.0, + extensions=tuple(extensions), ) def _fake_policy(self, job: JobSpec) -> JobStatus: diff --git a/src/dvsim/sim/report.py b/src/dvsim/sim/report.py index bdda6311..a859d178 100644 --- a/src/dvsim/sim/report.py +++ b/src/dvsim/sim/report.py @@ -14,7 +14,7 @@ from dvsim.logging import log from dvsim.report.artifacts import ReportArtifacts, display_report, render_static_content -from dvsim.report.data import IPMeta +from dvsim.report.data import CfgExtensionMeta, IPMeta from dvsim.sim.data import SimFlowResults, SimResultsSummary from dvsim.templates.render import render_template from dvsim.utils import TS_FORMAT_LONG @@ -159,6 +159,16 @@ def render( return artifacts +def _revision(ext: CfgExtensionMeta) -> str: + """Describe the git revision of a cfg extension, for the Markdown reports.""" + rev = ext.revision + if rev is None: + return "" + dirty = ", dirty" if rev.dirty else "" + origin = f", from {rev.origin}" if rev.origin else "" + return f" (revision `{rev.commit_short}`{dirty}{origin})" + + class MarkdownReportRenderer: """Renders a Markdown report of the sim results.""" @@ -213,6 +223,16 @@ def render_block(self, results: SimFlowResults) -> ReportArtifacts: if len(results.stages) > 0 and testplan_ref: report_md += f"\n### [Testplan]({testplan_ref})" report_md += f"\n### Simulator: {results.tool.name.upper()}" + merged = [ext for ext in results.extensions if ext.skipped is None] + if merged: + report_md += "\n### Cfg extensions:\n" + report_md += "".join(f"\n* `{ext.path}`{_revision(ext)}" for ext in merged) + for ext in results.extensions: + if ext.skipped is not None: + report_md += ( + f"\n\n> **Warning:** cfg extension `{ext.path}`{_revision(ext)} skipped: " + f"{ext.skipped}\n" + ) # Record a summary of the simulation results, coverage, and failure buckets, if applicable. result_summary = self.render_block_results(results) @@ -437,6 +457,9 @@ def render_summary(self, summary: SimResultsSummary) -> ReportArtifacts: name_link = f"[{name.upper()}]({html_report_path!s})" else: name_link = name.upper() + skipped = sum(ext.skipped is not None for ext in flow_result.extensions) + if skipped: + name_link += f" ({skipped} {_plural('extension', skipped)} skipped)" table.append( { diff --git a/src/dvsim/templates/reports/block_report.html b/src/dvsim/templates/reports/block_report.html index 4f6177fd..0bd406cf 100644 --- a/src/dvsim/templates/reports/block_report.html +++ b/src/dvsim/templates/reports/block_report.html @@ -36,6 +36,18 @@

Simulation Results: {{ block.variant_name(sep='/') }}

+ {% for ext in results.extensions|selectattr("skipped", "ne", none) %} +
+
+ +
+
+ {% endfor %} +
 
@@ -58,6 +70,15 @@

Simulation Results: {{ block.variant_name(sep='/') }}

Tool: {{ tool.name }} [{{ tool.version }}] + {% for ext in results.extensions|selectattr("skipped", "none") %} + {% set rev = ext.revision %} + + Extension: {{ ext.name }} + {%- if rev %} sha: {{ rev.commit_short }}{% endif %} + {%- if rev and rev.dirty %} (dirty){% endif %} + + {% endfor %} {% if build_seed %} Build seed: {{ build_seed }} diff --git a/src/dvsim/templates/reports/summary_report.html b/src/dvsim/templates/reports/summary_report.html index 77fbdc94..be2791c9 100644 --- a/src/dvsim/templates/reports/summary_report.html +++ b/src/dvsim/templates/reports/summary_report.html @@ -124,6 +124,13 @@

{{ title }}

{{ flow.block.variant_name(sep='/') | upper }}
+ {% set skipped = flow.extensions|selectattr("skipped", "ne", none)|list %} + {% if skipped %} + + {{ skipped|length }} extension{{ "s" if skipped|length > 1 }} skipped + + {% endif %} {{ flow.passed }} {{ flow.total }} diff --git a/src/dvsim/utils/git.py b/src/dvsim/utils/git.py index 0776d9de..a896f1bb 100644 --- a/src/dvsim/utils/git.py +++ b/src/dvsim/utils/git.py @@ -8,8 +8,10 @@ from urllib.parse import urlsplit, urlunsplit from git import Repo +from git.exc import GitCommandError, GitError from dvsim.logging import log +from dvsim.report.data import GitRevisionMeta __all__ = ("repo_root",) @@ -124,3 +126,31 @@ def git_https_url_with_commit(path: Path | None = None) -> str | None: url = "https://github.com/" + url.removeprefix(prefix) return f"{url}/tree/{commit}" + + +def git_revision(path: Path) -> GitRevisionMeta | None: + """Describe the checkout that holds path, for run metadata. + + Returns None if path isn't in a git checkout or its revision can't be read. + The checkout counts as dirty if path itself isn't tracked (untracked or + ignored), as the commit doesn't hold it. + """ + root = repo_root(path) + if root is None: + return None + try: + repo = Repo(root) + try: + repo.git.ls_files("--error-unmatch", str(path)) + tracked = True + except GitCommandError: + tracked = False + return GitRevisionMeta( + origin=git_origin_url(path=root), + commit=git_commit_hash(path=root), + commit_short=git_commit_hash(path=root, short=True), + dirty=git_is_dirty(path=root) or not tracked, + ) + except (GitError, ValueError) as err: + log.debug("Cannot read the git revision of %s: %s", root, err) + return None diff --git a/tests/flow/test_extension.py b/tests/flow/test_extension.py new file mode 100644 index 00000000..d0da6f71 --- /dev/null +++ b/tests/flow/test_extension.py @@ -0,0 +1,428 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for cfg extensions (--cfg-extend).""" + +import subprocess +from collections.abc import Sequence +from pathlib import Path + +import pytest + +from dvsim.cli.run import parse_args +from dvsim.flow.base import FlowCfg +from dvsim.flow.extension import CfgExtension, check_cfg_extensions_used +from dvsim.flow.factory import make_cfg + +__all__ = () + +# A sim cfg with just enough in it to load. Like a real block cfg, it gets its +# tool options from an imported file, so the test can check that an +# extension's list entries come after those. +CHILD_CFG = """{ + name: blk + flow: sim + tool: vcs + scratch_path: "{scratch_root}/blk" + import_cfgs: ["{proj_root}/hw/tool.hjson"] + build_modes: [{name: "waves_off"}, {name: "xprop"}, {name: "build_seed"}] + tests: [{name: "t1"}] + regressions: [] + testplan: "" + blk_var: "orig" + blk_empty: "" +}""" + +TOOL_CFG = '{build_opts: ["-f {proj_root}/sv.f"], run_opts: ["+tool"]}' + +PRIMARY_CFG = """{ + name: top + flow: sim + scratch_path: "{scratch_root}/top" + use_cfgs: ["{proj_root}/hw/blk_sim_cfg.hjson"] +}""" + + +@pytest.fixture +def proj(tmp_path: Path) -> Path: + """A git repo holding a primary cfg and its one child, "blk".""" + root = tmp_path.resolve() / "proj" + (root / "hw").mkdir(parents=True) + (root / "hw" / "blk_sim_cfg.hjson").write_text(CHILD_CFG) + (root / "hw" / "tool.hjson").write_text(TOOL_CFG) + (root / "hw" / "top_sim_cfgs.hjson").write_text(PRIMARY_CFG) + git = ["git", "-C", str(root)] + subprocess.run([*git, "init", "-q"], check=True) + subprocess.run([*git, "commit", "-q", "--allow-empty", "-m", "init"], check=True) + return root + + +def _write(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + return path + + +def _load(proj: Path, cfg: str, *extends: str, argv: Sequence[str] = ()) -> FlowCfg: + argv = [str(proj / "hw" / cfg), "--scratch-root", str(proj.parent / "scratch"), *argv] + argv += ["--branch", "b"] + for ext in extends: + argv += ["--cfg-extend", ext] + args = parse_args(argv) + args.timestamp = args.timestamp_long = "ts" + return make_cfg(args.cfg, args, str(proj)) + + +def _blk(cfg: FlowCfg) -> FlowCfg: + (blk,) = [c for c in cfg.cfgs if c.name == "blk"] + return blk + + +class TestParse: + """Parsing NAME=PATH.""" + + def test_name_and_path(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + assert CfgExtension.parse("blk=ext/x.hjson") == CfgExtension( + "blk", tmp_path.resolve() / "ext" / "x.hjson" + ) + + def test_symlinks_are_kept(self, tmp_path: Path) -> None: + """The path is spelt as {file_dir} spells it, through any symlink.""" + (tmp_path / "real").mkdir() + (tmp_path / "link").symlink_to(tmp_path / "real") + + ext = CfgExtension.parse(f"blk={tmp_path}/link/../link/x.hjson") + + assert ext.path == tmp_path / "link" / "x.hjson" + + @pytest.mark.parametrize("value", ["blk", "=x.hjson", "blk=", " = "]) + def test_rejects_malformed(self, value: str) -> None: + with pytest.raises(ValueError, match="NAME=PATH"): + CfgExtension.parse(value) + + def test_cli_rejects_malformed(self, proj: Path) -> None: + with pytest.raises(SystemExit): + parse_args([str(proj / "hw" / "top_sim_cfgs.hjson"), "--cfg-extend", "blk"]) + + +class TestMerge: + """Merging an extension into its cfg.""" + + @pytest.mark.parametrize("top", ["top_sim_cfgs.hjson", "blk_sim_cfg.hjson"]) + def test_lists_follow_the_cfgs_own(self, proj: Path, tmp_path: Path, top: str) -> None: + """The extension reaches the cfg whether it is a child or run alone. + + Its list entries land after everything the cfg imports, so build + options that need the tool's filelist come after it. + """ + ext = _write( + tmp_path / "overlay" / "ext.hjson", + """{ + build_opts: ["+incdir+/overlay", "/overlay/bind.sv"] + tests: [{name: "t2"}] + }""", + ) + + blk = _blk(_load(proj, top, f"blk={ext}")) + + assert blk.build_opts[:3] == [f"-f {proj}/sv.f", "+incdir+/overlay", "/overlay/bind.sv"] + assert blk.run_opts[:1] == ["+tool"] + assert [t.name for t in blk.tests] == ["t1", "t2"] + assert list(blk.cfg_extensions) == [CfgExtension("blk", ext)] + + def test_imports_and_order(self, proj: Path, tmp_path: Path) -> None: + """Extensions apply in command-line order, each with its own imports.""" + first = _write( + tmp_path / "a" / "ext.hjson", + '{import_cfgs: ["{file_dir}/more.hjson"], run_opts: ["+a"]}', + ) + _write(tmp_path / "a" / "more.hjson", '{run_opts: ["+a_more"]}') + second = _write(tmp_path / "b" / "ext.hjson", '{run_opts: ["+b"]}') + + blk = _blk(_load(proj, "top_sim_cfgs.hjson", f"blk={first}", f"blk={second}")) + + assert blk.run_opts[:4] == ["+tool", "+a", "+a_more", "+b"] + assert list(blk.cfg_extensions) == [CfgExtension("blk", first), CfgExtension("blk", second)] + + def test_named_twice_is_an_error( + self, proj: Path, tmp_path: Path, dvsim_log: pytest.LogCaptureFixture + ) -> None: + """The same NAME=PATH twice would merge the extension twice.""" + ext = _write(tmp_path / "ext.hjson", '{run_opts: ["+x"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}", f"blk={ext}") + + assert f"blk={ext} is given more than once" in dvsim_log.text + + def test_overrides_change_a_scalar(self, proj: Path, tmp_path: Path) -> None: + ext = _write( + tmp_path / "ext.hjson", + '{overrides: [{name: "blk_var", value: "new"}]}', + ) + + blk = _blk(_load(proj, "top_sim_cfgs.hjson", f"blk={ext}")) + + assert blk.blk_var == "new" + + def test_sets_a_scalar_the_cfg_leaves_empty(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{blk_empty: "set"}') + + blk = _blk(_load(proj, "top_sim_cfgs.hjson", f"blk={ext}")) + + assert blk.blk_empty == "set" + + @pytest.mark.parametrize( + ("key", "value"), [("tool", "xcelium"), ("scratch_path", "/elsewhere")] + ) + def test_conflicting_scalar_is_an_error( + self, proj: Path, tmp_path: Path, dvsim_log: pytest.LogCaptureFixture, key: str, value: str + ) -> None: + ext = _write(tmp_path / "ext.hjson", f'{{{key}: "{value}"}}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}") + + assert f"Cannot extend cfg 'blk' with {ext}: Value for key '{key}'" in dvsim_log.text + # The reason doesn't repeat the path. + assert f"'{ext}'" not in dvsim_log.text + + def test_tool_from_the_command_line_wins(self, proj: Path, tmp_path: Path) -> None: + """As in the cfg's own files, the extension's tool gives way to --tool.""" + ext = _write(tmp_path / "ext.hjson", '{tool: "xcelium"}') + + blk = _blk(_load(proj, "top_sim_cfgs.hjson", f"blk={ext}", argv=["--tool", "vcs"])) + + assert blk.tool == "vcs" + + def test_unresolved_import_is_an_error( + self, proj: Path, tmp_path: Path, dvsim_log: pytest.LogCaptureFixture + ) -> None: + ext = _write(tmp_path / "ext.hjson", '{import_cfgs: ["{nope}/x.hjson"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}") + + assert "Can't resolve the name of an imported file" in dvsim_log.text + + def test_use_cfgs_is_an_error(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{use_cfgs: ["/x.hjson"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}") + + def test_missing_file_is_an_error(self, proj: Path, tmp_path: Path) -> None: + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={tmp_path / 'nope.hjson'}") + + +class TestNames: + """Matching an extension to a cfg by name.""" + + def test_unknown_name_is_an_error(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{run_opts: ["+x"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blkk={ext}") + + def test_name_as_the_cfgs_own_files_give_it( + self, proj: Path, tmp_path: Path, dvsim_log: pytest.LogCaptureFixture + ) -> None: + """An extension matches the name before overrides; one that matched nothing is an error. + + Here the cfg's own overrides rename it from 'blk' to 'blk2' after the extensions merge. + """ + cfg = proj / "hw" / "blk_sim_cfg.hjson" + cfg.write_text( + CHILD_CFG.replace("name: blk", 'name: "{n}"\n n: blk').replace( + "testplan:", 'overrides: [{name: "n", value: "blk2"}]\n testplan:' + ) + ) + ext = _write(tmp_path / "ext.hjson", '{run_opts: ["+x"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk2={ext}") + assert "(loaded: blk)" in dvsim_log.text + + (blk2,) = _load(proj, "top_sim_cfgs.hjson", f"blk={ext}").cfgs + assert (blk2.name, blk2.run_opts.count("+x")) == ("blk2", 1) + + def test_primary_name_is_an_error(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{run_opts: ["+x"]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"top={ext}") + + def test_messages(self) -> None: + exts = [CfgExtension("a", Path("/a")), CfgExtension("top", Path("/t"))] + + assert check_cfg_extensions_used(exts, exts[:1], ["a", "b"], "top") == [ + ( + "Cfg extension /t names the primary cfg 'top': only a cfg that runs jobs " + "(a child, or a cfg run on its own) can be extended." + ) + ] + assert check_cfg_extensions_used(exts[:1], [], ["b"], None) == [ + "Cfg extension /a names 'a', but no loaded cfg has that name (loaded: b)." + ] + + +def _reasons(cfg: FlowCfg) -> list[str]: + """Why each skipped extension of cfg was skipped.""" + return [meta.skipped for meta in cfg.cfg_extensions.values() if meta.skipped is not None] + + +def _skipped(cfg: FlowCfg) -> list[CfgExtension]: + """The extensions of cfg that were skipped.""" + return [ext for ext, meta in cfg.cfg_extensions.items() if meta.skipped is not None] + + +def _load_skip(proj: Path, *extends: str) -> FlowCfg: + return _load(proj, "top_sim_cfgs.hjson", *extends, argv=["--on-cfg-extend-error", "skip"]) + + +class TestOnError: + """--on-cfg-extend-error: a broken extension stops the run, or is skipped.""" + + def test_default_is_error(self, proj: Path) -> None: + assert parse_args([str(proj / "hw" / "top_sim_cfgs.hjson")]).on_cfg_extend_error == "error" + + def test_parse_error_is_skipped(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", "{tests: [{name: ") + + blk = _blk(_load_skip(proj, f"blk={ext}")) + + assert [t.name for t in blk.tests] == ["t1"] + # It was matched, and skipped. + assert list(blk.cfg_extensions) == [CfgExtension("blk", ext)] + (reason,) = _reasons(blk) + assert "can't be parsed" in reason + # The reports name the extension beside the reason, so the reason doesn't. + assert str(ext) not in reason + + def test_parse_error_stops_the_run_by_default(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", "{tests: [{name: ") + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}") + + def test_partial_merge_is_undone(self, proj: Path, tmp_path: Path) -> None: + """Keys merged before the one that fails are put back as they were.""" + ext = _write( + tmp_path / "ext.hjson", + '{run_opts: ["+x"], tests: [{name: "t2"}], new_key: "n", blk_var: "clash"}', + ) + + blk = _blk(_load_skip(proj, f"blk={ext}")) + + assert blk.run_opts == ["+tool"] + assert [t.name for t in blk.tests] == ["t1"] + assert not hasattr(blk, "new_key") + assert blk.blk_var == "orig" + (reason,) = _reasons(blk) + assert "conflicting value" in reason + # The reason doesn't repeat the path, which the skip names already. + assert str(ext) not in reason + + @pytest.mark.parametrize( + ("overrides", "message"), + [ + ('[{name: "no_such_key", value: "x"}]', "which the cfg doesn't have"), + ('[{name: "blk_var", value: 3}]', "doesn't match the type"), + ('[{name: "blk_var"}]', "needs a name and a value"), + ('{name: "blk_var", value: "x"}', "Conflicting types"), + ('[{name: ["blk_var"], value: "x"}]', "needs a key name"), + ], + ) + def test_bad_override_is_skipped( + self, proj: Path, tmp_path: Path, overrides: str, message: str + ) -> None: + ext = _write(tmp_path / "ext.hjson", f'{{run_opts: ["+x"], overrides: {overrides}}}') + + blk = _blk(_load_skip(proj, f"blk={ext}")) + + assert blk.run_opts == ["+tool"] + assert getattr(blk, "overrides", []) == [] + (reason,) = _reasons(blk) + assert message in reason + + def test_bad_override_stops_the_run_by_default(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{overrides: [{name: "no_such_key", value: "x"}]}') + + with pytest.raises(SystemExit): + _load(proj, "top_sim_cfgs.hjson", f"blk={ext}") + + def test_override_taken_by_an_earlier_extension(self, proj: Path, tmp_path: Path) -> None: + first = _write(tmp_path / "a.hjson", '{overrides: [{name: "blk_var", value: "a"}]}') + second = _write(tmp_path / "b.hjson", '{overrides: [{name: "blk_var", value: "b"}]}') + + blk = _blk(_load_skip(proj, f"blk={first}", f"blk={second}")) + + assert blk.blk_var == "a" + assert _skipped(blk) == [CfgExtension("blk", second)] + (reason,) = _reasons(blk) + assert "already overridden" in reason + + def test_later_extensions_still_apply(self, proj: Path, tmp_path: Path) -> None: + broken = _write(tmp_path / "a.hjson", '{use_cfgs: ["/x.hjson"]}') + good = _write(tmp_path / "b.hjson", '{run_opts: ["+b"]}') + + blk = _blk(_load_skip(proj, f"blk={broken}", f"blk={good}")) + + assert blk.run_opts == ["+tool", "+b"] + # Both in command-line order; only the broken one skipped. + assert list(blk.cfg_extensions) == [CfgExtension("blk", broken), CfgExtension("blk", good)] + assert _skipped(blk) == [CfgExtension("blk", broken)] + + def test_missing_file_is_skipped(self, proj: Path, tmp_path: Path) -> None: + blk = _blk(_load_skip(proj, f"blk={tmp_path / 'nope.hjson'}")) + + (reason,) = _reasons(blk) + assert "Not a file" in reason + + def test_unresolved_import_is_skipped(self, proj: Path, tmp_path: Path) -> None: + ext = _write(tmp_path / "ext.hjson", '{import_cfgs: ["{nope}/x.hjson"]}') + + blk = _blk(_load_skip(proj, f"blk={ext}")) + + (reason,) = _reasons(blk) + assert "Can't resolve the name of an imported file" in reason + + @pytest.mark.parametrize("name", ["blkk", "top"]) + def test_unknown_name_is_skipped( + self, proj: Path, tmp_path: Path, dvsim_log: pytest.LogCaptureFixture, name: str + ) -> None: + """No cfg matched it, so no report shows it: there is only the warning.""" + ext = _write(tmp_path / "ext.hjson", '{run_opts: ["+x"]}') + + blk = _blk(_load_skip(proj, f"{name}={ext}")) + + assert f"Skipping: Cfg extension {ext} names" in dvsim_log.text + assert not blk.cfg_extensions + + assert blk.run_opts == ["+tool"] + + +class TestRevision: + """The git revision of each extension.""" + + def test_revision_is_recorded(self, proj: Path, tmp_path: Path) -> None: + overlay = tmp_path / "overlay" + ext = _write(overlay / "dv" / "ext.hjson", '{run_opts: ["+x"]}') + git = ["git", "-C", str(overlay)] + subprocess.run([*git, "init", "-q"], check=True) + subprocess.run([*git, "add", "dv"], check=True) + subprocess.run([*git, "commit", "-q", "-m", "init"], check=True) + head = subprocess.run( + [*git, "rev-parse", "HEAD"], check=True, capture_output=True, text=True + ).stdout.strip() + outside = _write(tmp_path / "loose" / "ext.hjson", '{run_opts: ["+y"]}') + + blk = _blk(_load(proj, "top_sim_cfgs.hjson", f"blk={ext}", f"blk={outside}")) + + revision = blk.cfg_extensions[CfgExtension("blk", ext)].revision + assert revision is not None + assert (revision.commit, revision.dirty) == (head, False) + assert blk.cfg_extensions[CfgExtension("blk", outside)].revision is None diff --git a/tests/report/test_extensions.py b/tests/report/test_extensions.py new file mode 100644 index 00000000..8383f381 --- /dev/null +++ b/tests/report/test_extensions.py @@ -0,0 +1,172 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Test how reports show the cfg extensions (--cfg-extend) of a flow.""" + +import json +from datetime import datetime, timezone +from pathlib import Path + +from dvsim.report.data import CfgExtensionMeta, GitRevisionMeta, IPMeta, ToolMeta +from dvsim.sim.data import SimFlowResults, SimResultsSummary +from dvsim.sim.report import HtmlReportRenderer, MarkdownReportRenderer +from dvsim.sim_results import BucketedFailures + +__all__ = () + +TS = datetime(2026, 1, 2, 3, 4, 5, tzinfo=timezone.utc) + + +def _results(*extensions: CfgExtensionMeta) -> SimFlowResults: + return SimFlowResults( + block=IPMeta( + name="blk", + commit="c" * 40, + commit_short="c" * 7, + branch="b", + url=None, + revision_info=None, + ), + tool=ToolMeta(name="vcs", version="1"), + timestamp=TS, + build_seed=None, + testplan_ref=None, + stages={}, + coverage=None, + cov_report_page=None, + vplan_report_page=None, + failed_jobs=BucketedFailures(buckets={}), + passed=1, + total=1, + percent=100.0, + extensions=extensions, + ) + + +def _summary(results: SimFlowResults) -> SimResultsSummary: + return SimResultsSummary( + top=None, + version=None, + timestamp=TS, + build_seed=None, + flow_results={"blk": results.summary()}, + report_path=Path("/reports"), + ) + + +NO_GIT = CfgExtensionMeta(path="/overlay/ok.hjson") +SKIPPED = CfgExtensionMeta(path="/overlay/broken.hjson", skipped="Key 'x' not found.") + + +class TestExtensionsInReports: + """The extensions, merged or skipped, in the results and reports.""" + + def test_summary_keeps_extensions(self) -> None: + assert _results(NO_GIT, SKIPPED).summary().extensions == (NO_GIT, SKIPPED) + + def test_json_round_trip(self) -> None: + results = _results(NO_GIT, SKIPPED) + assert SimFlowResults.model_validate_json(results.model_dump_json()) == results + + def test_markdown_names_skipped_extensions(self) -> None: + results = _results(NO_GIT, SKIPPED) + renderer = MarkdownReportRenderer() + + block = renderer.render_block(results)["report.md"] + summary = renderer.render_summary(_summary(results))["report.md"] + + assert ( + "**Warning:** cfg extension `/overlay/broken.hjson` skipped: Key 'x' not found." + in block + ) + # Merged extensions are listed, without a warning. + assert "* `/overlay/ok.hjson`" in block + assert "`/overlay/ok.hjson` skipped" not in block + assert "BLK (1 extension skipped)" in summary + + def test_markdown_without_skipped_extensions(self) -> None: + results = _results(NO_GIT) + renderer = MarkdownReportRenderer() + + assert "skipped" not in renderer.render_block(results)["report.md"] + assert "skipped" not in renderer.render_summary(_summary(results))["report.md"] + + def test_html_names_skipped_extensions(self) -> None: + results = _results(NO_GIT, SKIPPED) + + pages = HtmlReportRenderer().render(_summary(results), {"blk": results}) + + assert ( + "/overlay/broken.hjson skipped: Key 'x' not found." in pages["blk.html"] + ) + assert "1 extension skipped" in pages["index.html"] + + def test_html_without_skipped_extensions(self) -> None: + results = _results(NO_GIT) + + pages = HtmlReportRenderer().render(_summary(results), {"blk": results}) + + assert "skipped" not in pages["blk.html"] + assert "extension skipped" not in pages["index.html"] + + +TRACKED = CfgExtensionMeta( + path="/overlay/dv/ext.hjson", + revision=GitRevisionMeta( + origin="git@example.com:org/overlay.git", + commit="a" * 40, + commit_short="aaaaaaa", + dirty=True, + ), +) + + +class TestExtensionRevisions: + """The git revision of each extension in the reports.""" + + def test_json_without_extensions_leaves_out_the_key(self) -> None: + results = _results() + + assert "extensions" not in json.loads(results.model_dump_json()) + flow = json.loads(_summary(results).model_dump_json())["flow_results"]["blk"] + assert "extensions" not in flow + assert SimFlowResults.model_validate_json(results.model_dump_json()) == results + + def test_json_with_extensions_has_the_key(self) -> None: + results = _results(NO_GIT) + + assert json.loads(results.model_dump_json())["extensions"] == [ + {"path": "/overlay/ok.hjson", "skipped": None, "revision": None} + ] + flow = json.loads(_summary(results).model_dump_json())["flow_results"]["blk"] + assert flow["extensions"] == [ + {"path": "/overlay/ok.hjson", "skipped": None, "revision": None} + ] + + def test_markdown_gives_the_revision(self) -> None: + block = MarkdownReportRenderer().render_block(_results(TRACKED, NO_GIT))["report.md"] + + assert ( + "### Cfg extensions:\n\n* `/overlay/dv/ext.hjson` (revision `aaaaaaa`, dirty, " + "from git@example.com:org/overlay.git)\n* `/overlay/ok.hjson`" + ) in block + + def test_skipped_gives_the_revision(self) -> None: + skipped = TRACKED.model_copy(update={"skipped": "Not a file."}) + + block = MarkdownReportRenderer().render_block(_results(skipped))["report.md"] + page = HtmlReportRenderer().render(_summary(_results(skipped)), {"blk": _results(skipped)}) + + assert "(revision `aaaaaaa`, dirty, from git@example.com:org/overlay.git) skipped" in block + assert "### Cfg extensions" not in block + assert "(sha: aaaaaaa, dirty)" in page["blk.html"] + + def test_html_gives_the_revision(self) -> None: + results = _results(TRACKED) + + page = HtmlReportRenderer().render(_summary(results), {"blk": results})["blk.html"] + + assert "Extension: ext.hjson sha: aaaaaaa (dirty)" in page + assert "git@example.com:org/overlay.git" in page + assert "a" * 40 in page diff --git a/tests/utils/test_git.py b/tests/utils/test_git.py index 5516fe22..77386ecf 100644 --- a/tests/utils/test_git.py +++ b/tests/utils/test_git.py @@ -10,11 +10,13 @@ from git import Repo from hamcrest import assert_that, calling, equal_to, raises +from dvsim.report.data import GitRevisionMeta from dvsim.utils.git import ( git_commit_hash, git_https_url_with_commit, git_is_dirty, git_origin_url, + git_revision, repo_root, ) @@ -226,3 +228,50 @@ def test_git_https_url_with_commit(tmp_path: Path, url: str, expected: str) -> N git_https_url_with_commit(tmp_path), equal_to(expected.format(commit=commit)), ) + + @staticmethod + def test_git_revision(tmp_path: Path) -> None: + """Test git_revision describes the checkout holding a file.""" + ext = tmp_path / "sub" / "ext.hjson" + ext.parent.mkdir() + ext.write_text("{}") + + # Not in a git repo, or in one without a commit: nothing to say. + assert_that(git_revision(ext), equal_to(None)) + r = Repo.init(path=tmp_path) + assert_that(git_revision(ext), equal_to(None)) + + r.index.add([ext]) + r.index.commit("initial commit") + commit = r.head.commit.hexsha + + assert_that( + git_revision(ext), + equal_to( + GitRevisionMeta( + origin=None, + commit=commit, + commit_short=git_commit_hash(tmp_path, short=True), + dirty=False, + ) + ), + ) + + # An untracked or ignored file isn't in the commit, so its checkout + # counts as dirty; the other files there are still clean. + untracked = tmp_path / "sub" / "new.hjson" + untracked.write_text("{}") + ignored = tmp_path / "sub" / "ignored.hjson" + ignored.write_text("{}") + (tmp_path / ".git" / "info" / "exclude").write_text("ignored.hjson\n") + assert_that(git_revision(ext).dirty, equal_to(False)) + assert_that(git_revision(untracked).dirty, equal_to(True)) + assert_that(git_revision(ignored).dirty, equal_to(True)) + untracked.unlink() + + r.create_remote("origin", "https://oauth2:token@example.com/org/overlay.git") + ext.write_text("{x: 1}") + revision = git_revision(ext) + assert revision is not None + assert_that(revision.origin, equal_to("https://example.com/org/overlay.git")) + assert_that(revision.dirty, equal_to(True))