diff --git a/README.md b/README.md index 22a6ce5f..8bb4f579 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,7 @@ You can access it [online at opentitan.org/book/](https://opentitan.org/book/). * [Testplanner tool](./doc/testplanner.md) * [Design document](./doc/design_doc.md) +* [The `lowrisc-dv-evidence` format](./doc/dv_evidence.md) * [Glossary](./doc/glossary.md) ## How to contribute diff --git a/doc/dv_evidence.md b/doc/dv_evidence.md new file mode 100644 index 00000000..d30da1a1 --- /dev/null +++ b/doc/dv_evidence.md @@ -0,0 +1,94 @@ + +# The `lowrisc-dv-evidence` format + +A regression tells you which tests passed. +A verification plan asks a different question: of everything we said we would verify, how much is now backed by something that ran? +Answering it needs the regression's own outcomes in a form a planning tool can read, rather than a log directory and a human. + +This is that form. +DVSim writes one of these files per simulation flow, and this page specifies what any producer has to write rather than describing one tool's output. +Anything that can produce it can be scored against a verification plan, whether or not it is DVSim. + +## Where DVSim writes it + +`/cov_vplan/dv_evidence.json`, produced by the `cov_vplan` job, and only when the sim config names a `vplan`. +It is written before the annotation step runs and is archived alongside the reports, so it outlives the scratch area it describes. + +## Shape + +```json +{ + "schema": "lowrisc-dv-evidence", + "dut": "hmac", + "tool": "xcelium", + "produced_by": "dvsim 1.50.1", + "revision": "https://github.com/lowRISC/opentitan/tree/a1b2c3d (dirty)", + "timestamp": "2026-08-18T09:00:00+00:00", + "testcase": { + "hmac_smoke": [ + { "status": "passed", "seed": 1234, "log": "/scratch/hmac/1234.hmac_smoke/run.log" }, + { "status": "failed", "seed": 5678, "log": "...", "message": "UVM_ERROR", "line": 812 } + ], + "hmac_stress_all": [ + { "status": "not_run" } + ] + } +} +``` + +Fields are omitted when they have no value rather than written as `null`. + +### Top level + +| Key | Meaning | +| --- | --- | +| `schema` | Always `lowrisc-dv-evidence`. Identifies the format to whatever reads the file. | +| `testcase` | Test name to the list of runs of that test. The only required key. | +| `dut` | The design the results are about, named as a verification plan addresses it. | +| `tool` | The simulator that produced them. | +| `produced_by` | What wrote the file, with its version. | +| `revision` | The tree the results were produced against, suffixed ` (dirty)` when it was not clean. | +| `timestamp` | When the run started, as an ISO 8601 datetime with an offset. | + +### A run + +Every entry under `testcase` is keyed by the test name, because that is the name a plan refers to. +Reseeds of one test share the key and are told apart by `seed`. + +| Key | Meaning | +| --- | --- | +| `status` | One of `passed`, `failed`, `killed`, `not_run`. Required. | +| `seed` | The seed the run used, where the flow randomises. | +| `log` | Path to the run's log. | +| `message` | Why it ended that way. Present only on a run that did not pass. | +| `line` | The log line the failure was first reported at. | + +`killed` and `not_run` are separate on purpose. +A killed test started and was terminated, so the design was exercised and something went wrong. +A `not_run` test never started, because the scheduler cancelled it once a dependency failed or the run was shut down. +The two are different answers to "did we verify this", and collapsing them would let a build failure read as a passing plan item. + +There is no `waived` status. +A waiver needs an owner and a date, and a regression can supply neither, so a known failure is recorded as an inspection instead. + +## Inspections + +The format also carries an `inspection` key, for claims no simulation can measure, such as a parameterisation or a structural fact. +Those records are written by hand and live in the tree next to the plan they support. +DVSim never produces them; it only passes their path through to whatever consumes this format, so they are out of scope for this document. + +## Consumers + +[DVPlan](https://github.com/lowRISC/dvplan) reads it to back-annotate a verification plan. +It is not the only thing that could: the format carries no DVPlan concepts, and a dashboard or a CI job wanting machine-readable regression results can read the same file. + +## Changing it + +The pydantic models in `src/dvsim/report/dv_evidence.py` specify what a producer writes, and this document describes them. +They are not the whole format: the `inspection` half and the rules for scoring a file are the consumer's, and [DVPlan](https://github.com/lowRISC/dvplan) specifies those. +So the models here reject a file carrying an `inspection` key, which is correct, since DVSim writes evidence and never reads it back. +A change to the models or to this document is a change to the format, so change both, and bear in mind that a consumer may be reading files this repo wrote months ago. diff --git a/src/dvsim/flow/base.py b/src/dvsim/flow/base.py index 497e8a42..9a0f199c 100644 --- a/src/dvsim/flow/base.py +++ b/src/dvsim/flow/base.py @@ -18,7 +18,7 @@ import dvsim.instrumentation.runtime as instrumentation from dvsim.flow.hjson import set_target_attribute -from dvsim.job.data import CompletedJobStatus, JobSpec, WorkspaceConfig +from dvsim.job.data import CompletedJobStatus, JobSpec, JobStatusInfo, WorkspaceConfig from dvsim.job.status import JobStatus from dvsim.logging import log from dvsim.scheduler.resources import UnknownResourcePolicy @@ -491,9 +491,20 @@ def deploy_objects(self) -> Sequence[CompletedJobStatus]: interactive=self.interactive, backend=backend, resource_manager=resource_manager, + on_job_completed=self.on_job_completed, ) ) + def on_job_completed( + self, spec: JobSpec, status: JobStatus, reason: JobStatusInfo | None + ) -> None: + """Observe a job reaching a terminal state. Flows that care override this. + + Called by the scheduler as each job finishes, so a flow can record an outcome while the + run is still going. Not abstract, since observing this is opt-in. + """ + del spec, status, reason + @abstractmethod def gen_results(self, results: Sequence[CompletedJobStatus]) -> None: """Generate flow results. diff --git a/src/dvsim/job/data.py b/src/dvsim/job/data.py index bb269bd3..6197a2eb 100644 --- a/src/dvsim/job/data.py +++ b/src/dvsim/job/data.py @@ -10,6 +10,7 @@ """ from collections.abc import Callable, Mapping, Sequence +from enum import Enum from pathlib import Path from typing import TypeAlias @@ -20,12 +21,30 @@ __all__ = ( "CompletedJobStatus", + "DependencyPolicy", "JobSpec", "JobStatusInfo", "WorkspaceConfig", ) +class DependencyPolicy(Enum): + """When a job may start, given how the jobs it depends on ended.""" + + ALL_PASSING = "all_passing" + """Start only if every dependency passed, which is right for a job consuming their output.""" + + ANY_PASSING = "any_passing" + """Start if at least one dependency passed, for a job gathering whatever results exist.""" + + ALWAYS = "always" + """Start once every dependency is terminal, whatever they concluded. + + For a job whose input is the outcome itself rather than an artefact a dependency produced, so + a regression where nothing passed is still the thing it has to report on. + """ + + class WorkspaceConfig(BaseModel): """Workspace configuration.""" @@ -92,8 +111,8 @@ class JobSpec(BaseModel): dependencies: list[str] """Full names of the other Jobs that this one depends on.""" - needs_all_dependencies_passing: bool - """Wait for dependent jobs to pass before scheduling.""" + dependency_policy: DependencyPolicy + """What the jobs this one depends on must have concluded before it may be scheduled.""" weight: int """Weight to apply to the scheduling priority.""" timeout_mins: float | None diff --git a/src/dvsim/job/deploy.py b/src/dvsim/job/deploy.py index 6cdac479..6ff1b510 100644 --- a/src/dvsim/job/deploy.py +++ b/src/dvsim/job/deploy.py @@ -12,11 +12,18 @@ from typing import TYPE_CHECKING, ClassVar from dvsim.flow.base import FlowCfg -from dvsim.job.data import JobSpec +from dvsim.job.data import DependencyPolicy, JobSpec from dvsim.job.status import JobStatus from dvsim.job.time import JobTime from dvsim.logging import log from dvsim.report.data import IPMeta, ToolMeta +from dvsim.report.dv_evidence import write_evidence +from dvsim.report.vplan import ( + VPLAN_DIR, + VPlanInputs, + overall_coverage, + shell_command, +) from dvsim.test import Test from dvsim.tool.utils import get_sim_tool_plugin from dvsim.utils import ( @@ -87,10 +94,9 @@ def __init__(self, sim_cfg: "FlowCfg") -> None: # A list of jobs on which this job depends. self.dependencies = [] - # Indicates whether running this job requires all dependencies to pass. - # If this flag is set to False, any passing dependency will trigger - # this current job to run - self.needs_all_dependencies_passing = True + # What the jobs this one depends on must have concluded before it may run. The default + # suits anything consuming a dependency's output, which is most jobs + self.dependency_policy = DependencyPolicy.ALL_PASSING # These variables will be extracted from the hjson file by _set_attrs, # and then _check_attrs checks that they were indeed extracted. Define @@ -168,7 +174,7 @@ def get_job_spec(self) -> "JobSpec": ), workspace_cfg=self.sim_cfg.workspace_cfg, dependencies=[d.full_name for d in self.dependencies], - needs_all_dependencies_passing=self.needs_all_dependencies_passing, + dependency_policy=self.dependency_policy, weight=self.weight, timeout_mins=(None if self.gui else self.get_timeout_mins()), cmd=self.cmd, @@ -177,7 +183,7 @@ def get_job_spec(self) -> "JobSpec": interactive=self.sim_cfg.interactive, odir=self.odir, renew_odir=self.renew_odir, - log_path=Path(f"{self.odir}/{self.target}.log"), + log_path=self.log_path, pre_launch=self.pre_launch(), post_finish=self.post_finish(), pass_patterns=self.pass_patterns, @@ -379,6 +385,11 @@ def is_equivalent_job(self, item: "Deploy") -> bool: log.verbose('Deploy job "%s" is equivalent to "%s"', item.name, self.name) return True + @property + def log_path(self) -> Path: + """Path to the log this job writes.""" + return Path(f"{self.odir}/{self.target}.log") + def pre_launch(self) -> Callable[[], None]: """Get pre-launch callback.""" @@ -899,8 +910,8 @@ def __init__(self, run_items: Iterable[RunTest], sim_cfg: FlowCfg) -> None: super().__init__(sim_cfg) self.dependencies.extend(run_items) - # Run coverage merge even if one test passes. - self.needs_all_dependencies_passing = False + # Merge whatever coverage exists, so one passing test is enough to be worth merging. + self.dependency_policy = DependencyPolicy.ANY_PASSING # Append cov_db_dirs to the list of exports. self.merged_exports["cov_db_dirs"] = shlex.quote(" ".join(self.cov_db_dirs)) @@ -1038,104 +1049,114 @@ def _set_attrs(self) -> None: class CovVPlan(Deploy): - """Abstraction for generating a Verification Plan (vPlan) report using DVPlan.""" + """Back-annotate the DVPlan verification plan, as a job of its own. + + Scheduled like any other job, so the step gets a row in the status table and can depend on the + runs it annotates. + """ target = "cov_vplan" weight = 10 - def __init__(self, cov_report_job, sim_cfg) -> None: - self.report_job = cov_report_job + def __init__(self, dependencies: "Iterable[Deploy]", sim_cfg: "SimCfg") -> None: + """Construct the job, depending on whatever must finish before the plan can be scored.""" + # Register a copy of sim_cfg which is explicitly the SimCfg type + self._typed_sim_cfg: SimCfg = sim_cfg + # Extracted from the hjson cfg by _set_attrs, and declared here so a type checker knows + # they exist, as the base class does for its own + self.proj_root: str = "" + self.vplan: str = "" + self.dut_instance: str = "" + self.dvplan_inspect: str = "" # Populated by post_finish() once the job completes successfully. self.vplan_coverage: float | None = None super().__init__(sim_cfg) - self.dependencies.append(cov_report_job) + # Every run it scores has to be terminal first, so the collector's evidence is complete + self.dependencies.extend(dependencies) + # A failed or killed run is still evidence, and a regression where nothing passed is the + # case the plan most needs to describe, so this is scored whatever the dependencies did + self.dependency_policy = DependencyPolicy.ALWAYS def _define_attrs(self) -> None: super()._define_attrs() - self.mandatory_cmd_attrs.update( - { - "proj_root": False, - "vplan": False, - } - ) + self.mandatory_cmd_attrs.update({"proj_root": False, "vplan": False}) self.mandatory_misc_attrs.update( { "dut_instance": False, + # Optional. Unlike the coverage report and the test results, inspection records + # are written by hand and live in the tree, so dvsim only points dvplan at them + "dvplan_inspect": False, } ) def _set_attrs(self) -> None: - self.cov_vplan_dir = f"{self.sim_cfg.scratch_path}/{self.target}" + # The base class derives `odir` from an attribute named after the target, and it does so + # inside the super() call below, so this has to be set first. + self.cov_vplan_dir = f"{self.sim_cfg.scratch_path}/{VPLAN_DIR}" super()._set_attrs() self.qual_name = self.target self.full_name = f"{self.sim_cfg.name}{self._variant_suffix}:{self.qual_name}" + self.output_dirs = [self.odir] - self.prepare_opts = self.sim_cfg.cov_vplan_prepare_opts - self.process_opts = self.sim_cfg.cov_vplan_process_opts + @property + def annotated_hjson(self) -> Path: + """Where the annotated plan is written.""" + return self._inputs().annotated + + @property + def report_page(self) -> Path: + """Where the plan's HTML report is written.""" + return self._inputs().report + + def _inputs(self) -> VPlanInputs: + """Describe the annotation, so `report.vplan` needs nothing from the flow config.""" + cfg = self._typed_sim_cfg + return VPlanInputs( + vplan=Path(self.vplan), + out_dir=Path(self.odir), + dut_entity=cfg.name, + dut_instance=self.dut_instance, + cov_report_dir=Path(cfg.cov_report_dir) if cfg.cov else None, + tool=cfg.tool or "", + inspect=self.dvplan_inspect, + prepare_opts=list(cfg.cov_vplan_prepare_opts), + process_opts=list(cfg.cov_vplan_process_opts), + ) - # Calculate IP root. - vplan_path = Path(self.vplan) - self.ip_root = str(vplan_path.parent.parent) + def _construct_cmd(self) -> str: + """Build the dvplan invocation this job runs.""" + return shell_command(self._inputs()) - # Use fixed output filenames so the report location is always predictable. - self.annotated_hjson = f"{self.odir}/vplan_annotated.hjson" - self.gen_html = f"{self.odir}/vplan_annotated.html" - self.output_dirs = [self.odir] + def pre_launch(self) -> Callable[[], None]: + """Get pre-launch callback.""" + + def callback() -> None: + """Write the evidence dvplan annotates the vPlan from. + + Every run this job depends on is terminal by now, so the collector holds them all. + Written here rather than with the end-of-run reports because dvplan needs every coverage + source in one invocation, as `report.vplan._process_command` explains. + """ + cfg = self._typed_sim_cfg + write_evidence( + self._inputs().evidence, + cfg.run_evidence.evidence( + block=cfg.block_meta(), + tool=cfg.tool, + timestamp=cfg.run_timestamp().isoformat(), + ), + ) + + return callback def post_finish(self) -> Callable[[JobStatus], None]: """Get post finish callback.""" def callback(status: JobStatus) -> None: - """Extract the overall vPlan normalised coverage from the annotated HJSON.""" + """Read the plan's overall score back, for the flow's own report to quote.""" if self.dry_run or status != JobStatus.PASSED: return - hjson_path = Path(self.annotated_hjson) - if not hjson_path.exists(): - return - try: - import hjson # noqa: PLC0415 - - with hjson_path.open() as f: - data = hjson.load(f) - # HJSON vPlans are keyed: {dut_name: {fields...}} - root_node = next(iter(data.values()), {}) - raw = root_node.get("Normalized_Coverage") - if raw is not None: - self.vplan_coverage = float(str(raw).rstrip(" %")) - except Exception: # noqa: BLE001 - log.debug("Could not extract vPlan coverage from '%s'.", hjson_path) + self.vplan_coverage = overall_coverage(self.annotated_hjson) return callback - - def _construct_cmd(self) -> str: - """Construct the pure bash shell command, bypassing the base Makefile assumption.""" - import shlex - import shutil - - if shutil.which("dvplan") is None: - fallback = ( - "echo 'WARNING: dvplan tool not installed in PATH. Skipping vPlan generation.'" - ) - return f"/usr/bin/env bash -c {shlex.quote(fallback)}" - - def format_opts(opts): - return " ".join(opts) if isinstance(opts, list) else str(opts) - - prepare_opts_str = format_opts(self.prepare_opts) - process_opts_str = format_opts(self.process_opts) - - prepare_cmd = f"dvplan prepare_vplan {prepare_opts_str} {self.ip_root} {self.vplan} {self.annotated_hjson}" - prepare_cmd = " ".join(prepare_cmd.split()) - - vendor_tool = f"{self.sim_cfg.tool}_report" - report_path = self.report_job.cov_report_dir - - process_cmd = ( - f"dvplan process_results {process_opts_str} --coverage {vendor_tool} {report_path} " - f"-R {self.gen_html} -s {self.sim_cfg.name} {self.dut_instance} {self.annotated_hjson}" - ) - process_cmd = " ".join(process_cmd.split()) - - full_command = f"set -e; mkdir -p {self.odir}; {prepare_cmd} && {process_cmd}" - return f"/usr/bin/env bash -c {shlex.quote(full_command)}" diff --git a/src/dvsim/report/dv_evidence.py b/src/dvsim/report/dv_evidence.py new file mode 100644 index 00000000..6c6fbd90 --- /dev/null +++ b/src/dvsim/report/dv_evidence.py @@ -0,0 +1,207 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Regression results in the tool-neutral `lowrisc-dv-evidence` format. + +These models are the format's definition, and `doc/dv_evidence.md` describes them. It lives here +because dvsim is what produces the file, so anything reading one can be written against a public +spec rather than against whichever consumer happened to be built first. + +The format carries no planning-tool concepts, so a verification plan can be scored from any +regression flow that emits it, and a person can write one by hand. + +Built from what the scheduler concludes about each job, through its completion hook. That is the +same state the JSON report is derived from, so the two cannot disagree about a run, and it is +available early enough for the vPlan job to read while the run is still going. +""" + +from enum import Enum +from importlib.metadata import PackageNotFoundError, version +from pathlib import Path + +from pydantic import BaseModel, ConfigDict, Field + +from dvsim.job.data import JobSpec, JobStatusInfo +from dvsim.job.status import JobStatus +from dvsim.logging import log +from dvsim.report.data import IPMeta +from dvsim.scheduler.core import ALL_FAILED_DEP, FAILED_DEP, KILLED_QUEUED, KILLED_SCHEDULED + +__all__ = ( + "EvidenceFile", + "Outcome", + "RunEvidenceCollector", + "run_outcome", + "write_evidence", +) + +# What the file calls itself. Named for the evidence it holds, which is test runs and manual +# inspections alike, rather than for either metric type +SCHEMA_ID = "lowrisc-dv-evidence" + +# The `target` the scheduler gives a job that runs a test. Builds and coverage jobs share the +# same result stream and are filtered out on this +RUN_TARGET = "run" + +# Reasons the scheduler reports for a job it cancelled rather than ran, imported rather than +# restated so a reworded message cannot silently stop matching +_CANCELLED_REASONS = frozenset( + reason.message for reason in (FAILED_DEP, ALL_FAILED_DEP, KILLED_SCHEDULED, KILLED_QUEUED) +) + + +class Outcome(Enum): + """How one run of a test ended, in the neutral format's vocabulary. + + There is no waived outcome. A waiver needs an owner and a date, and a regression can supply + neither, so the format only allows one on an inspection, which is written by hand. + """ + + PASSED = "passed" + FAILED = "failed" + KILLED = "killed" + NOT_RUN = "not_run" + + def __str__(self) -> str: + """Return the outcome as it appears in a results file.""" + return self.value + + +def run_outcome(status: JobStatus, reason: JobStatusInfo | None) -> Outcome: + """Map a job's terminal status onto the neutral format's vocabulary. + + `JobStatus.KILLED` covers both a job terminated while executing and one cancelled before it was + dispatched, which are different answers to "did this test run at all". The reason separates + them, and the scheduler records one against every job it completes. + """ + if status == JobStatus.PASSED: + return Outcome.PASSED + if status == JobStatus.FAILED: + return Outcome.FAILED + if reason is not None and reason.message in _CANCELLED_REASONS: + return Outcome.NOT_RUN + return Outcome.KILLED + + +class TestRun(BaseModel): + """One run of one test.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + __test__ = False # Named Test*, so pytest would otherwise collect it as a test class. + + status: Outcome + seed: int | None = None + log: Path | None = None + message: str | None = None + line: int | None = None + + +class EvidenceFile(BaseModel): + """A regression's results, in the tool-neutral evidence format. + + dvsim only ever fills the `testcase` half. The format also has an `inspection` key, for claims + no simulation can measure, and those records are written by hand. + """ + + model_config = ConfigDict(frozen=True, extra="forbid", populate_by_name=True) + + testcase: dict[str, list[TestRun]] + + schema_id: str = Field(default=SCHEMA_ID, alias="schema") + dut: str | None = None + tool: str | None = None + produced_by: str | None = None + revision: str | None = None + timestamp: str | None = None + + +class RunEvidenceCollector: + """Accumulates the outcome of every test run of one flow, as the scheduler concludes them. + + Fed by `Scheduler.add_job_completion_callback`, so a job cancelled before it ever started is + recorded too, and a test the plan expected reads as a hole rather than as an absent test. + """ + + def __init__(self) -> None: + """Start with nothing recorded. Runs arrive as the scheduler completes them.""" + self._runs: dict[str, list[TestRun]] = {} + + def record(self, spec: JobSpec, status: JobStatus, reason: JobStatusInfo | None) -> None: + """Record how one job ended, keeping only the ones that run a test. + + Grouped by job name, which is the name a vPlan addresses. Reseeds of one test share it and + are told apart by their seeds. + """ + if spec.target != RUN_TARGET: + return + failed = status != JobStatus.PASSED + self._runs.setdefault(spec.name, []).append( + TestRun( + status=run_outcome(status, reason), + seed=spec.seed, + log=spec.log_path, + message=reason.message if reason is not None and failed else None, + line=_first_line(reason) if failed else None, + ) + ) + + def evidence( + self, + *, + block: IPMeta, + tool: str | None = None, + timestamp: str | None = None, + ) -> EvidenceFile: + """Build the evidence document for everything recorded so far.""" + return EvidenceFile( + testcase=self._runs, + dut=block.variant_name(sep="/"), + tool=tool, + produced_by=_produced_by(), + revision=_revision(block), + timestamp=timestamp, + ) + + +def write_evidence(path: Path, evidence: EvidenceFile) -> Path: + """Write the evidence file, creating its directory if needed, and return its path. + + `IPMeta.url` is already stripped of credentials by `git_origin_url`, which matters because this + file is archived alongside the reports. + """ + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + evidence.model_dump_json(by_alias=True, indent=2, exclude_none=True), encoding="utf-8" + ) + log.debug("Wrote results for %d tests to '%s'", len(evidence.testcase), path) + return path + + +def _revision(block: IPMeta) -> str: + """Describe the revision the results were produced against, marking an uncommitted tree. + + Marked the way `sim.report` marks it, since the two describe the same run. This file outlives + the run, so it is the only chance to record it. + """ + revision = block.revision_info or block.url or block.commit + if block.dirty and "(dirty)" not in revision: + revision += " (dirty)" + return revision + + +def _produced_by() -> str: + """Name dvsim and its version, or just dvsim when it is not installed as a package.""" + try: + return f"dvsim {version('dvsim').strip()}" + except PackageNotFoundError: + log.debug("DVSim package not found, so its version is left out of the results") + return "dvsim" + + +def _first_line(reason: JobStatusInfo | None) -> int | None: + """Get the first log line a failure was reported at, where one was recorded.""" + if reason is None or not reason.lines: + return None + first = reason.lines[0] + return first if isinstance(first, int) else first[0] diff --git a/src/dvsim/report/vplan.py b/src/dvsim/report/vplan.py new file mode 100644 index 00000000..6891f767 --- /dev/null +++ b/src/dvsim/report/vplan.py @@ -0,0 +1,184 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Back-annotate a DVPlan verification plan from a finished regression. + +This module builds the command; `job.deploy.CovVPlan` runs it as a scheduled job, so the step gets +its own row in the job status table alongside build, run, cov_merge and cov_report. + +Nothing happens at all unless the sim cfg names a `vplan`. +""" + +import glob +import shlex +from collections.abc import Sequence +from dataclasses import dataclass, field +from pathlib import Path + +import hjson + +from dvsim.logging import log + +__all__ = ("SKIP_WITHOUT_DVPLAN", "VPlanInputs", "overall_coverage", "shell_command") + +# Scratch subdirectory the annotated plan and its report are written to. Unchanged, so an existing +# link to the report still resolves +VPLAN_DIR = "cov_vplan" + +ANNOTATED_HJSON = "vplan_annotated.hjson" +ANNOTATED_HTML = "vplan_annotated.html" +EVIDENCE_JSON = "dv_evidence.json" + +# Whether dvplan is installed is decided by the script, on the machine the job lands on, rather +# than by dvsim on whichever host the run was launched from. Exits 0 so that a checkout without +# dvplan does not fail every regression that names a vPlan +SKIP_WITHOUT_DVPLAN = ( + "if ! command -v dvplan >/dev/null 2>&1; then " + "echo 'WARNING: dvplan is not installed on PATH. Skipping vPlan annotation.'; exit 0; fi;" +) + + +@dataclass(frozen=True) +class VPlanInputs: + """Everything the annotation needs, so this module never reaches back into a flow config.""" + + vplan: Path + """The verification plan to annotate.""" + out_dir: Path + """Where the annotated plan, its report and the evidence file are written.""" + dut_entity: str + """Name of the DUT entity, as a vPlan addresses it.""" + dut_instance: str + """Hierarchical path to the DUT in the testbench, such as `tb.dut`.""" + cov_report_dir: Path | None + """The vendor coverage report to annotate from, if the run produced one.""" + tool: str + """Simulator name, which selects the vendor report format.""" + inspect: str = "" + """Where hand-written inspection records live, if the cfg names any. A path or a glob.""" + prepare_opts: list[str] = field(default_factory=list) + process_opts: list[str] = field(default_factory=list) + + @property + def annotated(self) -> Path: + """Where the annotated plan is written.""" + return self.out_dir / ANNOTATED_HJSON + + @property + def report(self) -> Path: + """Where the plan's HTML report is written.""" + return self.out_dir / ANNOTATED_HTML + + @property + def evidence(self) -> Path: + """Where the regression's evidence file is written, and read back from.""" + return self.out_dir / EVIDENCE_JSON + + +def shell_command(inputs: VPlanInputs) -> str: + """Build the bash command that prepares and annotates the vPlan. + + Returned as one `bash -c` string because a scheduled job runs a shell command. `set -e` and the + `&&` mean a broken annotation shows as a failed job rather than a silently missing score. + + The command is the same whether or not dvplan is installed here, because here is not where it + runs. See `SKIP_WITHOUT_DVPLAN`. + """ + # The vPlan sits at //, so its grandparent is the IP root that + # `prepare_vplan` traces specifications against. + ip_root = inputs.vplan.parent.parent + prepare = [ + "dvplan", + "prepare_vplan", + *_opts(inputs.prepare_opts), + str(ip_root), + str(inputs.vplan), + str(inputs.annotated), + ] + process = _process_command(inputs) + + script = ( + f"set -e; {SKIP_WITHOUT_DVPLAN} " + f"mkdir -p {shlex.quote(str(inputs.out_dir))}; " + f"{shlex.join(prepare)} && {shlex.join(process)}" + ) + return f"/usr/bin/env bash -c {shlex.quote(script)}" + + +def _process_command(inputs: VPlanInputs) -> list[str]: + """Build the `process_results` invocation, with every coverage source it should read. + + Every source goes to one invocation on purpose: dvplan writes an item off as unmeasurable only + when none of the sources given to it can measure its field, so a second run would find the + items only its own source answers for already written off. + """ + coverage: list[str] = [] + if inputs.cov_report_dir: + coverage += ["--coverage", f"{inputs.tool}_report", str(inputs.cov_report_dir)] + # One source for both: dvplan reads the testcase and inspection metrics out of the same format. + # A glob is expanded here because the argv is built directly, with no shell to do it + evidence = [str(inputs.evidence)] + if inputs.inspect: + evidence += _expand(inputs.inspect) + coverage += ["--coverage", "dv_evidence", *evidence] + return [ + "dvplan", + "process_results", + *_opts(inputs.process_opts), + *coverage, + "-R", + str(inputs.report), + "-s", + inputs.dut_entity, + inputs.dut_instance, + str(inputs.annotated), + ] + + +def _opts(opts: Sequence[str]) -> list[str]: + """Split cfg-supplied options into argv entries, dropping empty ones. + + Two shapes turn up in real cfgs that a bare splat would pass to dvplan as literal arguments: + `[""]` for "none", which argparse reads as an empty positional, and `["--milestone-depth 1"]` + written as one string, which it reads as a single unknown flag. Split the way a shell would, so + an option carrying a quoted value stays one argument. + """ + return [token for opt in opts for token in shlex.split(opt)] + + +def _expand(pattern: str) -> list[str]: + """Expand an inspection path, which may be a file, a directory or a glob pattern. + + A cfg naming inspections through `{proj_root}` always produces an absolute pattern, which + `Path.glob` refuses, so this is one of the places the pathlib rule does not apply. + + A pattern matching nothing raises, because both other answers are worse: passing it through + fails the job with dvplan's own message once the regression has already run, and dropping it + scores the plan as though the cfg had never named inspections at all. The command is built + while the jobs are, so this lands before a single test starts. + """ + matches = sorted(glob.glob(pattern)) # noqa: PTH207 (Path.glob rejects an absolute pattern) + if not matches: + msg = f"No inspection records matched 'dvplan_inspect' pattern '{pattern}'." + raise ValueError(msg) + return matches + + +def overall_coverage(annotated: Path) -> float | None: + """Read the plan's overall normalised coverage back out of the annotated vPlan.""" + if not annotated.is_file(): + log.warning("No annotated vPlan at '%s', so its score is not reported.", annotated) + return None + try: + with annotated.open(encoding="utf-8") as f: + data = hjson.load(f) + # An HJSON vPlan is keyed by DUT name: {dut_name: {fields...}}. + root = next(iter(data.values()), {}) + raw = root.get("Normalized_Coverage") + if raw is None: + return None + return float(str(raw).rstrip(" %")) + except (OSError, ValueError, AttributeError, hjson.HjsonDecodeError): + log.exception("Could not read the vPlan score from '%s'.", annotated) + return None diff --git a/src/dvsim/scheduler/core.py b/src/dvsim/scheduler/core.py index 026ddd0e..e81f5c82 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, DependencyPolicy, JobSpec, JobStatusInfo from dvsim.job.status import JobStatus from dvsim.logging import log from dvsim.runtime.backend import RuntimeBackend @@ -23,6 +23,7 @@ __all__ = ( "JobPriorityFn", "JobRecord", + "OnJobCompletionCb", "OnJobStatusChangeCb", "OnRunEndCb", "OnRunStartCb", @@ -63,6 +64,12 @@ class JobRecord: # The arguments are: (job spec, old status, new status). OnJobStatusChangeCb: TypeAlias = Callable[[JobSpec, JobStatus, JobStatus], None] +# Callbacks for observers, for when a job reaches a terminal state. +# The arguments are: (job spec, terminal status, the reason recorded with it). +# Separate from the status-change callback, which carries no reason and so cannot tell a +# cancelled job from one killed while running. +OnJobCompletionCb: TypeAlias = Callable[[JobSpec, JobStatus, JobStatusInfo | None], None] + # Callbacks for observers, for when the scheduler receives a kill signal (termination). OnSchedulerKillCb: TypeAlias = Callable[[], None] @@ -153,6 +160,7 @@ def __init__( # noqa: PLR0913 self._on_run_start: list[OnRunStartCb] = [] self._on_run_end: list[OnRunEndCb] = [] self._on_job_status_change: list[OnJobStatusChangeCb] = [] + self._on_job_completion: list[OnJobCompletionCb] = [] self._on_kill_signal: list[OnSchedulerKillCb] = [] self._jobs = self.build_graph(jobs, self._backends, self._default_backend) @@ -165,6 +173,10 @@ def add_run_end_callback(self, cb: OnRunEndCb) -> None: """Register an observer to notify when the scheduler run ends.""" self._on_run_end.append(cb) + def add_job_completion_callback(self, cb: OnJobCompletionCb) -> None: + """Register an observer to be notified as each job reaches a terminal state.""" + self._on_job_completion.append(cb) + def add_job_status_change_callback(self, cb: OnJobStatusChangeCb) -> None: """Register an observer to notify when the status of a job in the scheduler changes.""" self._on_job_status_change.append(cb) @@ -322,6 +334,10 @@ def _mark_job_completed( ) self._change_job_status(job, status, reason) + # Notified after the status is settled, so an observer sees what the scheduler concluded + for cb in self._on_job_completion: + cb(job.spec, status, reason) + # If the job was running, mark it as no longer running. if job.spec.id in self._running: self._running.remove(job.spec.id) @@ -349,7 +365,10 @@ def _update_completed_job_deps(self, job: JobRecord) -> None: # Handle dependency management and marking dependents as ready if dep.remaining_deps == 0 and dep.status == JobStatus.SCHEDULED: - if dep.spec.needs_all_dependencies_passing: + policy = dep.spec.dependency_policy + if policy is DependencyPolicy.ALWAYS: + self._mark_job_ready(dep) + elif policy is DependencyPolicy.ALL_PASSING: if dep.passing_deps == len(dep.spec.dependencies): self._mark_job_ready(dep) else: diff --git a/src/dvsim/scheduler/runner.py b/src/dvsim/scheduler/runner.py index 0244fff3..1d80902d 100644 --- a/src/dvsim/scheduler/runner.py +++ b/src/dvsim/scheduler/runner.py @@ -11,7 +11,7 @@ from dvsim.runtime.backend import RuntimeBackend from dvsim.runtime.fake import FakePolicy, FakeRuntimeBackend from dvsim.runtime.registry import backend_registry -from dvsim.scheduler.core import Scheduler +from dvsim.scheduler.core import OnJobCompletionCb, Scheduler from dvsim.scheduler.log_manager import LogManager from dvsim.scheduler.resources import ( ResourceManager, @@ -76,6 +76,7 @@ async def run_scheduler( interactive: bool, backend: RuntimeBackend, resource_manager: ResourceManager | None, + on_job_completed: OnJobCompletionCb | None = None, ) -> list[CompletedJobStatus]: """Run the scheduler with the given set of job specifications. @@ -85,6 +86,8 @@ async def run_scheduler( interactive: run the tool in interactive mode? backend: the scheduler backend to use resource_manager: the scheduler resource manager to use, if any. + on_job_completed: observer notified as each job reaches a terminal state, with the + reason the scheduler recorded for it. Returns: List of completed job status objects. @@ -112,6 +115,9 @@ async def run_scheduler( ), ) + if on_job_completed is not None: + scheduler.add_job_completion_callback(on_job_completed) + if not interactive: status_printer = create_status_printer(jobs) diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index 6dc400c4..6cdbed1c 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -15,7 +15,7 @@ from typing import ClassVar from dvsim.flow.base import FlowCfg -from dvsim.job.data import CompletedJobStatus, JobSpec +from dvsim.job.data import CompletedJobStatus, JobSpec, JobStatusInfo from dvsim.job.deploy import ( CompileSim, CovAnalyze, @@ -29,6 +29,7 @@ from dvsim.logging import log from dvsim.modes import BuildMode, Mode, RunMode, find_mode from dvsim.regression import Regression +from dvsim.report.dv_evidence import RunEvidenceCollector from dvsim.sim.data import ( IPMeta, SimFlowResults, @@ -158,10 +159,15 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: self.cov_report_dir = "" self.cov_report_page = "" - # Options for vPlan processing + # Options for vPlan processing. Extracted from the hjson cfg, and declared here so a type + # checker knows they exist. Nothing happens unless `vplan` names a plan + self.vplan: str = "" # dut_instance is the hierarchical testbench path to the DUT (e.g. "tb.dut"), # distinct from `name`/`qual_name` which identify the sim config itself. self.dut_instance = "" + # A file, a directory of them, or a glob holding hand-written dvplan inspection records. + # No regression produces these, so dvsim only passes the path on + self.dvplan_inspect = "" self.cov_vplan_prepare_opts = [] self.cov_vplan_process_opts = [] @@ -178,6 +184,10 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: self.run_list = [] self.cov_merge_deploy = None self.cov_report_deploy = None + self.cov_vplan_deploy = None + # Filled in by the scheduler's completion hook, and read by the vPlan job once every run it + # depends on is terminal + self.run_evidence = RunEvidenceCollector() self.results_summary = OrderedDict() super().__init__(flow_cfg_file, hjson_data, args, mk_config) @@ -560,9 +570,12 @@ def _create_deploy_objects(self) -> None: self.cov_report_deploy = CovReport(self.cov_merge_deploy, self) self.deploy += [self.cov_merge_deploy, self.cov_report_deploy] - if getattr(self, "vplan", False): - self.cov_vplan_deploy = CovVPlan(self.cov_report_deploy, self) - self.deploy.append(self.cov_vplan_deploy) + if self.vplan and self.runs: + # Depends on the coverage report where there is one, so the vendor report exists + # to annotate from, and otherwise straight on the runs it scores. + vplan_deps = [self.cov_report_deploy] if self.cov_report_deploy else self.runs + self.cov_vplan_deploy = CovVPlan(vplan_deps, self) + self.deploy.append(self.cov_vplan_deploy) def _cov_analyze(self) -> None: """Open GUI tool for coverage analysis. @@ -684,6 +697,45 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None: path=reports_dir, ) + def on_job_completed( + self, spec: JobSpec, status: JobStatus, reason: JobStatusInfo | None + ) -> None: + """Record each run's outcome as the scheduler concludes it. + + Fed from the scheduler rather than from a job callback, because only the scheduler sees a job + it cancelled before dispatching it. + """ + self.run_evidence.record(spec, status, reason) + + def run_timestamp(self) -> datetime: + """Return when this run started, as an aware datetime. + + `self.timestamp` is a `TS_FORMAT` string, which is a dvsim convention no consumer of a report + can be expected to parse, so everything written out of this flow goes through here. + """ + return datetime.strptime(self.timestamp, TS_FORMAT).replace(tzinfo=timezone.utc) + + def block_meta(self, url: str | None = None) -> IPMeta: + """Describe the design under test, for anything this flow writes about the run. + + Shared by the reports and the vPlan evidence file, so a run cannot say it came from a clean + tree in one artefact and a dirty one in another. + + Args: + url: link to the IP in git, or None to derive it from the checkout. + + """ + return IPMeta( + name=self.name.lower(), + variant=(self.variant or "").lower() or None, + commit=self.commit, + commit_short=self.commit_short, + dirty=self.dirty, + branch=self.branch or "", + url=url if url is not None else (git_https_url_with_commit(path=Path(self.proj_root))), + revision_info=self.revision, + ) + def _gen_json_results( self, run_results: Sequence[CompletedJobStatus], @@ -704,18 +756,8 @@ def _gen_json_results( self.testplan.map_test_results(sim_results.table) # --- Metadata --- - timestamp = datetime.strptime(self.timestamp, TS_FORMAT).replace(tzinfo=timezone.utc) - - block = IPMeta( - name=self.name.lower(), - variant=(self.variant or "").lower() or None, - commit=self.commit, - commit_short=self.commit_short, - dirty=self.dirty, - branch=self.branch or "", - url=url, - revision_info=self.revision, - ) + timestamp = self.run_timestamp() + block = self.block_meta(url=url) tool = ToolMeta(name=self.tool.lower(), version="unknown") build_seed = self.build_seed if not self.run_only else None @@ -842,10 +884,13 @@ def make_test_result(tr) -> TestResult | None: cov_report_dir = self.cov_report_dir or "cov_report" cov_report_page = Path(cov_report_dir, self.cov_report_page) + # Linked only once the page is actually there. The job can be killed, and it exits without + # annotating anything where dvplan is not installed, so its output directory is not proof vplan_report_page = None vplan_coverage = None - if getattr(self, "cov_vplan_deploy", None): - vplan_report_page = Path(self.scratch_path) / CovVPlan.target / "vplan_annotated.html" + if self.cov_vplan_deploy is not None: + page = self.cov_vplan_deploy.report_page + vplan_report_page = page if page.is_file() else None vplan_coverage = self.cov_vplan_deploy.vplan_coverage failures = BucketedFailures.from_job_status(results=run_results) diff --git a/src/dvsim/utils/git.py b/src/dvsim/utils/git.py index ce537bdf..0776d9de 100644 --- a/src/dvsim/utils/git.py +++ b/src/dvsim/utils/git.py @@ -5,6 +5,7 @@ """Git utility functions.""" from pathlib import Path +from urllib.parse import urlsplit, urlunsplit from git import Repo @@ -13,6 +14,26 @@ __all__ = ("repo_root",) +def strip_url_credentials(url: str) -> str: + """Remove any username/password from a URL, keeping the rest of it intact. + + A CI checkout leaves its access token in the remote URL as ``https://oauth2:@host/org/repo``. + That URL is recorded in the run metadata and published in the reports, which get archived, so the + token would outlive the run that leaked it. + + An ssh URL keeps its user: ``git@`` names an ssh account rather than an identity to authenticate + as, so dropping it gives a URL that no longer addresses the remote. A password is stripped + whatever the scheme carries it. + """ + parts = urlsplit(url) + userinfo, _, host = parts.netloc.rpartition("@") + if not userinfo: + return url + if ":" not in userinfo and parts.scheme not in ("http", "https"): + return url + return urlunsplit((parts.scheme, host, parts.path, parts.query, parts.fragment)) + + def repo_root(path: Path) -> Path | None: """Given a sub dir in a git repo provide the root path. @@ -56,7 +77,11 @@ def git_is_dirty(path: Path | None = None) -> bool: def git_origin_url(path: Path | None = None) -> str | None: - """Get the git remote origin url, or None if no ``origin`` remote is configured.""" + """Get the git remote origin url, or None if no ``origin`` remote is configured. + + Any credentials the remote carries are stripped, so that the url is safe to record in run + metadata and reports. + """ root = repo_root(path=path or Path.cwd()) if root is None: @@ -68,7 +93,7 @@ def git_origin_url(path: Path | None = None) -> str | None: if "origin" not in [remote.name for remote in r.remotes]: return None - return r.remote("origin").url + return strip_url_credentials(r.remote("origin").url) def git_https_url_with_commit(path: Path | None = None) -> str | None: diff --git a/tests/job/test_cov_vplan.py b/tests/job/test_cov_vplan.py new file mode 100644 index 00000000..1425a214 --- /dev/null +++ b/tests/job/test_cov_vplan.py @@ -0,0 +1,125 @@ +# 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 the vPlan back-annotation job. + +These construct the job for real. Every other test around the vPlan exercises a helper in +isolation, which cannot catch the job failing to build itself: `Deploy.__init__` derives +attributes by name and order, so a missing one is an `AttributeError` at config time that no +amount of testing the command builder would reveal. +""" + +from pathlib import Path +from types import SimpleNamespace + +import pytest +from hamcrest import assert_that, contains_string, equal_to, is_, none + +from dvsim.job.data import DependencyPolicy, WorkspaceConfig +from dvsim.job.deploy import CovVPlan +from dvsim.job.status import JobStatus +from dvsim.report.vplan import ANNOTATED_HJSON, ANNOTATED_HTML, VPLAN_DIR + + +def _cfg(**overrides: object) -> SimpleNamespace: + """Build the smallest sim cfg a `CovVPlan` can be constructed against.""" + attrs: dict[str, object] = { + "name": "hmac", + "flow": "sim", + "variant": "", + "tool": "xcelium", + "gui": False, + "interactive": False, + "dry_run": False, + "scratch_path": "/scratch/hmac", + "commit": "abc123", + "commit_short": "abc", + "branch": "main", + "revision": "", + "build_mode": "default", + "exports": [], + "flow_makefile": "sim.mk", + "proj_root": "/proj", + "vplan": "/proj/hw/ip/hmac/data/hmac_vplan.hjson", + "dut_instance": "tb.dut", + "dvplan_inspect": "", + "cov": True, + "cov_report_dir": "/scratch/hmac/cov_report", + "cov_vplan_prepare_opts": ["--bypass-trace"], + "cov_vplan_process_opts": [""], + "timeout_mins": None, + "max_odirs": 5, + "workspace_cfg": WorkspaceConfig( + timestamp="20260818_090000", + project_root=Path("/proj"), + scratch_root=Path("/scratch"), + scratch_path=Path("/scratch/hmac"), + ), + } + attrs.update(overrides) + return SimpleNamespace(**attrs) + + +@pytest.fixture +def job() -> CovVPlan: + """Construct the job the way the sim flow does.""" + return CovVPlan([], _cfg()) + + +def test_the_job_constructs_and_lands_in_its_own_output_directory(job: CovVPlan) -> None: + """`Deploy` derives `odir` from an attribute named after the target, inside `_set_attrs`. + + That ordering is easy to get wrong and fails at config time rather than at run time, taking + the whole invocation down before a single test starts. + """ + assert_that(job.odir, equal_to(f"/scratch/hmac/{VPLAN_DIR}")) + assert_that(job.qual_name, equal_to("cov_vplan")) + assert_that(job.full_name, equal_to("hmac:cov_vplan")) + assert_that(job.annotated_hjson, equal_to(Path("/scratch/hmac") / VPLAN_DIR / ANNOTATED_HJSON)) + assert_that(job.report_page, equal_to(Path("/scratch/hmac") / VPLAN_DIR / ANNOTATED_HTML)) + + +def test_the_job_builds_a_runnable_command(job: CovVPlan) -> None: + """The command is built during construction, so a broken builder is a config-time failure.""" + assert_that(job.cmd, contains_string("dvplan prepare_vplan --bypass-trace")) + assert_that(job.cmd, contains_string("dvplan process_results")) + assert_that(job.cmd, contains_string("--coverage xcelium_report")) + assert_that(job.cmd, contains_string("--coverage dv_evidence")) + # `cov_vplan_process_opts: [""]` is idiomatic hjson for "none" and must not become an + # empty argument, which argparse would read as the DUT name. + assert_that(job.cmd, contains_string("-s hmac tb.dut")) + + +def test_a_run_without_coverage_still_annotates() -> None: + """Without --cov there is no vendor report, and the plan is scored from the evidence alone.""" + job = CovVPlan([], _cfg(cov=False)) + + assert_that(job.cmd, contains_string("--coverage dv_evidence")) + assert_that("xcelium_report" in job.cmd, is_(False)) + + +def test_an_inspection_pattern_matching_nothing_fails_at_config_time() -> None: + """The cfg names records that are not there, so the run must stop before it burns a regression. + + The command is built in `Deploy.__init__`, so this lands while the jobs are still being + created rather than hours later inside dvplan. + """ + with pytest.raises(ValueError, match="No inspection records matched"): + CovVPlan([], _cfg(dvplan_inspect="/proj/hw/ip/hmac/dv/inspections/*.json")) + + +def test_a_failing_run_still_gets_its_plan_scored(job: CovVPlan) -> None: + """A regression where nothing passed is the case the plan most needs to describe. + + `ANY_PASSING` would not do here. With `--cov` this job has one dependency, the coverage + report, so anything that stops the report also stops the plan being scored. + """ + assert_that(job.dependency_policy, equal_to(DependencyPolicy.ALWAYS)) + + +def test_no_score_is_read_back_when_the_job_did_not_pass(job: CovVPlan) -> None: + """Reading a plan the job failed to write would report a stale or partial figure.""" + job.post_finish()(JobStatus.FAILED) + + assert_that(job.vplan_coverage, is_(none())) diff --git a/tests/report/test_dv_evidence.py b/tests/report/test_dv_evidence.py new file mode 100644 index 00000000..40393753 --- /dev/null +++ b/tests/report/test_dv_evidence.py @@ -0,0 +1,272 @@ +# 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 the tool-neutral evidence file written for vPlan back-annotation. + +The format is a contract with dvplan and with any other flow that consumes it, so these cover +what the file says as much as how it is built: which job statuses map onto which outcome, and that a +run the scheduler cancelled is still reported rather than dropped. +""" + +import json +from pathlib import Path +from types import SimpleNamespace + +import pytest +from hamcrest import assert_that, contains_string, equal_to, has_key, is_, none, not_ + +from dvsim.job.data import DependencyPolicy, JobSpec, JobStatusInfo, WorkspaceConfig +from dvsim.job.status import JobStatus +from dvsim.report.data import IPMeta, ToolMeta +from dvsim.report.dv_evidence import ( + SCHEMA_ID, + Outcome, + RunEvidenceCollector, + run_outcome, + write_evidence, +) +from dvsim.scheduler.core import ( + ALL_FAILED_DEP, + FAILED_DEP, + KILLED_QUEUED, + KILLED_RUNNING_SIGTERM, + KILLED_SCHEDULED, + OnJobCompletionCb, +) +from dvsim.sim.flow import SimCfg + +_BLOCK = IPMeta( + name="hmac", + variant=None, + commit="abc123", + commit_short="abc", + branch="main", + url="https://github.com/lowRISC/mocha/tree/abc123", + revision_info=None, +) +_TOOL = "xcelium" +_WORKSPACE = WorkspaceConfig( + timestamp="20260813_060029", + project_root=Path("/proj"), + scratch_root=Path("/scratch"), + scratch_path=Path("/scratch/hmac"), +) + + +def _spec( + name: str, + *, + seed: int | None = 0, + target: str = "run", +) -> JobSpec: + """Build the job spec the scheduler hands an observer when a job completes.""" + return JobSpec( + name=name, + job_type="RunTest", + target=target, + backend=None, + resources=None, + seed=seed, + full_name=f"hmac:{seed}.{name}", + qual_name=f"{seed}.{name}", + block=_BLOCK, + tool=ToolMeta(name=_TOOL, version="unknown"), + workspace_cfg=_WORKSPACE, + dependencies=[], + dependency_policy=DependencyPolicy.ALL_PASSING, + weight=1, + timeout_mins=None, + cmd="make run", + exports={}, + dry_run=False, + interactive=False, + odir=f"/scratch/hmac/{seed}.{name}", + renew_odir=True, + log_path=Path(f"/scratch/hmac/{seed}.{name}/run.log"), + pre_launch=lambda: None, + post_finish=lambda _s: None, + pass_patterns=[], + fail_patterns=[], + ) + + +def _collect(*records: tuple[JobSpec, JobStatus, JobStatusInfo | None]) -> RunEvidenceCollector: + """Feed a collector the way the scheduler's completion hook does.""" + collector = RunEvidenceCollector() + for spec, status, reason in records: + collector.record(spec, status, reason) + return collector + + +def _evidence(*records: tuple[JobSpec, JobStatus, JobStatusInfo | None]): + """Build the evidence document for a set of completed jobs.""" + return _collect(*records).evidence(block=_BLOCK, tool=_TOOL, timestamp="2026-08-13T06:00:29Z") + + +@pytest.mark.parametrize( + ("status", "reason", "expected"), + [ + (JobStatus.PASSED, None, Outcome.PASSED), + (JobStatus.FAILED, JobStatusInfo(message="UVM_ERROR"), Outcome.FAILED), + # Killed while executing is a different answer from never having started. + (JobStatus.KILLED, KILLED_RUNNING_SIGTERM, Outcome.KILLED), + (JobStatus.KILLED, None, Outcome.KILLED), + (JobStatus.KILLED, FAILED_DEP, Outcome.NOT_RUN), + (JobStatus.KILLED, ALL_FAILED_DEP, Outcome.NOT_RUN), + (JobStatus.KILLED, KILLED_SCHEDULED, Outcome.NOT_RUN), + (JobStatus.KILLED, KILLED_QUEUED, Outcome.NOT_RUN), + ], + ids=[ + "passed", + "failed", + "killed_running", + "killed_no_reason", + "dep_failed", + "all_deps_failed", + "dep_killed", + "killed_queued", + ], +) +def test_job_status_maps_onto_the_neutral_vocabulary( + status: JobStatus, reason: JobStatusInfo | None, expected: Outcome +) -> None: + """`JobStatus.KILLED` covers two outcomes, and the scheduler's reason separates them. + + The cancel reasons are imported from `scheduler.core` rather than restated, so rewording one + of them fails here instead of silently reclassifying every cancelled run as `killed`. + """ + assert_that(run_outcome(status, reason), is_(expected)) + + +def test_a_cancelled_run_is_reported_rather_than_dropped() -> None: + """A run the scheduler cancelled is a hole in the plan, so it has to appear as `not_run`. + + Dropping it would leave the test looking like it passed everything it attempted, when the + plan expected a run that never happened. + """ + evidence = _evidence( + (_spec("hmac_smoke", seed=0), JobStatus.PASSED, None), + (_spec("hmac_smoke", seed=1), JobStatus.KILLED, FAILED_DEP), + ) + + statuses = [run.status for run in evidence.testcase["hmac_smoke"]] + assert_that(statuses, equal_to([Outcome.PASSED, Outcome.NOT_RUN])) + + +def test_runs_are_grouped_by_test_name() -> None: + """Reseeds of one test share a name, which is what a vPlan addresses them by.""" + evidence = _evidence( + (_spec("hmac_smoke", seed=0), JobStatus.PASSED, None), + (_spec("hmac_smoke", seed=1), JobStatus.PASSED, None), + (_spec("hmac_stress", seed=2), JobStatus.PASSED, None), + ) + + assert_that(sorted(evidence.testcase), equal_to(["hmac_smoke", "hmac_stress"])) + assert_that(len(evidence.testcase["hmac_smoke"]), equal_to(2)) + assert_that([run.seed for run in evidence.testcase["hmac_smoke"]], equal_to([0, 1])) + + +def test_only_run_jobs_are_tests() -> None: + """Builds and coverage jobs share the result stream and are not tests. + + They carry names of their own, so including them would invent testcase items a vPlan could + never have asked for. + """ + evidence = _evidence( + (_spec("hmac_smoke", target="run"), JobStatus.PASSED, None), + (_spec("default", target="build"), JobStatus.PASSED, None), + (_spec("cov_merge", target="cov_merge"), JobStatus.PASSED, None), + ) + + assert_that(list(evidence.testcase), equal_to(["hmac_smoke"])) + + +def test_a_failing_run_records_what_reproduces_and_explains_it() -> None: + """A failing run carries the seed, the log and the failure somebody needs to read.""" + reason = JobStatusInfo(message="UVM_ERROR digest mismatch", lines=[481]) + evidence = _evidence((_spec("hmac_smoke", seed=7), JobStatus.FAILED, reason)) + + run = evidence.testcase["hmac_smoke"][0] + assert_that(run.status, is_(Outcome.FAILED)) + assert_that(run.seed, equal_to(7)) + assert_that(run.log, equal_to(Path("/scratch/hmac/7.hmac_smoke/run.log"))) + assert_that(run.message, equal_to("UVM_ERROR digest mismatch")) + assert_that(run.line, equal_to(481)) + + +def test_a_passing_run_records_no_failure() -> None: + """Failure detail is only meaningful for a run that did not pass.""" + evidence = _evidence((_spec("hmac_smoke"), JobStatus.PASSED, JobStatusInfo(message="ignored"))) + + assert_that(evidence.testcase["hmac_smoke"][0].message, is_(none())) + + +def test_written_results_name_their_schema_and_provenance(tmp_path: Path) -> None: + """The file says what it is and where it came from, which is what makes it auditable later.""" + evidence = _evidence( + (_spec("hmac_smoke", seed=0), JobStatus.PASSED, None), + (_spec("hmac_smoke", seed=1), JobStatus.FAILED, JobStatusInfo(message="boom")), + ) + + path = write_evidence(tmp_path / "reports" / "dv_evidence.json", evidence) + written = json.loads(path.read_text(encoding="utf-8")) + + assert_that(written["schema"], equal_to(SCHEMA_ID)) + assert_that(written["dut"], equal_to("hmac")) + assert_that(written["tool"], equal_to(_TOOL)) + assert_that(written["produced_by"], contains_string("dvsim")) + assert_that(written["testcase"], has_key("hmac_smoke")) + # A test maps straight to its runs, with no wrapper object in between. + statuses = [run["status"] for run in written["testcase"]["hmac_smoke"]] + assert_that(statuses, equal_to(["passed", "failed"])) + + +def test_the_written_provenance_records_a_dirty_tree(tmp_path: Path) -> None: + """A vPlan figure produced from uncommitted work must not read as coming from the commit. + + dvsim's own report marks the revision '(dirty)', so an evidence file that dropped the flag + would disagree with the report for the same run, and the disagreement would only show up + when somebody went back to reproduce the figure. + """ + collector = _collect((_spec("hmac_smoke"), JobStatus.PASSED, None)) + clean = write_evidence(tmp_path / "clean.json", collector.evidence(block=_BLOCK, tool=_TOOL)) + dirty = write_evidence( + tmp_path / "dirty.json", + collector.evidence(block=_BLOCK.model_copy(update={"dirty": True}), tool=_TOOL), + ) + + # Same block either way, so only the flag can account for the difference. + assert_that( + json.loads(clean.read_text(encoding="utf-8"))["revision"], not_(contains_string("dirty")) + ) + assert_that( + json.loads(dirty.read_text(encoding="utf-8"))["revision"], contains_string("(dirty)") + ) + + +def test_the_collector_is_reachable_from_the_scheduler_hook() -> None: + """The collector's method has to match the callback the scheduler will call it through. + + Wiring it up is the one part unit tests would otherwise miss entirely: a signature drift + here surfaces only at the end of a real regression, when the vPlan job reads an empty file. + """ + collector = RunEvidenceCollector() + scheduler_cb: OnJobCompletionCb = collector.record + + scheduler_cb(_spec("hmac_smoke", seed=3), JobStatus.PASSED, None) + + assert_that(collector.evidence(block=_BLOCK).testcase, has_key("hmac_smoke")) + + +def test_a_flow_forwards_completions_to_its_collector() -> None: + """`SimCfg.on_job_completed` is the override the scheduler drives, so it has to forward. + + Called unbound against a stand-in, since constructing a real `SimCfg` needs a whole hjson cfg + and none of it bears on the forwarding. + """ + flow = SimpleNamespace(run_evidence=RunEvidenceCollector()) + + SimCfg.on_job_completed(flow, _spec("hmac_smoke", seed=3), JobStatus.PASSED, None) + + assert_that(flow.run_evidence.evidence(block=_BLOCK).testcase, has_key("hmac_smoke")) diff --git a/tests/report/test_vplan.py b/tests/report/test_vplan.py new file mode 100644 index 00000000..5e8cd658 --- /dev/null +++ b/tests/report/test_vplan.py @@ -0,0 +1,173 @@ +# 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 back-annotating a DVPlan verification plan after a regression. + +The command built here is the interface to another tool, whose positional shape is a fixed +contract, so it is checked as carefully as anything that runs in this process. The rest covers +the promise that no vPlan problem can fail a regression that otherwise passed, and that the one +cfg mistake which can is caught while the jobs are still being built. +""" + +from dataclasses import replace +from pathlib import Path + +import pytest +from hamcrest import assert_that, contains_string, equal_to, is_, none + +from dvsim.report.vplan import ( + ANNOTATED_HJSON, + VPlanInputs, + _expand, + _process_command, + overall_coverage, + shell_command, +) + + +def _inputs(tmp_path: Path, **overrides: object) -> VPlanInputs: + """Build the inputs for one annotation, overriding whatever a test cares about.""" + base = VPlanInputs( + vplan=tmp_path / "hw" / "ip" / "hmac" / "doc" / "hmac_vplan.hjson", + out_dir=tmp_path / "out", + dut_entity="hmac", + dut_instance="tb.dut", + cov_report_dir=Path("/scratch/hmac/cov_report"), + tool="xcelium", + ) + return replace(base, **overrides) + + +def test_the_command_keeps_dvplan_s_positional_contract(tmp_path: Path) -> None: + """`process_results` takes its three positionals last, with `-s` as a flag before them. + + This pins what dvsim emits. It cannot check dvplan's side, which lives in another repo, so a + failure here means the argv moved and the two need reconciling. Getting `-s` wrong is the + error that reads as `--summary` swallowing the DUT name, hence pinning it rather than + discovering it in a nightly. + """ + inputs = _inputs(tmp_path) + + command = _process_command(inputs) + + assert_that(command[:2], equal_to(["dvplan", "process_results"])) + # -s is a switch, and the three positionals follow it in order. + assert_that(command[-4:], equal_to(["-s", "hmac", "tb.dut", str(inputs.annotated)])) + + +def test_every_coverage_source_reaches_one_invocation(tmp_path: Path) -> None: + """The vendor report, the test results and the inspections annotate in a single run. + + dvplan writes a plan item off as unmeasurable only when none of the sources it was given can + measure the item's field, so splitting these would lose whichever metric the first run lacked. + """ + inspections = tmp_path / "inspections" + inspections.mkdir() + command = _process_command(_inputs(tmp_path, inspect=str(inspections))) + + joined = " ".join(command) + assert_that(joined, contains_string("--coverage xcelium_report /scratch/hmac/cov_report")) + assert_that(joined, contains_string("--coverage dv_evidence")) + # One source, so the inspections ride along with the evidence rather than repeating the flag. + assert_that(joined, contains_string(str(inspections))) + assert_that(joined.count("--coverage"), equal_to(2)) + + +def test_the_vendor_report_is_left_out_when_there_is_none(tmp_path: Path) -> None: + """Without coverage the plan is still annotated, from the recorded evidence alone.""" + command = _process_command(_inputs(tmp_path, cov_report_dir=None)) + + assert_that(" ".join(command), contains_string("--coverage dv_evidence")) + assert_that("xcelium_report" in " ".join(command), is_(False)) + + +def test_an_absolute_inspection_glob_expands(tmp_path: Path) -> None: + """A cfg names inspections through `{proj_root}`, so the pattern is always absolute. + + `Path().glob` rejects an absolute pattern outright, so getting this wrong raises rather than + degrading, which would take a passing regression down with it. + """ + for name in ("reset", "security"): + (tmp_path / f"{name}.inspect.json").write_text("{}", encoding="utf-8") + + matches = _expand(str(tmp_path / "*.inspect.json")) + + assert_that( + matches, + equal_to([str(tmp_path / "reset.inspect.json"), str(tmp_path / "security.inspect.json")]), + ) + + +def test_a_directory_of_inspections_is_passed_through(tmp_path: Path) -> None: + """A path that exists needs no expansion, since dvplan reads a directory itself.""" + folder = tmp_path / "inspections" + folder.mkdir() + + assert_that(_expand(str(folder)), equal_to([str(folder)])) + + +def test_a_pattern_matching_nothing_is_a_config_error(tmp_path: Path) -> None: + """Naming inspections that do not exist is a cfg mistake, and both other answers are worse. + + Passing the pattern on would fail the job with dvplan's own message once the regression has + already run, and dropping it would score the plan as though the cfg had never named any. + """ + pattern = str(tmp_path / "nothing" / "*.json") + + with pytest.raises(ValueError, match="No inspection records matched"): + _expand(pattern) + + +@pytest.mark.parametrize( + ("content", "expected"), + [ + ('{hmac: {Normalized_Coverage: "82.5%"}}', 82.5), + ("{hmac: {Normalized_Coverage: 82.5}}", 82.5), + # A plan with no score annotated yet is not an error, it simply has none to report. + ("{hmac: {Description: nothing scored}}", None), + ("{}", None), + ("not hjson at all {{{", None), + ], + ids=["percent_string", "bare_number", "no_score", "empty", "malformed"], +) +def test_the_overall_score_is_read_back_or_reported_as_absent( + tmp_path: Path, content: str, expected: float | None +) -> None: + """The score is quoted in the flow's report, so an unreadable plan must not raise.""" + annotated = tmp_path / ANNOTATED_HJSON + annotated.write_text(content, encoding="utf-8") + + assert_that(overall_coverage(annotated), equal_to(expected)) + + +def test_a_missing_annotated_plan_reports_no_score(tmp_path: Path) -> None: + """Nothing was produced, so there is nothing to quote and nothing to raise about.""" + assert_that(overall_coverage(tmp_path / "absent.hjson"), is_(none())) + + +def test_a_missing_dvplan_is_decided_where_the_job_runs(tmp_path: Path) -> None: + """A checkout without dvplan must not fail every regression that names a vPlan. + + Testing PATH here would answer for the host dvsim was launched from, which on a compute farm + is not the host the job lands on, so the guard goes in the script and is checked there. + """ + command = shell_command(_inputs(tmp_path)) + + assert_that(command, contains_string("command -v dvplan")) + assert_that(command, contains_string("WARNING")) + assert_that(command, contains_string("exit 0")) + + +def test_the_command_fails_the_job_when_dvplan_does(tmp_path: Path) -> None: + """A broken annotation shows as a failed job rather than a silently missing score. + + `set -e` and the `&&` are what carry a non-zero exit out to the scheduler, so they are + checked rather than assumed. + """ + command = shell_command(_inputs(tmp_path)) + + assert_that(command, contains_string("set -e")) + assert_that(command, contains_string("prepare_vplan")) + assert_that(command, contains_string("&&")) + assert_that(command, contains_string("process_results")) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 8619c81a..a3b2f726 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -19,7 +19,7 @@ import pytest from hamcrest import assert_that, calling, empty, equal_to, only_contains, raises -from dvsim.job.data import CompletedJobStatus, JobSpec, WorkspaceConfig +from dvsim.job.data import CompletedJobStatus, DependencyPolicy, JobSpec, WorkspaceConfig from dvsim.job.status import JobStatus from dvsim.launcher.base import ErrorMessage, Launcher, LauncherBusyError, LauncherError from dvsim.report.data import IPMeta, ToolMeta @@ -302,7 +302,7 @@ def job_spec_factory( "resources": None, "seed": None, "dependencies": [], - "needs_all_dependencies_passing": True, + "dependency_policy": DependencyPolicy.ALL_PASSING, "weight": 1, "timeout_mins": None, "cmd": "echo 'test_cmd'", @@ -618,10 +618,10 @@ class TestSchedulingStructure: @staticmethod @pytest.mark.asyncio @pytest.mark.timeout(DEFAULT_TIMEOUT) - @pytest.mark.parametrize("needs_all_passing", [True, False]) - async def test_no_deps(fxt: Fxt, *, needs_all_passing: bool) -> None: + @pytest.mark.parametrize("policy", list(DependencyPolicy)) + async def test_no_deps(fxt: Fxt, policy: DependencyPolicy) -> None: """Tests scheduling of jobs without any listed dependencies.""" - job = job_spec_factory(fxt.tmp_path, needs_all_dependencies_passing=needs_all_passing) + job = job_spec_factory(fxt.tmp_path, dependency_policy=policy) result = await Scheduler([job], fxt.backends, MOCK_BACKEND).run() _assert_result_status(result, 1) @@ -630,14 +630,13 @@ async def _dep_test_case( fxt: Fxt, dep_list: dict[int, list[int]], passes: list[int], - *, - all_passing: bool, + policy: DependencyPolicy, ) -> None: """Run a simple dependency test, with 5 jobs where jobs 2 & 4 will fail.""" jobs = make_many_jobs( fxt.tmp_path, 5, - needs_all_dependencies_passing=all_passing, + dependency_policy=policy, interdeps=dep_list, ) fxt.mock_ctx.set_config(jobs[2], MockJob(default_status=JobStatus.FAILED)) @@ -672,7 +671,9 @@ async def test_needs_any_dep( passes: list[int], ) -> None: """Tests scheduling of jobs with dependencies that don't need all passing.""" - await TestSchedulingStructure._dep_test_case(fxt, dep_list, passes, all_passing=False) + await TestSchedulingStructure._dep_test_case( + fxt, dep_list, passes, DependencyPolicy.ANY_PASSING + ) @staticmethod @pytest.mark.asyncio @@ -694,7 +695,31 @@ async def test_needs_all_deps( passes: list[int], ) -> None: """Tests scheduling of jobs with dependencies that need all passing.""" - await TestSchedulingStructure._dep_test_case(fxt, dep_list, passes, all_passing=True) + await TestSchedulingStructure._dep_test_case( + fxt, dep_list, passes, DependencyPolicy.ALL_PASSING + ) + + @staticmethod + @pytest.mark.asyncio + @pytest.mark.timeout(DEFAULT_TIMEOUT) + @pytest.mark.parametrize( + ("dep_list", "passes"), + [ + # One failing dependency, which both of the other policies treat as a reason to skip + ({1: [2]}, [0, 1, 3]), + # Every dependency failed, which is the case a vPlan score most needs to describe + ({3: [2, 4]}, [0, 1, 3]), + # A mix, so a passing dependency is not what releases the job + ({0: [1, 2, 3, 4]}, [0, 1, 3]), + ], + ) + async def test_runs_whatever_the_deps_concluded( + fxt: Fxt, + dep_list: dict[int, list[int]], + passes: list[int], + ) -> None: + """Tests scheduling of jobs that only wait for their dependencies to be terminal.""" + await TestSchedulingStructure._dep_test_case(fxt, dep_list, passes, DependencyPolicy.ALWAYS) @staticmethod @pytest.mark.asyncio diff --git a/tests/utils/test_git.py b/tests/utils/test_git.py index 06bde5ce..5516fe22 100644 --- a/tests/utils/test_git.py +++ b/tests/utils/test_git.py @@ -132,6 +132,66 @@ def test_git_origin_url(tmp_path: Path) -> None: equal_to(url), ) + @staticmethod + @pytest.mark.parametrize( + ("url", "expected"), + [ + ( + "https://github.com/lowRISC/test.git", + "https://github.com/lowRISC/test.git", + ), + ("git@github.com:lowRISC/test.git", "git@github.com:lowRISC/test.git"), + # An ssh user is part of the address rather than a credential, so it stays. Dropping + # it would leave a url that no longer reaches the remote. + ( + "ssh://git@github.com/lowRISC/test.git", + "ssh://git@github.com/lowRISC/test.git", + ), + ( + "git+ssh://git@github.com/lowRISC/test.git", + "git+ssh://git@github.com/lowRISC/test.git", + ), + ( + "https://oauth2:ghs_secrettoken@github.com/lowRISC/test.git", + "https://github.com/lowRISC/test.git", + ), + ( + "https://someuser@github.com/lowRISC/test.git", + "https://github.com/lowRISC/test.git", + ), + # A password is a credential whatever the scheme carries it. + ( + "ssh://user:secretpw@github.com/lowRISC/test.git", + "ssh://github.com/lowRISC/test.git", + ), + ], + ids=[ + "plain_https", + "ssh_scp_form", + "ssh_url_form", + "git_ssh_scheme", + "token", + "user_only", + "ssh_with_password", + ], + ) + def test_git_origin_url_strips_credentials(tmp_path: Path, url: str, expected: str) -> None: + """A token in the remote url never reaches the caller, whatever the url's shape. + + The url is recorded in the run metadata and published in the reports, so a credential + left in it outlives the run. Both ssh forms carry an '@' without being credentialed, and + have to survive untouched or the recorded url stops addressing the remote. + """ + r = Repo.init(path=tmp_path) + + file = tmp_path / "a" + file.write_text("file to commit") + r.index.add([file]) + r.index.commit("initial commit") + r.create_remote("origin", url) + + assert_that(git_origin_url(tmp_path), equal_to(expected)) + @staticmethod @pytest.mark.parametrize( ("url", "expected"), @@ -141,6 +201,10 @@ def test_git_origin_url(tmp_path: Path) -> None: "https://github.com/lowRISC/test.git", "https://github.com/lowRISC/test/tree/{commit}", ), + ( + "https://oauth2:ghs_secrettoken@github.com/lowRISC/test.git", + "https://github.com/lowRISC/test/tree/{commit}", + ), ], ) def test_git_https_url_with_commit(tmp_path: Path, url: str, expected: str) -> None: