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

[Hanna Reitz <hreitz@redhat.com>]

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