diff --git a/doc/design_doc.md b/doc/design_doc.md index 7865918c..d8bd7e8d 100644 --- a/doc/design_doc.md +++ b/doc/design_doc.md @@ -149,6 +149,27 @@ 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. +The sim reports and JSON results list the extensions that each configuration merged, 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**. @@ -218,6 +239,20 @@ A version of the generated report (in Markdown format) is logged to the console. A slightly longer version of the generated report is written to an HTML file. The HTML report is optionally published to the report server. +A test can be marked as expected to fail, giving the reason: + +```hjson +tests: [ + {name: "foo_smoke", xfail: "foo doesn't support bar yet (issue #123)"} +] +``` + +A run of such a test that fails is reported as XFAIL: it has its own buckets and count, and doesn't fail the invocation. +It still counts in the total that the pass rate divides by, so the pass rate is the share of runs that passed. +A run that passes is reported as XPASS and counts as a failure (strict), so the list of expected failures stays current. +Neither counts as passed. +A run that times out, can't be launched, or is cancelled by the scheduler is a failure as usual, not an XFAIL. + # Design ## Language diff --git a/src/dvsim/cli/run.py b/src/dvsim/cli/run.py index 60d36877..63eae362 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,21 @@ 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." + ), + ) + 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..51aad8ef 100644 --- a/src/dvsim/flow/base.py +++ b/src/dvsim/flow/base.py @@ -17,11 +17,13 @@ import hjson import dvsim.instrumentation.runtime as instrumentation +from dvsim.flow.extension import CfgExtension, load_cfg_extension 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 +35,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 +149,15 @@ 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) merged into this cfg, in order, as + # the reports describe them, 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 +215,41 @@ 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) + try: + ext_data = load_cfg_extension(ext, initial_values, arg_keys) + for key, value in ext_data.items(): + set_target_attribute(str(ext.path), self.__dict__, key, value) + except RuntimeError as err: + log.error("Cannot extend cfg %r: %s", name, err) + sys.exit(1) + self.cfg_extensions[ext] = CfgExtensionMeta( + path=str(ext.path), revision=git_revision(ext.path) + ) + 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..dba1ecc6 --- /dev/null +++ b/src/dvsim/flow/extension.py @@ -0,0 +1,113 @@ +# 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_extensions_used", + "load_cfg_extension", +) + + +@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 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 diff --git a/src/dvsim/flow/factory.py b/src/dvsim/flow/factory.py index 166db158..be76401c 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,19 @@ 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. + 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, + ) + for error in errors: + log.error("%s", error) + if errors: + 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/job/data.py b/src/dvsim/job/data.py index bb269bd3..6e14bfeb 100644 --- a/src/dvsim/job/data.py +++ b/src/dvsim/job/data.py @@ -23,6 +23,8 @@ "JobSpec", "JobStatusInfo", "WorkspaceConfig", + "is_xfail", + "is_xpass", ) @@ -131,6 +133,9 @@ class JobSpec(BaseModel): fail_patterns: Sequence[str] """regex patterns to match on to determine if the job has failed.""" + xfail: str | None = None + """Why the job is expected to fail, or None if it is expected to pass.""" + @property def id(self) -> str: """Returns a string that uniquely identifies this job.""" @@ -154,6 +159,8 @@ class JobStatusInfo(BaseModel): """Relevant line information (in the job script or the job itself).""" context: Sequence[str] | None = None """Arbitrary context strings.""" + cancelled: bool = False + """The scheduler stopped the job for a reason outside it (a failed dependency or a signal).""" class CompletedJobStatus(BaseModel): @@ -201,3 +208,25 @@ class CompletedJobStatus(BaseModel): """Status of the job.""" fail_msg: JobStatusInfo | None """Error message.""" + + xfail: str | None = None + """Why the job is expected to fail, or None if it is expected to pass.""" + + +def is_xfail(xfail: str | None, status: JobStatus, info: JobStatusInfo | None) -> bool: + """Whether a job that is expected to fail failed (XFAIL). + + Only a job that ran and failed counts. A killed job (a timeout, or a job that could not be + launched) and a job the scheduler cancelled are failures, so that a hang or a broken + environment doesn't pass for the expected failure. + """ + return ( + xfail is not None + and status == JobStatus.FAILED + and not (info is not None and info.cancelled) + ) + + +def is_xpass(xfail: str | None, status: JobStatus) -> bool: + """Whether a job that is expected to fail passed (XPASS), which counts as a failure.""" + return xfail is not None and status == JobStatus.PASSED diff --git a/src/dvsim/job/deploy.py b/src/dvsim/job/deploy.py index 02146e00..f239c023 100644 --- a/src/dvsim/job/deploy.py +++ b/src/dvsim/job/deploy.py @@ -93,6 +93,9 @@ def __init__(self, sim_cfg: "FlowCfg") -> None: # this current job to run self.needs_all_dependencies_passing = True + # Why the job is expected to fail, if it is (only tests can be). + self.xfail: str | None = None + # These variables will be extracted from the hjson file by _set_attrs, # and then _check_attrs checks that they were indeed extracted. Define # placeholder values here to allow a type checker to know the class has @@ -183,6 +186,7 @@ def get_job_spec(self) -> "JobSpec": post_finish=self.post_finish(), pass_patterns=self.pass_patterns, fail_patterns=self.fail_patterns, + xfail=self.xfail, ) def _define_attrs(self) -> None: @@ -763,6 +767,8 @@ def _set_attrs(self) -> None: # When running a test, we should always renew the output directory self.renew_odir = True + self.xfail = self.test_obj.xfail or None + # 'test' is used as a substitution variable in the HJson. self.test = self.name self.build_mode = self.test_obj.build_mode.name diff --git a/src/dvsim/report/data.py b/src/dvsim/report/data.py index e385d204..08aadd0f 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,35 @@ class ToolMeta(BaseModel): """Name of the tool.""" version: str """Version of the tool.""" + + +class CfgExtensionMeta(BaseModel): + """A cfg extension (--cfg-extend) merged into a flow's cfg.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + path: str + """Path of the extension file.""" + 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/scheduler/core.py b/src/dvsim/scheduler/core.py index 026ddd0e..2eaa4086 100644 --- a/src/dvsim/scheduler/core.py +++ b/src/dvsim/scheduler/core.py @@ -13,7 +13,7 @@ from types import FrameType from typing import Any, TypeAlias -from dvsim.job.data import CompletedJobStatus, JobSpec, JobStatusInfo +from dvsim.job.data import CompletedJobStatus, JobSpec, JobStatusInfo, is_xfail, is_xpass from dvsim.job.status import JobStatus from dvsim.logging import log from dvsim.runtime.backend import RuntimeBackend @@ -69,20 +69,28 @@ class JobRecord: # Standard context messages used for killed/failed jobs in the scheduler. FAILED_DEP = JobStatusInfo( - message="Job cancelled because one of its dependencies failed or was killed." + message="Job cancelled because one of its dependencies failed or was killed.", + cancelled=True, ) ALL_FAILED_DEP = JobStatusInfo( - message="Job cancelled because all of its dependencies failed or were killed." + message="Job cancelled because all of its dependencies failed or were killed.", + cancelled=True, ) KILLED_SCHEDULED = JobStatusInfo( - message="Job cancelled because one of its dependencies was killed." + message="Job cancelled because one of its dependencies was killed.", + cancelled=True, +) +KILLED_QUEUED = JobStatusInfo( + message="Job killed whilst waiting to begin execution.", + cancelled=True, ) -KILLED_QUEUED = JobStatusInfo(message="Job killed whilst waiting to begin execution.") KILLED_RUNNING_SIGINT = JobStatusInfo( - message="Job killed by a SIGINT signal to the scheduler whilst executing." + message="Job killed by a SIGINT signal to the scheduler whilst executing.", + cancelled=True, ) KILLED_RUNNING_SIGTERM = JobStatusInfo( - message="Job killed by a SIGTERM signal to the scheduler whilst executing." + message="Job killed by a SIGTERM signal to the scheduler whilst executing.", + cancelled=True, ) @@ -270,12 +278,23 @@ def _change_job_status( job.status_info = info if new_status != JobStatus.RUNNING: + level = log.VERBOSE + expected = "" + if is_xfail(job.spec.xfail, new_status, info): + level = log.INFO + expected = f" (expected to fail: {job.spec.xfail})" + elif is_xpass(job.spec.xfail, new_status): + level = log.WARNING + expected = f" (but expected to fail: {job.spec.xfail})" + elif new_status in (JobStatus.FAILED, JobStatus.KILLED): + level = log.ERROR log.log( - log.ERROR if new_status in (JobStatus.FAILED, JobStatus.KILLED) else log.VERBOSE, - "Status change to [%s: %s] for %s", + level, + "Status change to [%s: %s] for %s%s", new_status.shorthand, new_status.name.capitalize(), job.spec.full_name, + expected, ) for cb in self._on_job_status_change: @@ -405,6 +424,7 @@ async def run(self) -> list[CompletedJobStatus]: else 0.0, status=job.status, fail_msg=job.status_info, + xfail=job.spec.xfail, ) for job in self._jobs.values() ] diff --git a/src/dvsim/sim/data.py b/src/dvsim/sim/data.py index 888dfad1..0b35f3d4 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__ = ( @@ -41,6 +42,17 @@ class TestResult(BaseModel): percent: float """Percentage test pass rate.""" + xfailed: int = 0 + """Number of runs that failed, as expected (XFAIL). Not counted as passed.""" + xpassed: int = 0 + """Number of runs that passed, but were expected to fail (XPASS). Counted as failures.""" + xfail: str | None = None + """Why the test is expected to fail, or None if it is expected to pass.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_unused_xfail(handler(self)) + class Testpoint(BaseModel): """Testpoint.""" @@ -57,6 +69,15 @@ class Testpoint(BaseModel): percent: float """Percentage test pass rate.""" + xfailed: int = 0 + """Number of runs that failed, as expected (XFAIL). Not counted as passed.""" + xpassed: int = 0 + """Number of runs that passed, but were expected to fail (XPASS). Counted as failures.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_unused_xfail(handler(self)) + class TestStage(BaseModel): """Test stages.""" @@ -73,6 +94,15 @@ class TestStage(BaseModel): percent: float """Percentage test pass rate.""" + xfailed: int = 0 + """Number of runs that failed, as expected (XFAIL). Not counted as passed.""" + xpassed: int = 0 + """Number of runs that passed, but were expected to fail (XPASS). Counted as failures.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_unused_xfail(handler(self)) + class CodeCoverageMetrics(BaseModel): """CodeCoverage metrics.""" @@ -169,6 +199,33 @@ 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 + + +def _without_unused_xfail(data: dict[str, Any]) -> dict[str, Any]: + """Leave out the xfail keys of results that have no xfail test. + + Results without xfail tests then have the keys they had before xfail was added, so a reader + built for those (whose models forbid unknown keys) can still load them. + """ + for key in ("xfailed", "xpassed"): + if data.get(key) == 0: + data.pop(key) + if "xfail" in data and data["xfail"] is None: + data.pop("xfail") + if "xfailed_jobs" in data and not data["xfailed_jobs"].get("buckets"): + data.pop("xfailed_jobs") + return data + + class SimFlowResults(BaseModel): """Flow results data.""" @@ -207,6 +264,21 @@ class SimFlowResults(BaseModel): percent: float """Percentage test pass rate.""" + extensions: tuple[CfgExtensionMeta, ...] = () + """The cfg extensions (--cfg-extend) named for this flow.""" + + xfailed: int = 0 + """Number of runs that failed, as expected (XFAIL). Not counted as passed.""" + xpassed: int = 0 + """Number of runs that passed, but were expected to fail (XPASS). Counted as failures.""" + + xfailed_jobs: BucketedFailures = BucketedFailures(buckets={}) + """Bucketed overview of the expected failures (XFAIL).""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_unused_xfail(_without_empty_extensions(handler(self))) + def summary(self) -> "SimFlowSummary": """Load results from JSON file. @@ -247,6 +319,18 @@ class SimFlowSummary(BaseModel): percent: float """Percentage test pass rate.""" + extensions: tuple[CfgExtensionMeta, ...] = () + """The cfg extensions (--cfg-extend) named for this flow.""" + + xfailed: int = 0 + """Number of runs that failed, as expected (XFAIL). Not counted as passed.""" + xpassed: int = 0 + """Number of runs that passed, but were expected to fail (XPASS). Counted as failures.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + return _without_unused_xfail(_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..8f4c13d5 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -8,7 +8,7 @@ import random import sys from collections import OrderedDict, defaultdict -from collections.abc import Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from datetime import datetime, timezone from importlib.metadata import PackageNotFoundError, version from pathlib import Path @@ -55,6 +55,15 @@ _MAX_TEST_RESEEDS = 2 +def _xfail_counts(results: Iterable[TestResult]) -> dict[str, int]: + """Sum the XFAIL and XPASS counts of some test results.""" + results = list(results) + return { + "xfailed": sum(r.xfailed for r in results), + "xpassed": sum(r.xpassed for r in results), + } + + class SimCfg(FlowCfg): """Simulation configuration object. @@ -779,6 +788,9 @@ def make_test_result(tr) -> TestResult | None: passed=tr.passing, total=tr.total, percent=100.0 * tr.passing / (tr.total or 1), + xfailed=tr.xfailed, + xpassed=tr.xpassed, + xfail=tr.xfail, ) # 1. Mapped testpoints — only include if at least one test ran @@ -804,6 +816,7 @@ def make_test_result(tr) -> TestResult | None: passed=tp_passed, total=tp_total, percent=100.0 * tp_passed / tp_total if tp_total else 0.0, + **_xfail_counts(test_results.values()), ) for name, tr in test_results.items(): @@ -824,6 +837,7 @@ def make_test_result(tr) -> TestResult | None: passed=tp_passed, total=tp_total, percent=100.0 * tp_passed / tp_total if tp_total else 0.0, + **_xfail_counts(unmapped_tests.values()), ) for name, tr in unmapped_tests.items(): @@ -842,6 +856,7 @@ def make_test_result(tr) -> TestResult | None: passed=stage_passed, total=stage_total, percent=100.0 * stage_passed / stage_total if stage_total else 0.0, + **_xfail_counts(stage_to_trs[stage_name].values()), ) total_passed = sum(tr.passed for tr in all_trs.values()) @@ -874,9 +889,12 @@ def make_test_result(tr) -> TestResult | None: vplan_coverage = self.cov_vplan_deploy.vplan_coverage failures = BucketedFailures.from_job_status(results=run_results) + expected_failures = BucketedFailures.from_job_status(results=run_results, expected=True) if failures.buckets: self.errors_seen = True + extensions = self.cfg_extensions.values() + # --- Final result --- return SimFlowResults( block=block, @@ -893,6 +911,9 @@ 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), + **_xfail_counts(all_trs.values()), + xfailed_jobs=expected_failures, ) def _fake_policy(self, job: JobSpec) -> JobStatus: diff --git a/src/dvsim/sim/report.py b/src/dvsim/sim/report.py index bdda6311..e94083c4 100644 --- a/src/dvsim/sim/report.py +++ b/src/dvsim/sim/report.py @@ -14,8 +14,9 @@ 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.sim_results import BucketedFailures from dvsim.templates.render import render_template from dvsim.utils import TS_FORMAT_LONG from dvsim.utils.fs import relative_to @@ -35,6 +36,14 @@ def _plural(item: str, n: int | Collection[Any], suffix: str = "s") -> str: return item if n == 1 else item + suffix +def _xfail_cell(xfailed: int, xpassed: int) -> str: + """Describe the XFAIL and XPASS runs of a test or group, for a table cell.""" + cell = str(xfailed) if xfailed else "" + if xpassed: + cell += f" ({xpassed} XPASS)" + return cell.strip() + + def _indent_by_levels(lines: Iterable[tuple[int, str]], indent_spaces: int = 4) -> str: """Format per-line indentation of (0-indexed level, msg) log messages.""" return "\n".join(" " * lvl * indent_spaces + msg for lvl, msg in lines) @@ -114,11 +123,17 @@ def render( sorted_buckets = dict( sorted(results.failed_jobs.buckets.items(), key=lambda kv: len(kv[1]), reverse=True) ) + sorted_xfail_buckets = dict( + sorted( + results.xfailed_jobs.buckets.items(), key=lambda kv: len(kv[1]), reverse=True + ) + ) artifacts[block_file] = render_template( path="reports/block_report.html", data={ "results": results, "failed_jobs": sorted_buckets, + "xfailed_jobs": sorted_xfail_buckets, "version": summary.version, }, ) @@ -159,6 +174,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 +238,9 @@ 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()}" + if results.extensions: + report_md += "\n### Cfg extensions:\n" + report_md += "".join(f"\n* `{ext.path}`{_revision(ext)}" for ext in results.extensions) # Record a summary of the simulation results, coverage, and failure buckets, if applicable. result_summary = self.render_block_results(results) @@ -272,7 +300,14 @@ def render_block_results(self, results: SimFlowResults) -> str: report_md += "\n\n" + coverage_table if results.failed_jobs.buckets: - bucket_summary = self.render_bucket_summary(results) + bucket_summary = self.render_bucket_summary(results.failed_jobs) + if bucket_summary: + report_md += "\n\n" + bucket_summary + + if results.xfailed_jobs.buckets: + bucket_summary = self.render_bucket_summary( + results.xfailed_jobs, title="Expected Failures", noun="expected failure" + ) if bucket_summary: report_md += "\n\n" + bucket_summary @@ -290,6 +325,14 @@ def render_result_table(self, results: SimFlowResults) -> str: ("Total", "center"), ("Pass Rate", "center"), ] + show_xfail = any( + t.xfail + for stage in results.stages.values() + for tp in stage.testpoints.values() + for t in tp.tests.values() + ) + if show_xfail: + column_info.append(("XFail", "center")) table = [] hidden_names = ("n.a.", "unmapped") @@ -314,19 +357,23 @@ def render_result_table(self, results: SimFlowResults) -> str: result.total, pass_rate, ] + if show_xfail: + row.append(_xfail_cell(result.xfailed, result.xpassed)) table.append(row) pass_rate = "-- %" if stage.total == 0 else f"{stage.percent:.2f} %" # TODO: note the calculated stage totals are currently not correct. - table.append( - [stage_name, None, "**TOTAL**", None, None, stage.passed, stage.total, pass_rate] - ) + row = [stage_name, None, "**TOTAL**", None, None, stage.passed, stage.total, pass_rate] + if show_xfail: + row.append(_xfail_cell(stage.xfailed, stage.xpassed)) + table.append(row) # TODO: note the calculated overall totals are currently not correct. pass_rate = "-- %" if results.total == 0 else f"{results.percent:.2f} %" - table.append( - [None, None, "**TOTAL**", None, None, results.passed, results.total, pass_rate] - ) + row = [None, None, "**TOTAL**", None, None, results.passed, results.total, pass_rate] + if show_xfail: + row.append(_xfail_cell(results.xfailed, results.xpassed)) + table.append(row) if not table: return "" @@ -366,16 +413,21 @@ def render_coverage_table(self, results: SimFlowResults) -> str: return report_md - def render_bucket_summary(self, results: SimFlowResults) -> str: + def render_bucket_summary( + self, + failures: BucketedFailures, + title: str = "Failure Buckets", + noun: str = "failure", + ) -> str: """Generate a Markdown string with a summary of the buckets (failures/killed).""" - lines = [(0, "## Failure Buckets")] + lines = [(0, f"## {title}")] for bucket, tests in sorted( - results.failed_jobs.buckets.items(), + failures.buckets.items(), key=lambda kv: len(kv[1]), reverse=True, ): - lines.append((0, f"* `{bucket}` has {len(tests)} {_plural('failure', tests)}:")) + lines.append((0, f"* `{bucket}` has {len(tests)} {_plural(noun, tests)}:")) grouped_tests = defaultdict(list) for job in tests: @@ -383,9 +435,7 @@ def render_bucket_summary(self, results: SimFlowResults) -> str: displayed = list(grouped_tests.items())[: self.MAX_TESTS_PER_BUCKET] for name, reseeds in displayed: - lines.append( - (1, f"* Test {name} has {len(reseeds)} {_plural('failure', reseeds)}.") - ) + lines.append((1, f"* Test {name} has {len(reseeds)} {_plural(noun, reseeds)}.")) for failure in reseeds[: self.MAX_RESEEDS_PER_BUCKETED_TEST]: lines.append((2, f"* {failure.qual_name}\\")) @@ -398,7 +448,7 @@ def render_bucket_summary(self, results: SimFlowResults) -> str: extra = len(reseeds) - self.MAX_RESEEDS_PER_BUCKETED_TEST if extra > 0: - lines.append((2, f"* ... and {extra} more {_plural('failure', extra)}.")) + lines.append((2, f"* ... and {extra} more {_plural(noun, extra)}.")) extra = len(grouped_tests) - self.MAX_TESTS_PER_BUCKET if extra > 0: @@ -447,6 +497,13 @@ def render_summary(self, summary: SimResultsSummary) -> ReportArtifacts: "Coverage": coverage, } ) + if flow_result.xfailed or flow_result.xpassed: + table[-1]["XFail"] = _xfail_cell(flow_result.xfailed, flow_result.xpassed) + + # Every row needs every column. + if any("XFail" in row for row in table): + for row in table: + row.setdefault("XFail", "") if table: colalign = ("center",) * len(table[0]) diff --git a/src/dvsim/sim_results.py b/src/dvsim/sim_results.py index c4d8e971..41526812 100644 --- a/src/dvsim/sim_results.py +++ b/src/dvsim/sim_results.py @@ -11,11 +11,12 @@ from pydantic import BaseModel, ConfigDict +from dvsim.job.data import is_xfail, is_xpass from dvsim.job.status import JobStatus from dvsim.testplan import Result if TYPE_CHECKING: - from dvsim.job.data import CompletedJobStatus + from dvsim.job.data import CompletedJobStatus, JobStatusInfo __all__ = () @@ -127,37 +128,53 @@ class BucketedFailures(BaseModel): """Mapping of common error message strings to the full job failure summary.""" @staticmethod - def from_job_status(results: Sequence["CompletedJobStatus"]) -> "BucketedFailures": - """Construct from CompletedJobStatus objects.""" + def from_job_status( + results: Sequence["CompletedJobStatus"], *, expected: bool = False + ) -> "BucketedFailures": + """Construct from CompletedJobStatus objects. + + By default, bucket the unexpected failures: failed or killed jobs that aren't + expected to fail, and runs of xfail tests that passed (XPASS). With expected=True, + bucket the expected failures (XFAIL) instead. + """ buckets = {} + def add( + bucket: str, job_status: "CompletedJobStatus", info: "JobStatusInfo | None" + ) -> None: + # TODO: expose all relevant line numbers through the bucket, not just the first. + # We only expose the first one for now to keep changes to the scheduler minimal. + first_line_num = None + if info is not None and info.lines: + lines = info.lines[0] + first_line_num = lines if isinstance(lines, int) else lines[0] + + buckets.setdefault(bucket, []).append( + JobFailureOverview( + name=job_status.name, + qual_name=job_status.qual_name, + seed=job_status.seed, + line=first_line_num, + log_path=job_status.log_path, + log_context=(info.context or []) if info is not None else [], + ), + ) + for job_status in results: - if job_status.status in (JobStatus.FAILED, JobStatus.KILLED): - if job_status.fail_msg is None: - continue - - bucket = _bucketize(job_status.fail_msg.message) - - if bucket not in buckets: - buckets[bucket] = [] - - # TODO: expose all relevant line numbers through the bucket, not just the first. - # We only expose the first one for now to keep changes to the scheduler minimal. - first_line_num = None - if job_status.fail_msg.lines: - lines = job_status.fail_msg.lines[0] - first_line_num = lines if isinstance(lines, int) else lines[0] - - buckets[bucket].append( - JobFailureOverview( - name=job_status.name, - qual_name=job_status.qual_name, - seed=job_status.seed, - line=first_line_num, - log_path=job_status.log_path, - log_context=job_status.fail_msg.context or [], - ), - ) + if is_xpass(job_status.xfail, job_status.status): + if not expected: + add(f"Passed, but expected to fail: {job_status.xfail}", job_status, None) + continue + + if job_status.status not in (JobStatus.FAILED, JobStatus.KILLED): + continue + if ( + job_status.fail_msg is None + or is_xfail(job_status.xfail, job_status.status, job_status.fail_msg) != expected + ): + continue + + add(_bucketize(job_status.fail_msg.message), job_status, job_status.fail_msg) return BucketedFailures( buckets=buckets, @@ -206,6 +223,13 @@ def _add_run(self, job_status: "CompletedJobStatus") -> None: row.job_runtime = job_status.job_runtime row.simulated_time = job_status.simulated_time - if job_status.status == JobStatus.PASSED: + if job_status.xfail is not None: + row.xfail = job_status.xfail + if is_xpass(job_status.xfail, job_status.status): + row.xpassed += 1 + elif is_xfail(job_status.xfail, job_status.status, job_status.fail_msg): + # An expected failure counts in total, but not in passing. + row.xfailed += 1 + elif job_status.status == JobStatus.PASSED: row.passing += 1 row.total += 1 diff --git a/src/dvsim/templates/reports/block_report.html b/src/dvsim/templates/reports/block_report.html index 4f6177fd..1a984bb6 100644 --- a/src/dvsim/templates/reports/block_report.html +++ b/src/dvsim/templates/reports/block_report.html @@ -12,6 +12,45 @@ {% set stages = results.stages %} {% set coverage = results.coverage %} {% block content %} +{% macro bucket_table(title, buckets) %} +
| + | Test | +seed | +line | +log context | +
|---|---|---|---|---|
| {{ msg }} | +{{ job_list|length }} test run{{ "s" if job_list|length > 1 else "" }} | +|||
| + | {{ job.name }} | +{{ job.seed }} | +{{ job.line }} | +
+ {% for context_line in job.log_context %}
+ {{ context_line }}
+ {% endfor %}
+ |
+
| - | Test | -seed | -line | -log context | -
|---|---|---|---|---|
| {{ msg }} | -{{ job_list|length }} test run{{ "s" if job_list|length > 1 else "" }} | -|||
| - | {{ job.name }} | -{{ job.seed }} | -{{ job.line }} | -
- {% for context_line in job.log_context %}
- {{ context_line }}
- {% endfor %}
- |
-