Re: [PATCH 12/12] [NOT FOR MERGE] tests/qtest: Introduce qtest_validate_args

[Thomas Huth]

On 06/02/2023 16.04, Fabiano Rosas wrote:
> The QEMU binary can be built with a varied set of features/devices
> which are opaque to the tests. Add a centralized point for parsing and
> validating the command line.
>
> Tests can now be skipped with the following pattern:
>
> qts = qtest_init(args);
> if (!qts) {
>      return;
> }
>
> For now, the only validation is that the -device options all
> correspond to devices that are actually present in the build.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> Would this be better than checking for missing devices in individual
> tests?

This is certainly an interesting idea! ... some things still bug me, though:

- We still need to change all the calling sites (to check for
  !qts) ... so the effort seems to be in a similar ballpark as
  adding qtest_has_device() to the various problematic tests

- This will now call qtest_has_device for each and every device
  in the parameter list, even if it is not necessary. And at
  least the first call to qtest_has_device() is rather expensive
  since it has to fire up a separate QEMU to retrieve the list
  of supported the devices. So adding this to all tests might
  cause a slow-down to the tests...

- It could maybe even hide bugs if you don't look closely, e.g.
  if you have a typo in the device name in a test, the test then
  gets skipped automatically instead of failing ... ok, that's
  unlikely for new tests where you look closely, but still, it
  gives me slightly bad feeling.

So I think I rather tend to go for explicit calls to qtest_has_device() as 
you did in your first 11 patches.

Anyway, I'm interested in what do others think of this? Any other opinions?

 Thomas