Re: [PATCH 4/6] tests/qemu-iotests/meson.build: Call the 'check' script directly

[Paolo Bonzini]

On 2/8/22 11:13, Thomas Huth wrote:
> We can get a nicer progress indication if we add the iotests
> individually via the 'check' script instead of going through
> the check-block.sh wrapper.
>
> For this, we have to add some of the sanity checks that have
> originally been done in the tests/check-block.sh script (whether
> "bash" is available or whether CFLAGS contain -fsanitize switches)
> to the meson.build file now, and add the environment variables
> that have been set up by the tests/check-block.sh script before.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qemu-iotests/meson.build | 45 ++++++++++++++++++++++++++++------
>   1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index e1832c90e0..5a6ccd35d8 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -1,9 +1,29 @@
> -if not have_tools or targetos == 'windows'
> +if not have_tools or targetos == 'windows' or \
> +   config_host.has_key('CONFIG_GPROF')
>     subdir_done()
>   endif
>   
> +bash = find_program('bash', required: false)
> +if not bash.found() or \
> +   run_command(bash, ['--version']).stdout().contains('GNU bash, version 3')
> +  message('bash >= v4.0 not available ==> Disabled the qemu-iotests.')
> +  subdir_done()
> +endif
> +
> +foreach cflag: config_host['QEMU_CFLAGS'].split()
> +  if cflag.startswith('-fsanitize') and \
> +     not cflag.contains('safe-stack') and not cflag.contains('cfi-icall')
> +    message('Sanitizers are enabled ==> Disabled the qemu-iotests.')
> +    subdir_done()
> +  endif
> +endforeach
> +
>   qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
> -qemu_iotests_env = {'PYTHON': python.full_path()}
> +qemu_iotests_env = {
> +  'PYTHON': python.full_path(),
> +  'PYTHONUTF8': '1',
> +  'QEMU_CHECK_BLOCK_AUTO': '1'
> +}
>   qemu_iotests_formats = {
>     'qcow2': 'quick',
>     'raw': 'slow',
> @@ -18,16 +38,25 @@ foreach k, v : emulators
>     endif
>   endforeach
>   
> +check_script = find_program(meson.current_build_dir() / 'check')
> +iotests = run_command(python, [check_script.full_path(), '-g', 'auto', '-n'],
> +                      check: true).stdout().strip().replace('tests/', 
> '').split('\n')

The main disadvantage here is that changes to the test directory will 
not rebuild meson.build.  So when one creates a new test, meson has to 
be rerun manually.

It is probably possible to do something like:

- run "check -g auto -n" with configure_file() and store the output in a 
file

- in the Makefile, always run "check -g auto -n", like

check-block-test-list: check-block-test-list.stamp
check-block-test-list.stamp:
        ./check -g auto -n > check-block-test-list.stamp; \
        if cmp check-block-test-list.stamp check-block-test-list; then \
                cp check-block-test-list.stamp check-block-test-list; \
        fi
.PHONY: check-block-test-list.stamp

check check-block: check-block-test-list.stamp

before giving control to "meson test"; but I'm not sure if that will 
also cause a rebuild of Makefile.mtest and a meson rerun.

But I'm not sure it's worth it...

Regarding the issue that Peter pointed out, that's my fault for not 
being aware of the "diff" being in the output of failing tests.  There 
are three ways to fix it:

1) reverting my patches

2) Thomas's series, but only if the above issue is fixed

3) shipping the tests/qemu-iotests/*.bad files as artifacts

4) not using -tap (either reverting commit d316859f4e, "check-block: 
replace -makecheck with TAP output", or just removing -tap from 
tests/qemu-iotests/meson.build).

My preference is 4, then 1 or 2 (depending on the complexity), then 3.

Paolo

>   foreach format, speed: qemu_iotests_formats
>     if speed == 'quick'
>       suites = 'block'
>     else
>       suites = ['block-' + speed, speed]
>     endif
> -  test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), 
> format],
> -       depends: qemu_iotests_binaries, env: qemu_iotests_env,
> -       protocol: 'tap',
> -       suite: suites,
> -       timeout: 0,
> -       is_parallel: false)
> +  foreach tst: iotests
> +    test('iotest-' + format + '-' + tst,
> +         python, args: [check_script.full_path(), '-tap', '-' + format, tst],
> +         depends: qemu_iotests_binaries,
> +         env: qemu_iotests_env + \
> +              { 'TEST_DIR':
> +                meson.current_build_dir() / 'scratch' / format + '-' + tst },
> +         protocol: 'tap',
> +         suite: suites,
> +         timeout: 0)
> +  endforeach
>   endforeach