From a8684aa7c378bcaf6573e69d4ab1659126ba5e7b Mon Sep 17 00:00:00 2001
From: Rupert Swarbrick <rswarbrick@lowrisc.org>
Date: Thu, 21 May 2026 12:06:53 +0100
Subject: [PATCH] refactor: Collect tps/cgs with tighter types in testplan.py

There's no functional change, but this avoids needing to pass around
constructors. As a result, the inferred types for Testplan.testpoints
and Testpaln.covergroups are tighter, meaning that some following code
is now well-typed.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
---
 src/dvsim/testplan.py | 61 +++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 37 deletions(-)

diff --git a/src/dvsim/testplan.py b/src/dvsim/testplan.py
index dee1a95a..10b29dbd 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
 
@@ -241,7 +242,6 @@ class Testplan:
     """
 
     rsvd_keywords = ["import_testplans", "testpoints", "covergroups"]
-    element_cls = {"testpoint": Testpoint, "covergroup": Covergroup}
 
     @staticmethod
     def _parse_hjson(filename):
@@ -255,37 +255,21 @@ def _parse_hjson(filename):
         sys.exit(1)
 
     @staticmethod
-    def _create_testplan_elements(kind: str, raw_dicts_list: list, tags: set) -> list[Element]:
-        """Create testplan elements from the list of raw dicts.
+    def _check_duplicates(kind: str, elements: Sequence[Element]) -> None:
+        """Check that there aren't multiple elements in the list that share a name.
 
-        kind is either 'testpoint' or 'covergroup'.
-        raw_dicts_list is a list of dictionaries extracted from the HJson file.
-        """
-        items = []
-        item_names = set()
+        Args:
+          kind: A string describing the type of object (for use in error messages)
 
-        try:
-            elem_cls = Testplan.element_cls[kind]
-        except KeyError:
-            err_msg = f"No known class for kind={kind}"
-            raise RuntimeError(err_msg) from None
-
-        for dict_entry in raw_dicts_list:
-            try:
-                item = elem_cls(dict_entry)
-            except (KeyError, ValueError) as e:
-                err_msg = f"Failed to create {kind} from dictionary."
-                raise RuntimeError(err_msg) from e
+          elements: The list of elements to check for duplicates
 
-            if item.name in item_names:
-                err_msg = f"Duplicate items with name {item.name}."
-                raise ValueError(err_msg)
-
-            # Filter out the item by tags if provided.
-            if item.has_tags(tags):
-                items.append(item)
-                item_names.add(item.name)
-        return items
+        """
+        known_names: set[str] = set()
+        for elem in elements:
+            if elem.name in known_names:
+                msg = f"Duplicate {kind} items with name {elem.name}"
+                raise ValueError(msg)
+            known_names.add(elem.name)
 
     @staticmethod
     def _get_percentage(value, total) -> str:
@@ -335,8 +319,8 @@ def __init__(self, filename, repo_top=None, name=None) -> None:
         It overrides the name set in the testplan HJson.
         """
         self.name = None
-        self.testpoints = []
-        self.covergroups = []
+        self.testpoints: list[Testpoint] = []
+        self.covergroups: list[Covergroup] = []
         self.test_results_mapped = False
 
         # Split the filename into filename and tags, if provided.
@@ -454,14 +438,17 @@ def _parse_testplan(self, filename: Path, tags: set, repo_top=None) -> None:
 
         self.name = obj.get("name")
 
-        testpoints = obj.get("testpoints", [])
-        self.testpoints = self._create_testplan_elements("testpoint", testpoints, tags)
+        all_tps = [Testpoint(t) for t in obj.get("testpoints", [])]
+        all_cgs = [Covergroup(cg) for cg in obj.get("covergroups", [])]
 
-        covergroups = obj.get("covergroups", [])
-        self.covergroups = self._create_testplan_elements("covergroup", covergroups, set())
+        if not (all_tps or all_cgs):
+            raise ValueError("Merged testplan doesn't contain any testpoints or covergroups")
 
-        if not testpoints and not covergroups:
-            sys.exit(1)
+        Testplan._check_duplicates("testpoint", all_tps)
+        Testplan._check_duplicates("covergroup", all_cgs)
+
+        self.testpoints = [t for t in all_tps if t.has_tags(tags)]
+        self.covergroups = [cg for cg in all_cgs if cg.has_tags(tags)]
 
         # Any variable in the testplan that is not a recognized HJson field can
         # be used as a substitution variable.