Skip to content

Commit 08614f0

Browse files
committed
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 <rswarbrick@lowrisc.org>
1 parent 0e33948 commit 08614f0

2 files changed

Lines changed: 30 additions & 37 deletions

File tree

src/dvsim/sim/flow.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,9 @@ def _create_objects(self) -> None:
330330
self.regressions.extend(self.testplan.get_stage_regressions())
331331
else:
332332
# Create a dummy testplan with no entries.
333-
self.testplan = Testplan(None, name=self.name)
333+
self.testplan = Testplan(
334+
"<dummy testplan>", repo_top=Path(self.proj_root), name=self.name
335+
)
334336

335337
# Create regressions
336338
self.regressions = Regression.create_regressions(self.regressions, self, self.tests)

src/dvsim/testplan.py

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@
44

55
"""Testpoint and Testplan classes for maintaining the testplan."""
66

7-
import os
87
import re
98
import sys
109
from collections import defaultdict
1110
from pathlib import Path
12-
from typing import TextIO
11+
from typing import Any, TextIO
1312

1413
import hjson
1514
from tabulate import tabulate
@@ -260,15 +259,9 @@ class Testplan:
260259
element_cls = {"testpoint": Testpoint, "covergroup": Covergroup}
261260

262261
@staticmethod
263-
def _parse_hjson(filename):
264-
"""Parses an input file with Hjson and returns a dict."""
265-
try:
266-
return hjson.load(Path(filename).open())
267-
except OSError:
268-
pass
269-
except hjson.scanner.HjsonDecodeError:
270-
pass
271-
sys.exit(1)
262+
def _parse_hjson(filename: Path) -> dict[str, Any]:
263+
"""Parse an Hjson file at the given path and return it as a dict."""
264+
return hjson.loads(Path(filename).read_text(encoding="utf-8"))
272265

273266
@staticmethod
274267
def _create_testplan_elements(kind: str, raw_dicts_list: list, tags: set) -> list[Element]:
@@ -311,36 +304,38 @@ def _get_percentage(value, total) -> str:
311304
perc = value / total * 100 * 1.0
312305
return f"{round(perc, 2):.2f} %"
313306

314-
def __init__(self, filename, repo_top=None, name=None) -> None:
307+
def __init__(self, tagged_filename: str, repo_top: Path, name: str) -> None:
315308
"""Initialize the testplan.
316309
317-
filename is the Hjson file that captures the testplan. It may be
318-
suffixed with tags separated with a colon delimiter to filter the
319-
testpoints. For example: path/too/foo_testplan.hjson:bar:baz
320-
repo_top is an optional argument indicating the path to top level repo
321-
/ project directory. It is used with filename arg.
322-
name is an optional argument indicating the name of the testplan / DUT.
323-
It overrides the name set in the testplan Hjson.
310+
Args:
311+
312+
tagged_filename: Describes the Hjson file that captures the testplan.
313+
This is a string, rather than a Path object, because
314+
it may be suffixed with tags separated with a colon
315+
delimiter to filter the testpoints.
316+
317+
For example: "path/to/foo_testplan.hjson:bar:baz"
318+
319+
repo_top: The path to the top level repo / project directory.
320+
This is combined with the filename argument.
321+
322+
name: The name of the testplan / DUT. It overrides any
323+
name set in the testplan Hjson.
324+
324325
"""
325-
self.name = None
326+
self.name = name
326327
self.testpoints = []
327328
self.covergroups = []
328329
self.test_results_mapped = False
329330

330331
# Split the filename into filename and tags, if provided.
331-
split = str(filename).split(":")
332+
split = tagged_filename.split(":")
332333
filename = Path(split[0])
333334
tags = set(split[1:])
334335

335336
if filename.exists():
336337
self._parse_testplan(filename, tags, repo_top)
337338

338-
if name:
339-
self.name = name
340-
341-
if not self.name:
342-
sys.exit(1)
343-
344339
# Represents current progress towards each stage. Stage = N.A.
345340
# is used to indicate the unmapped tests.
346341
self.progress = {}
@@ -402,7 +397,7 @@ def _get_imported_testplan_paths(
402397

403398
return result
404399

405-
def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
400+
def _parse_testplan(self, filename: Path, tags: set[str], repo_top: Path) -> None:
406401
"""Parse testplan Hjson file and create the testplan elements.
407402
408403
It creates the list of testpoints and covergroups extracted from the
@@ -411,10 +406,6 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
411406
filename is the path to the testplan file written in Hjson format.
412407
repo_top is an optional argument indicating the path to repo top.
413408
"""
414-
if repo_top is None:
415-
# Assume dvsim's original location: $REPO_TOP/util/dvsim.
416-
repo_top = Path(__file__).parent.parent.parent.resolve()
417-
418409
obj = Testplan._parse_hjson(filename)
419410

420411
parsed = set()
@@ -430,7 +421,7 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
430421
if testplan in parsed:
431422
sys.exit(1)
432423
parsed.add(testplan)
433-
data = self._parse_hjson(os.path.join(repo_top, testplan))
424+
data = self._parse_hjson(repo_top / testplan)
434425
imported_testplans.extend(
435426
self._get_imported_testplan_paths(
436427
testplan,
@@ -725,16 +716,16 @@ def get_test_results_summary(self):
725716
result["Pass Rate"] = self._get_percentage(tr.passing, tr.total)
726717
return result
727718

728-
def get_sim_results(self, sim_results_file, fmt="md"):
719+
def get_sim_results(self, sim_results_file: str, fmt="md"):
729720
"""Returns the mapped sim result tables in HTML formatted text.
730721
731-
The data extracted from the sim_results table HJson file is mapped into
722+
The data extracted from the sim_results table Hjson file is mapped into
732723
a test results, test progress, covergroup progress and coverage tables.
733724
734725
fmt is either 'md' (markdown) or 'html'.
735726
"""
736727
assert fmt in ["md", "html"]
737-
sim_results = Testplan._parse_hjson(sim_results_file)
728+
sim_results = Testplan._parse_hjson(Path(sim_results_file))
738729
test_results_ = sim_results.get("test_results", None)
739730

740731
test_results = []

0 commit comments

Comments
 (0)