[PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Thomas Huth]

If there is a failing iotest, the output is currently not logged to
the console anymore. To get this working again, we need to run the
meson test runner with "--print-errorlogs" (and without "--verbose"
due to a current meson bug that will be fixed here:
https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
We could update the "meson test" call in tests/Makefile.include,
but actually it's nicer and easier if we simply do not treat the
iotests as separate test target anymore and integrate them along
with the other test suites. This has the disadvantage of not getting
the detailed progress indication there anymore, but since that was
only working right in single-threaded "make -j1" mode anyway, it's
not a huge loss right now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v4: updated commit description

 meson.build            | 6 +++---
 scripts/mtest2make.py  | 4 ----
 tests/Makefile.include | 9 +--------
 3 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/meson.build b/meson.build
index 2d6601467f..4f5b9f5ec5 100644
--- a/meson.build
+++ b/meson.build
@@ -3,9 +3,9 @@ project('qemu', ['c'], meson_version: '>=0.59.3',
                           'b_staticpic=false', 'stdsplit=false'],
         version: files('VERSION'))
 
-add_test_setup('quick', exclude_suites: ['block', 'slow', 'thorough'], is_default: true)
-add_test_setup('slow', exclude_suites: ['block', 'thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
-add_test_setup('thorough', exclude_suites: ['block'], env: ['G_TEST_SLOW=1', 'SPEED=thorough'])
+add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
+add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
+add_test_setup('thorough', env: ['G_TEST_SLOW=1', 'SPEED=thorough'])
 
 not_found = dependency('', required: false)
 keyval = import('keyval')
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index 4d542e8aaa..304634b71e 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -101,10 +101,6 @@ def emit_suite(name, suite, prefix):
 testsuites = defaultdict(Suite)
 for test in introspect['tests']:
     process_tests(test, targets, testsuites)
-# HACK: check-block is a separate target so that it runs with --verbose;
-# only write the dependencies
-emit_suite_deps('block', testsuites['block'], 'check')
-del testsuites['block']
 emit_prolog(testsuites, 'check')
 for name, suite in testsuites.items():
     emit_suite(name, suite, 'check')
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e7153c8e91..b89018cdcc 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -147,16 +147,9 @@ check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
 # Consolidated targets
 
-.PHONY: check-block check check-clean get-vm-images
+.PHONY: check check-clean get-vm-images
 check:
 
-ifneq ($(.check-block.deps),)
-check: check-block
-check-block: run-ninja
-	$(if $(MAKE.n),,+)$(MESON) test $(MTESTARGS) $(.mtestargs) --verbose \
-		--logbase iotestslog $(call .speed.$(SPEED), block block-slow block-thorough)
-endif
-
 check-build: run-ninja
 
 check-clean:
-- 
2.27.0

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Hanna Reitz]

On 10.03.22 08:50, Thomas Huth wrote:
> If there is a failing iotest, the output is currently not logged to
> the console anymore. To get this working again, we need to run the
> meson test runner with "--print-errorlogs" (and without "--verbose"
> due to a current meson bug that will be fixed here:
> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
> We could update the "meson test" call in tests/Makefile.include,
> but actually it's nicer and easier if we simply do not treat the
> iotests as separate test target anymore and integrate them along
> with the other test suites. This has the disadvantage of not getting
> the detailed progress indication there anymore, but since that was
> only working right in single-threaded "make -j1" mode anyway, it's
> not a huge loss right now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   v4: updated commit description
>
>   meson.build            | 6 +++---
>   scripts/mtest2make.py  | 4 ----
>   tests/Makefile.include | 9 +--------
>   3 files changed, 4 insertions(+), 15 deletions(-)

I can’t really say I understand what’s going on in this patch and around 
it, but I can confirm that it before this patch, fail diffs aren’t 
printed; but afterwards, they are.  So I’m afraid all I can give is a

Tested-by: Hanna Reitz <hreitz@redhat.com>

If noone else steps up and you need a tree for this to go in, I’d be up 
for it.

Hanna

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Thomas Huth]

On 18/03/2022 18.04, Hanna Reitz wrote:
> On 10.03.22 08:50, Thomas Huth wrote:
>> If there is a failing iotest, the output is currently not logged to
>> the console anymore. To get this working again, we need to run the
>> meson test runner with "--print-errorlogs" (and without "--verbose"
>> due to a current meson bug that will be fixed here:
>> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
>> We could update the "meson test" call in tests/Makefile.include,
>> but actually it's nicer and easier if we simply do not treat the
>> iotests as separate test target anymore and integrate them along
>> with the other test suites. This has the disadvantage of not getting
>> the detailed progress indication there anymore, but since that was
>> only working right in single-threaded "make -j1" mode anyway, it's
>> not a huge loss right now.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   v4: updated commit description
>>
>>   meson.build            | 6 +++---
>>   scripts/mtest2make.py  | 4 ----
>>   tests/Makefile.include | 9 +--------
>>   3 files changed, 4 insertions(+), 15 deletions(-)
> 
> I can’t really say I understand what’s going on in this patch and around it, 
> but I can confirm that it before this patch, fail diffs aren’t printed; but 
> afterwards, they are

It's a bug in Meson. It will be fixed in 0.61.3 and later (so this patch 
won't be needed there anymore), but the update to meson 0.61.3 caused other 
problems so we also can't do that right now... so I'm not sure whether we 
now want to have this patch here included, wait for a better version of 
meson, or even rather want to revert the TAP support / meson integration 
again for 7.0 ... ?

  Thomas

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Hanna Reitz]

On 18.03.22 18:36, Thomas Huth wrote:
> On 18/03/2022 18.04, Hanna Reitz wrote:
>> On 10.03.22 08:50, Thomas Huth wrote:
>>> If there is a failing iotest, the output is currently not logged to
>>> the console anymore. To get this working again, we need to run the
>>> meson test runner with "--print-errorlogs" (and without "--verbose"
>>> due to a current meson bug that will be fixed here:
>>> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
>>> We could update the "meson test" call in tests/Makefile.include,
>>> but actually it's nicer and easier if we simply do not treat the
>>> iotests as separate test target anymore and integrate them along
>>> with the other test suites. This has the disadvantage of not getting
>>> the detailed progress indication there anymore, but since that was
>>> only working right in single-threaded "make -j1" mode anyway, it's
>>> not a huge loss right now.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   v4: updated commit description
>>>
>>>   meson.build            | 6 +++---
>>>   scripts/mtest2make.py  | 4 ----
>>>   tests/Makefile.include | 9 +--------
>>>   3 files changed, 4 insertions(+), 15 deletions(-)
>>
>> I can’t really say I understand what’s going on in this patch and 
>> around it, but I can confirm that it before this patch, fail diffs 
>> aren’t printed; but afterwards, they are
>
> It's a bug in Meson. It will be fixed in 0.61.3 and later (so this 
> patch won't be needed there anymore), but the update to meson 0.61.3 
> caused other problems so we also can't do that right now... so I'm not 
> sure whether we now want to have this patch here included, wait for a 
> better version of meson, or even rather want to revert the TAP support 
> / meson integration again for 7.0 ... ?

I don’t have anything against this patch, I just don’t fully understand 
what it does, and how it works.

So as far as I understand, check-block was its own target and used 
--verbose so that the progress indication would work (with -j1). Now 
that causes problems because of a bug in meson, and so this patch drops 
that special-casing again.  The only disadvantage is that the progress 
indication (which only worked with -j1) no longer ever works.

(Is that right?)

I personally don’t mind that disadvantage, because on CI systems it 
doesn’t really matter anyway; and on developers’ systems, I would assume 
`make check` to always be run with -jX anyway.

Hanna

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Thomas Huth]

On 21/03/2022 10.06, Hanna Reitz wrote:
> On 18.03.22 18:36, Thomas Huth wrote:
>> On 18/03/2022 18.04, Hanna Reitz wrote:
>>> On 10.03.22 08:50, Thomas Huth wrote:
>>>> If there is a failing iotest, the output is currently not logged to
>>>> the console anymore. To get this working again, we need to run the
>>>> meson test runner with "--print-errorlogs" (and without "--verbose"
>>>> due to a current meson bug that will be fixed here:
>>>> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
>>>> We could update the "meson test" call in tests/Makefile.include,
>>>> but actually it's nicer and easier if we simply do not treat the
>>>> iotests as separate test target anymore and integrate them along
>>>> with the other test suites. This has the disadvantage of not getting
>>>> the detailed progress indication there anymore, but since that was
>>>> only working right in single-threaded "make -j1" mode anyway, it's
>>>> not a huge loss right now.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   v4: updated commit description
>>>>
>>>>   meson.build            | 6 +++---
>>>>   scripts/mtest2make.py  | 4 ----
>>>>   tests/Makefile.include | 9 +--------
>>>>   3 files changed, 4 insertions(+), 15 deletions(-)
>>>
>>> I can’t really say I understand what’s going on in this patch and around 
>>> it, but I can confirm that it before this patch, fail diffs aren’t 
>>> printed; but afterwards, they are
>>
>> It's a bug in Meson. It will be fixed in 0.61.3 and later (so this patch 
>> won't be needed there anymore), but the update to meson 0.61.3 caused 
>> other problems so we also can't do that right now... so I'm not sure 
>> whether we now want to have this patch here included, wait for a better 
>> version of meson, or even rather want to revert the TAP support / meson 
>> integration again for 7.0 ... ?
> 
> I don’t have anything against this patch, I just don’t fully understand what 
> it does, and how it works.
> 
> So as far as I understand, check-block was its own target and used --verbose 
> so that the progress indication would work (with -j1). Now that causes 
> problems because of a bug in meson, and so this patch drops that 
> special-casing again.  The only disadvantage is that the progress indication 
> (which only worked with -j1) no longer ever works.
> 
> (Is that right?)

Right!

> I personally don’t mind that disadvantage, because on CI systems it doesn’t 
> really matter anyway; and on developers’ systems, I would assume `make 
> check` to always be run with -jX anyway.

Right again. So currently the only question is: Do we want to see a nice 
progress output with -j1 and do not care about the error logs, or do we 
rather want to see the error logs with -j1 and do not care about the nice 
progress output? For -jX with X > 1, the patch does not change much, and 
we'd need a newer version of meson to fix that.

  Thomas

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Hanna Reitz]

On 21.03.22 10:17, Thomas Huth wrote:
> On 21/03/2022 10.06, Hanna Reitz wrote:
>> On 18.03.22 18:36, Thomas Huth wrote:
>>> On 18/03/2022 18.04, Hanna Reitz wrote:
>>>> On 10.03.22 08:50, Thomas Huth wrote:
>>>>> If there is a failing iotest, the output is currently not logged to
>>>>> the console anymore. To get this working again, we need to run the
>>>>> meson test runner with "--print-errorlogs" (and without "--verbose"
>>>>> due to a current meson bug that will be fixed here:
>>>>> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
>>>>> We could update the "meson test" call in tests/Makefile.include,
>>>>> but actually it's nicer and easier if we simply do not treat the
>>>>> iotests as separate test target anymore and integrate them along
>>>>> with the other test suites. This has the disadvantage of not getting
>>>>> the detailed progress indication there anymore, but since that was
>>>>> only working right in single-threaded "make -j1" mode anyway, it's
>>>>> not a huge loss right now.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>   v4: updated commit description
>>>>>
>>>>>   meson.build            | 6 +++---
>>>>>   scripts/mtest2make.py  | 4 ----
>>>>>   tests/Makefile.include | 9 +--------
>>>>>   3 files changed, 4 insertions(+), 15 deletions(-)
>>>>
>>>> I can’t really say I understand what’s going on in this patch and 
>>>> around it, but I can confirm that it before this patch, fail diffs 
>>>> aren’t printed; but afterwards, they are
>>>
>>> It's a bug in Meson. It will be fixed in 0.61.3 and later (so this 
>>> patch won't be needed there anymore), but the update to meson 0.61.3 
>>> caused other problems so we also can't do that right now... so I'm 
>>> not sure whether we now want to have this patch here included, wait 
>>> for a better version of meson, or even rather want to revert the TAP 
>>> support / meson integration again for 7.0 ... ?
>>
>> I don’t have anything against this patch, I just don’t fully 
>> understand what it does, and how it works.
>>
>> So as far as I understand, check-block was its own target and used 
>> --verbose so that the progress indication would work (with -j1). Now 
>> that causes problems because of a bug in meson, and so this patch 
>> drops that special-casing again.  The only disadvantage is that the 
>> progress indication (which only worked with -j1) no longer ever works.
>>
>> (Is that right?)
>
> Right!
>
>> I personally don’t mind that disadvantage, because on CI systems it 
>> doesn’t really matter anyway; and on developers’ systems, I would 
>> assume `make check` to always be run with -jX anyway.
>
> Right again. So currently the only question is: Do we want to see a 
> nice progress output with -j1 and do not care about the error logs, or 
> do we rather want to see the error logs with -j1 and do not care about 
> the nice progress output? For -jX with X > 1, the patch does not 
> change much, and we'd need a newer version of meson to fix that.

OK, to me the answer sounds obvious.  We absolutely need error logs, 
nice output is secondary to it.

Waiting for a new usable version of meson is not really an option, 
because when it comes around, we can just revert this patch (or take any 
other course of action that seems best then).

I guess we could revert TAP and/or the meson integration, I suppose 
that’d mean we’d get some progress output again, but it’s just the plain 
one from the iotests’ `check` script, right?  I’m hard-pressed to find 
good arguments against that, but I don’t really like that idea either.

Having this patch as a workaround until the functionality can be 
restored (which seems in sight) seems absolutely fine to me.  I guess 
I’ll just take it to my tree, then.  Won’t stop others from being able 
to protest, after all. :)

(I.e.: Thanks, applied to my block branch: 
https://gitlab.com/hreitz/qemu/-/commits/block)

Hanna

Re: [PATCH v4] tests: Do not treat the iotests as separate meson test target anymore

[Thomas Huth]

On 21/03/2022 14.11, Hanna Reitz wrote:
> On 21.03.22 10:17, Thomas Huth wrote:
>> On 21/03/2022 10.06, Hanna Reitz wrote:
>>> On 18.03.22 18:36, Thomas Huth wrote:
>>>> On 18/03/2022 18.04, Hanna Reitz wrote:
>>>>> On 10.03.22 08:50, Thomas Huth wrote:
>>>>>> If there is a failing iotest, the output is currently not logged to
>>>>>> the console anymore. To get this working again, we need to run the
>>>>>> meson test runner with "--print-errorlogs" (and without "--verbose"
>>>>>> due to a current meson bug that will be fixed here:
>>>>>> https://github.com/mesonbuild/meson/commit/c3f145ca2b9f5.patch ).
>>>>>> We could update the "meson test" call in tests/Makefile.include,
>>>>>> but actually it's nicer and easier if we simply do not treat the
>>>>>> iotests as separate test target anymore and integrate them along
>>>>>> with the other test suites. This has the disadvantage of not getting
>>>>>> the detailed progress indication there anymore, but since that was
>>>>>> only working right in single-threaded "make -j1" mode anyway, it's
>>>>>> not a huge loss right now.
>>>>>>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>>   v4: updated commit description
>>>>>>
>>>>>>   meson.build            | 6 +++---
>>>>>>   scripts/mtest2make.py  | 4 ----
>>>>>>   tests/Makefile.include | 9 +--------
>>>>>>   3 files changed, 4 insertions(+), 15 deletions(-)
>>>>>
>>>>> I can’t really say I understand what’s going on in this patch and 
>>>>> around it, but I can confirm that it before this patch, fail diffs 
>>>>> aren’t printed; but afterwards, they are
>>>>
>>>> It's a bug in Meson. It will be fixed in 0.61.3 and later (so this patch 
>>>> won't be needed there anymore), but the update to meson 0.61.3 caused 
>>>> other problems so we also can't do that right now... so I'm not sure 
>>>> whether we now want to have this patch here included, wait for a better 
>>>> version of meson, or even rather want to revert the TAP support / meson 
>>>> integration again for 7.0 ... ?
>>>
>>> I don’t have anything against this patch, I just don’t fully understand 
>>> what it does, and how it works.
>>>
>>> So as far as I understand, check-block was its own target and used 
>>> --verbose so that the progress indication would work (with -j1). Now that 
>>> causes problems because of a bug in meson, and so this patch drops that 
>>> special-casing again.  The only disadvantage is that the progress 
>>> indication (which only worked with -j1) no longer ever works.
>>>
>>> (Is that right?)
>>
>> Right!
>>
>>> I personally don’t mind that disadvantage, because on CI systems it 
>>> doesn’t really matter anyway; and on developers’ systems, I would assume 
>>> `make check` to always be run with -jX anyway.
>>
>> Right again. So currently the only question is: Do we want to see a nice 
>> progress output with -j1 and do not care about the error logs, or do we 
>> rather want to see the error logs with -j1 and do not care about the nice 
>> progress output? For -jX with X > 1, the patch does not change much, and 
>> we'd need a newer version of meson to fix that.
> 
> OK, to me the answer sounds obvious.  We absolutely need error logs, nice 
> output is secondary to it.
> 
> Waiting for a new usable version of meson is not really an option, because 
> when it comes around, we can just revert this patch (or take any other 
> course of action that seems best then).
> 
> I guess we could revert TAP and/or the meson integration, I suppose that’d 
> mean we’d get some progress output again, but it’s just the plain one from 
> the iotests’ `check` script, right?

Right, reverting means to get the old progress output again.

>  I’m hard-pressed to find good arguments 
> against that, but I don’t really like that idea either.

Me neither, since it feels like a big step backwards, but if someone insists 
on having both, progress indication *and* error logs, it's currently the 
only feasible way, I think.

> Having this patch as a workaround until the functionality can be restored 
> (which seems in sight) seems absolutely fine to me.  I guess I’ll just take 
> it to my tree, then.  Won’t stop others from being able to protest, after 
> all. :)
> 
> (I.e.: Thanks, applied to my block branch: 
> https://gitlab.com/hreitz/qemu/-/commits/block)

Thank you!

  Thomas