From a0dafb9ad13c1de67c5c68895520dddd6321a9dc Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Thu, 21 May 2026 10:30:39 +0100 Subject: [PATCH] refactor: Tidy up how wildcards get expanded in testpoints I think the existing implementation had a bit of a problem if you had multiple wildcard patterns that all expanded to lists of values. Presumably no-one ever cared, but it also wasn't very easy to see how the code worked. Signed-off-by: Rupert Swarbrick --- src/dvsim/testplan.py | 109 +++++++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 23 deletions(-) diff --git a/src/dvsim/testplan.py b/src/dvsim/testplan.py index dee1a95a..b37b1efa 100644 --- a/src/dvsim/testplan.py +++ b/src/dvsim/testplan.py @@ -8,6 +8,7 @@ import re import sys from collections import defaultdict +from collections.abc import Sequence from pathlib import Path from typing import TextIO @@ -173,31 +174,87 @@ def _validate(self) -> None: msg = f"'tests' key in {self} is not a list." raise ValueError(msg) - def do_substitutions(self, substitutions) -> None: + @staticmethod + def _expand_str(pattern: str, new_vals: Sequence[str], txt: str) -> list[str]: + """Return a list of copies of txt with each expansion of pattern. + + Note that if txt has multiple copies of the pattern then they are + replaced "diagonally". If pattern is "x", new_vals is ["0", "1"] and + txt is "x x" then the result is ["0 0", "1 1"]. + + Args: + pattern: A string for which we should search. + + new_vals: A list of values to use as replacements. + + txt: The string in which to replace the given pattern. + + """ + return [txt.replace(pattern, v) for v in new_vals] + + @staticmethod + def _expand_str_list(pattern: str, new_vals: Sequence[str], txt_list: list[str]) -> list[str]: + """Return a each expansion of pattern across elements of txt_list. + + This works by calling _expand_str to expand pattern for each element of + txt_list, collecting the list of results. + + Args: + pattern: A string for which we should search. + + new_vals: A list of values to use as replacements. + + txt_list: List of strings to expand + + """ + ret: list[str] = [] + for txt in txt_list: + ret.extend(Testpoint._expand_str(pattern, new_vals, txt)) + return ret + + def do_substitutions(self, substitutions: dict, reserved_names: Sequence[str]) -> None: """Substitute {wildcards} in tests. - If tests have {wildcards}, they are substituted with the 'correct' - values using the key=value pairs provided by the substitutions arg. - Wildcards with no substitution arg are replaced by an empty string. + If tests have wildcards, they are substituted using key=value pairs + from the substitutions argument. + + If a value in the substitutions argument is a list, the substitution + will make multiple testpoints from a templated value. For example, if + substitutions is {'a': ['x', 'y']} and self.tests is ['test-{a}'] then + self.tests will become ['test-x', 'test-y']. + + A wildcard use with no match in substitutions will be replaced by an + empty string (in a single item). For example, if substitutions is {} + and self.tests is ['test-{a}'] then self.tests will become ['test-']. - substitutions is a dictionary of wildcard-replacement pairs. """ resolved_tests = [] for test in self.tests: - match = re.findall(r"{([A-Za-z0-9\_]+)}", test) - if not match: - resolved_tests.append(test) - continue + # Iterate over all the wildcards found in the test name, creating a + # map from wildcard string ("{my_wildcard}") to a list of strings. + wildcard_expansions: dict[str, list[str]] = {} + for key in re.findall(r"{([A-Za-z0-9\_]+)}", test): + if key in reserved_names: + msg = f"Wildcard using reserved name, {key}." + raise ValueError(msg) + + raw_value = substitutions.get(key, [""]) + if isinstance(raw_value, list): + value = raw_value + elif isinstance(raw_value, str): + value = [raw_value] + else: + msg = f"Unknown type to replace wildcard {key}. Value was {raw_value}." + raise TypeError(msg) + + wildcard_expansions[f"{{{key}}}"] = value - # 'match' is a list of wildcards used in the test. Get their - # corresponding values. - subst = {item: substitutions.get(item, "") for item in match} + # Iterate over the expansions, applying each in turn. + expanded = [test] + for pattern, values in wildcard_expansions.items(): + expanded = Testpoint._expand_str_list(pattern, values, expanded) - resolved = [test] - for item, value in subst.items(): - values = value if isinstance(value, list) else [value] - resolved = [t.replace(f"{{{item}}}", v) for t in resolved for v in values] - resolved_tests.extend(resolved) + resolved_tests.extend(expanded) self.tests = resolved_tests @@ -240,7 +297,6 @@ class Testplan: The list of Testpoints and Covergroups make up the testplan. """ - rsvd_keywords = ["import_testplans", "testpoints", "covergroups"] element_cls = {"testpoint": Testpoint, "covergroup": Covergroup} @staticmethod @@ -345,7 +401,11 @@ def __init__(self, filename, repo_top=None, name=None) -> None: tags = set(split[1:]) if filename.exists(): - self._parse_testplan(filename, tags, repo_top) + try: + self._parse_testplan(filename, tags, repo_top) + except Exception as e: + msg = f"Error when parsing testplan at {filename}." + raise RuntimeError(msg) from e if name: self.name = name @@ -463,11 +523,14 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None: if not testpoints and not covergroups: sys.exit(1) - # Any variable in the testplan that is not a recognized HJson field can - # be used as a substitution variable. - substitutions = {k: v for k, v in obj.items() if k not in self.rsvd_keywords} + # Wildcards in testpoints can mostly point to any value from the + # object. The following names are *not* allowed (because they wouldn't + # really make much sense). + reserved_names = ("import_testplans", "testpoints", "covergroups") + + # Apply wildcards in each testpoint for tp in self.testpoints: - tp.do_substitutions(substitutions) + tp.do_substitutions(obj, reserved_names) self._sort()