Re: [PATCH v3 3/4] dts: add per-test-suite configuration

[Luca Vizzarro <Luca.Vizzarro@arm.com>]

Hi Dean!

Thank you for your review!

On 13/11/2024 21:44, Dean Marx wrote:
> I like the idea of mapping the suite to specific test cases, and for
> the most part the custom configuration option as well. The only
> thing that I feel should be different is the way the code generation
> is documented, I think it might be worth providing an example within
> conf.yaml through a comment near the suites section, rather than
> just in the dts.rst file. It might be a little more clear where to
> create the custom config class as well.

So... this is a weird one. I've integrated the code generation under 
dts-check-format.sh, so nobody should really worry about it, as that 
should be run before submitting. If it's not, then the CI will 
eventually pick it up when dts-check-format will be in place.

>> +class HelloWorldConfig(TestSuiteConfig):
>> +    """Example custom configuration for the `TestHelloWorld`
>> test suite."""
>> +
>> +    #: Timeout for the DPDK apps.
>> +    timeout: int = 50
>> --
>> 2.43.0
> 
> Additionally, I was a bit confused by the custom config examples, do
> these fields (timeout, my_custom_field) actually affect the suite in
> any way as of this patch? Or is this just so that we can potentially
> add configuration options through this method in the future?

The hello_world suite doesn't really need this, I've just added it as an 
example. But yes test suites are affected. If you see the hello_world 
suite, it calls `self.config.timeout` which is a reference to this 
field. This class specifically is now wholly integrated in the 
configuration:

   test_runs:
   - test_suites:
       hello_world:  # this is a mapping to HelloWorldConfig
         # like before the behavior of this field is:
	# - if unset or empty list, all test cases are run
         # - if individual test cases are listed, those are run
         test_cases: [single_core] # field inherited from TestSuiteConfig
         timeout: 20               # optional field from HelloWorldConfig

What I've added for the simplicity is to be able to represent any 
TestSuiteConfig as a string instead of a mapping:

   test_runs:
   - test_suites:
       hello_world: all # special keyword which enables all test cases
       # or
       hello_world: single_core all_cores # a whitespace-delimited list 
of test suites

Mind that if the underlying test suite configuration has a mandatory 
field that has no default, then the string representation will fail and 
Pydantic will complain that the (required) field is missing.

Please let me know if I answered every query, and don't hesitate to ask 
for further clarifications.

Best,
Luca