refactor: Tidy up how wildcards get expanded in testpoints#221
Conversation
rswarbrick
force-pushed
the
wildcard-expand
branch
from
0dcf943 to
a0dafb9
Compare
|
|
||
| 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-']. |
There was a problem hiding this comment.
Aside: the original implementation feels unintuitive - I feel like most similar systems would retain one copy of the unsubstituted text (i.e. just leave it as ['test-{a}']).
There was a problem hiding this comment.
I'm not completely sure, but this might be an intentional feature. It also guarantees that test names don't have stray curly braces in them at the end.
I think any improvement to this would probably need to be separate work.
| ret.extend(Testpoint._expand_str(pattern, new_vals, txt)) | ||
| return ret | ||
|
|
||
| def do_substitutions(self, substitutions: dict, reserved_names: Sequence[str]) -> None: |
There was a problem hiding this comment.
Can we make this substitutions: dict[str, str | list[str]] or is this too strong of a typing to enforce here at this stage? If not, at least substitutions: dict[str, Any]?
There was a problem hiding this comment.
Because the dictionary fundamentally comes from a parsed hjson file, I think we'll need the weaker type. But that's probably better than what I had.
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 <rswarbrick@lowrisc.org>
rswarbrick
force-pushed
the
wildcard-expand
branch
from
a0dafb9 to
8195af2
Compare
2 participants
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.