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

[Thomas Huth <thuth@redhat.com>]

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