Skip to content

Commit 98f7bc5

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 0a7cd7a commit 98f7bc5

2 files changed

Lines changed: 27 additions & 63 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: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
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
@@ -244,15 +243,9 @@ class Testplan:
244243
element_cls = {"testpoint": Testpoint, "covergroup": Covergroup}
245244

246245
@staticmethod
247-
def _parse_hjson(filename):
248-
"""Parses an input file with HJson and returns a dict."""
249-
try:
250-
return hjson.load(Path(filename).open())
251-
except OSError:
252-
pass
253-
except hjson.scanner.HjsonDecodeError:
254-
pass
255-
sys.exit(1)
246+
def _parse_hjson(filename: Path) -> dict:
247+
"""Parse an hjson file at the given path and return it as a dict."""
248+
return hjson.load(Path(filename).open())
256249

257250
@staticmethod
258251
def _create_testplan_elements(kind: str, raw_dicts_list: list, tags: set) -> list[Element]:
@@ -295,64 +288,37 @@ def _get_percentage(value, total) -> str:
295288
perc = value / total * 100 * 1.0
296289
return f"{round(perc, 2):.2f} %"
297290

298-
@staticmethod
299-
def get_dv_style_css() -> str:
300-
"""Returns text with HTML CSS style for a table."""
301-
return (
302-
"<style>\n"
303-
"table.dv {\n"
304-
" border: 1px solid black;\n"
305-
" border-collapse: collapse;\n"
306-
" width: 100%;\n"
307-
" text-align: left;\n"
308-
" vertical-align: middle;\n"
309-
" display: table;\n"
310-
" font-size: smaller;\n"
311-
"}\n"
312-
"table.dv th, td {\n"
313-
" border: 1px solid black;\n"
314-
"}\n"
315-
"</style>\n"
316-
)
317-
318-
def __str__(self) -> str:
319-
lines = [f"Name: {self.name}\n"]
320-
lines += ["Testpoints:"]
321-
lines += [f"{t}" for t in self.testpoints]
322-
lines += ["Covergroups:"]
323-
lines += [f"{c}" for c in self.covergroups]
324-
return "\n".join(lines)
325-
326-
def __init__(self, filename, repo_top=None, name=None) -> None:
291+
def __init__(self, filename: str, repo_top: Path, name: str) -> None:
327292
"""Initialize the testplan.
328293
329-
filename is the HJson file that captures the testplan. It may be
330-
suffixed with tags separated with a colon delimiter to filter the
331-
testpoints. For example: path/too/foo_testplan.hjson:bar:baz
332-
repo_top is an optional argument indicating the path to top level repo
333-
/ project directory. It is used with filename arg.
334-
name is an optional argument indicating the name of the testplan / DUT.
335-
It overrides the name set in the testplan HJson.
294+
Args:
295+
filename: Describes the HJson file that captures the testplan. This
296+
is a string, rather than a Path object, because it may be
297+
suffixed with tags separated with a colon delimiter to
298+
filter the testpoints.
299+
300+
For example: path/too/foo_testplan.hjson:bar:baz
301+
302+
repo_top: The path to the top level repo / project directory. This is
303+
combined with the filename argument.
304+
305+
name: The name of the testplan / DUT. It overrides any name set
306+
in the testplan HJson.
307+
336308
"""
337-
self.name = None
309+
self.name = name
338310
self.testpoints = []
339311
self.covergroups = []
340312
self.test_results_mapped = False
341313

342314
# Split the filename into filename and tags, if provided.
343-
split = str(filename).split(":")
315+
split = filename.split(":")
344316
filename = Path(split[0])
345317
tags = set(split[1:])
346318

347319
if filename.exists():
348320
self._parse_testplan(filename, tags, repo_top)
349321

350-
if name:
351-
self.name = name
352-
353-
if not self.name:
354-
sys.exit(1)
355-
356322
# Represents current progress towards each stage. Stage = N.A.
357323
# is used to indicate the unmapped tests.
358324
self.progress = {}
@@ -414,7 +380,7 @@ def _get_imported_testplan_paths(
414380

415381
return result
416382

417-
def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
383+
def _parse_testplan(self, filename: Path, tags: set[str], repo_top: Path) -> None:
418384
"""Parse testplan Hjson file and create the testplan elements.
419385
420386
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:
423389
filename is the path to the testplan file written in HJson format.
424390
repo_top is an optional argument indicating the path to repo top.
425391
"""
426-
if repo_top is None:
427-
# Assume dvsim's original location: $REPO_TOP/util/dvsim.
428-
repo_top = Path(__file__).parent.parent.parent.resolve()
429-
430392
obj = Testplan._parse_hjson(filename)
431393

432394
parsed = set()
@@ -442,7 +404,7 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
442404
if testplan in parsed:
443405
sys.exit(1)
444406
parsed.add(testplan)
445-
data = self._parse_hjson(os.path.join(repo_top, testplan))
407+
data = self._parse_hjson(repo_top / testplan)
446408
imported_testplans.extend(
447409
self._get_imported_testplan_paths(
448410
testplan,
@@ -737,7 +699,7 @@ def get_test_results_summary(self):
737699
result["Pass Rate"] = self._get_percentage(tr.passing, tr.total)
738700
return result
739701

740-
def get_sim_results(self, sim_results_file, fmt="md"):
702+
def get_sim_results(self, sim_results_file: str, fmt="md"):
741703
"""Returns the mapped sim result tables in HTML formatted text.
742704
743705
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"):
746708
fmt is either 'md' (markdown) or 'html'.
747709
"""
748710
assert fmt in ["md", "html"]
749-
sim_results = Testplan._parse_hjson(sim_results_file)
711+
sim_results = Testplan._parse_hjson(Path(sim_results_file))
750712
test_results_ = sim_results.get("test_results", None)
751713

752714
test_results = []

0 commit comments

Comments
 (0)