From 98f7bc5f9f281201729d4368aeb4f83c9929a4f6 Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Thu, 21 May 2026 13:21:20 +0100 Subject: [PATCH] refactor: Remove outdated (and unused) default repo_top argument This code was based on dvsim living inside the OpenTitan repository. Fortunately, it is never used: we always pass a third argument to _parse_testplan. Also, fail more understandably if there is an hjson parse error or an invalid filename passed to _parse_hjson. Finally, simplify the Testplan constructor so that it always expects a name for its testplan (rather than exiting the program silently if there isn't one). We also change that argument to explicitly have type str (as opposed to Path) and explain why: the string can have suffix values that filter the set of tests. Signed-off-by: Rupert Swarbrick --- src/dvsim/sim/flow.py | 4 +- src/dvsim/testplan.py | 86 ++++++++++++------------------------------- 2 files changed, 27 insertions(+), 63 deletions(-) diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index bbd907a0..8c8536b6 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -330,7 +330,9 @@ def _create_objects(self) -> None: self.regressions.extend(self.testplan.get_stage_regressions()) else: # Create a dummy testplan with no entries. - self.testplan = Testplan(None, name=self.name) + self.testplan = Testplan( + "", repo_top=Path(self.proj_root), name=self.name + ) # Create regressions self.regressions = Regression.create_regressions(self.regressions, self, self.tests) diff --git a/src/dvsim/testplan.py b/src/dvsim/testplan.py index dee1a95a..a6fe1b3b 100644 --- a/src/dvsim/testplan.py +++ b/src/dvsim/testplan.py @@ -4,7 +4,6 @@ """Testpoint and Testplan classes for maintaining the testplan.""" -import os import re import sys from collections import defaultdict @@ -244,15 +243,9 @@ class Testplan: element_cls = {"testpoint": Testpoint, "covergroup": Covergroup} @staticmethod - def _parse_hjson(filename): - """Parses an input file with HJson and returns a dict.""" - try: - return hjson.load(Path(filename).open()) - except OSError: - pass - except hjson.scanner.HjsonDecodeError: - pass - sys.exit(1) + def _parse_hjson(filename: Path) -> dict: + """Parse an hjson file at the given path and return it as a dict.""" + return hjson.load(Path(filename).open()) @staticmethod def _create_testplan_elements(kind: str, raw_dicts_list: list, tags: set) -> list[Element]: @@ -295,64 +288,37 @@ def _get_percentage(value, total) -> str: perc = value / total * 100 * 1.0 return f"{round(perc, 2):.2f} %" - @staticmethod - def get_dv_style_css() -> str: - """Returns text with HTML CSS style for a table.""" - return ( - "\n" - ) - - def __str__(self) -> str: - lines = [f"Name: {self.name}\n"] - lines += ["Testpoints:"] - lines += [f"{t}" for t in self.testpoints] - lines += ["Covergroups:"] - lines += [f"{c}" for c in self.covergroups] - return "\n".join(lines) - - def __init__(self, filename, repo_top=None, name=None) -> None: + def __init__(self, filename: str, repo_top: Path, name: str) -> None: """Initialize the testplan. - filename is the HJson file that captures the testplan. It may be - suffixed with tags separated with a colon delimiter to filter the - testpoints. For example: path/too/foo_testplan.hjson:bar:baz - repo_top is an optional argument indicating the path to top level repo - / project directory. It is used with filename arg. - name is an optional argument indicating the name of the testplan / DUT. - It overrides the name set in the testplan HJson. + Args: + filename: Describes the HJson file that captures the testplan. This + is a string, rather than a Path object, because it may be + suffixed with tags separated with a colon delimiter to + filter the testpoints. + + For example: path/too/foo_testplan.hjson:bar:baz + + repo_top: The path to the top level repo / project directory. This is + combined with the filename argument. + + name: The name of the testplan / DUT. It overrides any name set + in the testplan HJson. + """ - self.name = None + self.name = name self.testpoints = [] self.covergroups = [] self.test_results_mapped = False # Split the filename into filename and tags, if provided. - split = str(filename).split(":") + split = filename.split(":") filename = Path(split[0]) tags = set(split[1:]) if filename.exists(): self._parse_testplan(filename, tags, repo_top) - if name: - self.name = name - - if not self.name: - sys.exit(1) - # Represents current progress towards each stage. Stage = N.A. # is used to indicate the unmapped tests. self.progress = {} @@ -414,7 +380,7 @@ def _get_imported_testplan_paths( return result - def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None: + def _parse_testplan(self, filename: Path, tags: set[str], repo_top: Path) -> None: """Parse testplan Hjson file and create the testplan elements. It creates the list of testpoints and covergroups extracted from the @@ -423,10 +389,6 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None: filename is the path to the testplan file written in HJson format. repo_top is an optional argument indicating the path to repo top. """ - if repo_top is None: - # Assume dvsim's original location: $REPO_TOP/util/dvsim. - repo_top = Path(__file__).parent.parent.parent.resolve() - obj = Testplan._parse_hjson(filename) parsed = set() @@ -442,7 +404,7 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None: if testplan in parsed: sys.exit(1) parsed.add(testplan) - data = self._parse_hjson(os.path.join(repo_top, testplan)) + data = self._parse_hjson(repo_top / testplan) imported_testplans.extend( self._get_imported_testplan_paths( testplan, @@ -737,7 +699,7 @@ def get_test_results_summary(self): result["Pass Rate"] = self._get_percentage(tr.passing, tr.total) return result - def get_sim_results(self, sim_results_file, fmt="md"): + def get_sim_results(self, sim_results_file: str, fmt="md"): """Returns the mapped sim result tables in HTML formatted text. The data extracted from the sim_results table HJson file is mapped into @@ -746,7 +708,7 @@ def get_sim_results(self, sim_results_file, fmt="md"): fmt is either 'md' (markdown) or 'html'. """ assert fmt in ["md", "html"] - sim_results = Testplan._parse_hjson(sim_results_file) + sim_results = Testplan._parse_hjson(Path(sim_results_file)) test_results_ = sim_results.get("test_results", None) test_results = []