From 72416b8880c6d54ab62e33c70a3013222d35c508 Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Thu, 4 Jun 2026 13:39:59 +0100 Subject: [PATCH] refactor: Improve and document Testplan.get_stage_regressions This was prompted by trying to understand the function in order to document what it did. Changes to the code: - Sort the tests associated with a stage instead of using the default ordering that was presumably based on the way the set of tests got built up from testpoints. - Explicitly use Testpoint.stages for the class variable instead of tp.stages: I wasted five minutes trying to figure out what was going on here. - Avoid the "ms" name (the thing that is now called a development stage was called a milestone in OpenTitan until 2022: see d1aeac6). - Use setdefault instead of a defaultdict - Iterate over items rather than iterating over keys and doing a lookup each time. Signed-off-by: Rupert Swarbrick --- src/dvsim/testplan.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/dvsim/testplan.py b/src/dvsim/testplan.py index 6f14fc96..7f5ff904 100644 --- a/src/dvsim/testplan.py +++ b/src/dvsim/testplan.py @@ -6,7 +6,6 @@ import re import sys -from collections import defaultdict from collections.abc import Sequence from pathlib import Path from typing import Any, TextIO @@ -590,16 +589,25 @@ def _sort(self) -> None: self.testpoints.sort(key=lambda x: x.stage) self.covergroups.sort(key=lambda x: x.name) - def get_stage_regressions(self): - regressions = defaultdict(set) + def get_stage_regressions(self) -> list[dict[str, str | list[str]]]: + """Return the testpoints, grouped by verification stage. + + The returned value has a dict for each verification stage with + testpoints. This dict has two keys: name and tests. The name field + gives the name of the verification stage. The tests field is a list of + the names of tests associated with testpoints in the stage. + + """ + stage_to_tests: dict[str, set[str]] = {} + for tp in self.testpoints: - if tp.not_mapped: + if tp.not_mapped or tp.stage not in Testpoint.stages[1:]: continue - if tp.stage in tp.stages[1:]: - regressions[tp.stage].update({t for t in tp.tests if t}) - # Build regressions dict into a Hjson-like data structure - return [{"name": ms, "tests": list(regressions[ms])} for ms in regressions] + stage_to_tests.setdefault(tp.stage, set()).update(tp.tests) + + # Build stage_to_tests dict into a Hjson-like data structure + return [{"name": stage, "tests": sorted(tps)} for stage, tps in stage_to_tests.items()] def write_testplan_doc(self, output: TextIO) -> None: """Write testplan documentation in markdown from the Hjson testplan."""