Repository navigation
[uss_qualifier/suites] Improve test suite resource input definitions - #1821
BenjaminPelletier wants to merge 2 commits into
Conversation
Shouldn't we mark those as generated in gitattributes, so the diff is not shown by default in PR, but still expandable if needed? (If yes, probably worth and issue since it's shouldn't be fixed there) |
| resource_inputs: Optional[list[ResourceInput]] | ||
| """Enumeration of the resources used by this test suite. May not be specified together with `resources`.""" | ||
|
|
||
| local_resources: Optional[dict[ResourceID, ResourceDeclaration]] |
There was a problem hiding this comment.
Shouldn't that one be updated?
(At least the doc, but probably the behavior no?)
There was a problem hiding this comment.
That would be an entirely different PR as this PR only deals with what are effectively function arguments to a test suite whereas this line (local_resources) is effectively local variable assignments. A ResourceDeclaration is very different from a resource type name.
| raise ValueError( | ||
| "May not specify both resources and resource_inputs in the same TestSuiteDefinition" | ||
| ) | ||
| super().__init__(previous_instance, **kwargs) |
There was a problem hiding this comment.
Since it was a dict before, it's was (probably) not possible to set the same id twice. Shouldn't we have a check to prevent that?
| supplied_fields = (previous_instance or {}).keys() | kwargs.keys() | ||
| if "resources" in supplied_fields and "resource_inputs" in supplied_fields: | ||
| raise ValueError( | ||
| "May not specify both resources and resource_inputs in the same TestSuiteDefinition" |
There was a problem hiding this comment.
nit: now it's possible to not define anything since both are optional, witch is probably fine but just to be sure should it be checked?
This branch has not been deployed
2 participants
This PR enhances the information that can be provided about resource inputs to a test suite. Instead of a dict with just resource type as the value, a list of resource inputs is added. In general, lists of items are better than dicts with arbitrary keys because the latter is difficult to deal with in a number of schema-checking activities, as I learned with ResourceCollection.resource_declarations in configuration. The former is also easier to expand with additional pieces of information later. The old
resourcesfield remains supported to allow leisurely upgrading toresource_inputs, and indeed this PR only uses the new field in one of the test suites. A description field for the test suite overall is also added and rendered into documentation.The size of this PR is smaller than it looks initially; only the first commit is substantive, and then the second commit is just
make formatto rerender the auto-generated test suite documentation.