Skip to content

Commit 7ed0a97

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 da12a19 commit 7ed0a97

2 files changed

Lines changed: 29 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: 26 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,37 @@ 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+
tagged_filename: Describes the Hjson file that captures the testplan.
312+
This is a string, rather than a Path object, because
313+
it may be suffixed with tags separated with a colon
314+
delimiter to filter the testpoints.
315+
316+
For example: "path/to/foo_testplan.hjson:bar:baz"
317+
318+
repo_top: The path to the top level repo / project directory.
319+
This is combined with the filename argument.
320+
321+
name: The name of the testplan / DUT. It overrides any
322+
name set in the testplan Hjson.
323+
324324
"""
325-
self.name = None
325+
self.name = name
326326
self.testpoints = []
327327
self.covergroups = []
328328
self.test_results_mapped = False
329329

330330
# Split the filename into filename and tags, if provided.
331-
split = str(filename).split(":")
331+
split = tagged_filename.split(":")
332332
filename = Path(split[0])
333333
tags = set(split[1:])
334334

335335
if filename.exists():
336336
self._parse_testplan(filename, tags, repo_top)
337337

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

403397
return result
404398

405-
def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
399+
def _parse_testplan(self, filename: Path, tags: set[str], repo_top: Path) -> None:
406400
"""Parse testplan Hjson file and create the testplan elements.
407401
408402
It creates the list of testpoints and covergroups extracted from the
@@ -411,10 +405,6 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
411405
filename is the path to the testplan file written in Hjson format.
412406
repo_top is an optional argument indicating the path to repo top.
413407
"""
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-
418408
obj = Testplan._parse_hjson(filename)
419409

420410
parsed = set()
@@ -430,7 +420,7 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
430420
if testplan in parsed:
431421
sys.exit(1)
432422
parsed.add(testplan)
433-
data = self._parse_hjson(os.path.join(repo_top, testplan))
423+
data = self._parse_hjson(repo_top / testplan)
434424
imported_testplans.extend(
435425
self._get_imported_testplan_paths(
436426
testplan,
@@ -725,16 +715,16 @@ def get_test_results_summary(self):
725715
result["Pass Rate"] = self._get_percentage(tr.passing, tr.total)
726716
return result
727717

728-
def get_sim_results(self, sim_results_file, fmt="md"):
718+
def get_sim_results(self, sim_results_file: str, fmt="md"):
729719
"""Returns the mapped sim result tables in HTML formatted text.
730720
731-
The data extracted from the sim_results table HJson file is mapped into
721+
The data extracted from the sim_results table Hjson file is mapped into
732722
a test results, test progress, covergroup progress and coverage tables.
733723
734724
fmt is either 'md' (markdown) or 'html'.
735725
"""
736726
assert fmt in ["md", "html"]
737-
sim_results = Testplan._parse_hjson(sim_results_file)
727+
sim_results = Testplan._parse_hjson(Path(sim_results_file))
738728
test_results_ = sim_results.get("test_results", None)
739729

740730
test_results = []

0 commit comments

Comments
 (0)