Re: [PATCH 17/22] tests: forcibly run 'make check-venv' for crash tests

[Thomas Huth <thuth@redhat.com>]

On 19/11/2025 09.05, Paolo Bonzini wrote:
> 
> 
> Il mar 18 nov 2025, 22:50 John Snow <jsnow@redhat.com 
> <mailto:jsnow@redhat.com>> ha scritto:
> 
> 
> 
>     On Mon, Nov 17, 2025, 3:47 PM Paolo Bonzini <pbonzini@redhat.com
>     <mailto:pbonzini@redhat.com>> wrote:
> 
> 
> 
>         Il lun 17 nov 2025, 20:07 Daniel P. Berrangé <berrange@redhat.com
>         <mailto:berrange@redhat.com>> ha scritto:
> 
>             On Mon, Nov 17, 2025 at 01:51:25PM -0500, John Snow wrote:
>              > In order to convert the existing Makefile target from a manual
>              > invocation of mkvenv to one that uses the meson dependency
>             system, we
>              > need to not suppress ninja here.
>              >
>              > I'm not sure if this creates problems I am not aware of; but
>             invoking
>              > ninja here is no longer spurious but will become necessary.
> 
>             Yes, this will likely create problems. From the commit message that
>             introduced NINJA=":"...
> 
>               Avoid it with the same
>                  trick that we are using in buildtest-template.yml already
>             by disabling
>                  the up-to-date check via NINJA=":".
> 
> 
>         Move the check-venv call to buildtest-template.yml, right after
>         configure? It's cheap enough.
> 
>         Paolo
> 
> 
>     What's the root issue here? That there's enough of a time delay between
>     the actual configure and the test running that it re-runs configure?
> 
> 
> That the git repo is cloned again and is therefore newer than the artifacts. 
> That triggers a full rebuild.

Right. At least for the other "functional" and "check-system" jobs in our 
CI, this was a big issue, calling "make check-functional/qtest/iotest" there 
triggered a rebuild of all binaries, increasing our CI runtime significantly.

For the "crash test" jobs, it was mainly about avoiding to rerun the 
"configure" step for cosmetical reasons each time the job runs. The rebuild 
of the qemu binaries is triggered by "make check-iotests/qtests/function" in 
the other jobs, and not by "make check-venv" alone, so it's not as bad as in 
the other jobs here:
Without your patch, the jobs take about 16 minutes (crash-test-debian) and 
10 minutes (crash-test-fedora):

  https://gitlab.com/thuth/qemu/-/jobs/12138792667
  https://gitlab.com/thuth/qemu/-/jobs/12138792674

With your patch applied:

  https://gitlab.com/thuth/qemu/-/jobs/12139438583
  https://gitlab.com/thuth/qemu/-/jobs/12139438599

the jobs take approx. the same amount of time, so I think it's OK here to 
drop the NINJA=":" again.

But I think we should give Paolo's idea a try and move the "make check-venv" 
to buildtest-template.yml instead. In fact, that might even help to slightly 
decrease the total runtime of the jobs since all the other "check" jobs 
don't have to do this again (for example the check-functional jobs are 
running it, too, see: 
https://gitlab.com/qemu-project/qemu/-/jobs/12139584312#L1051).

  Thomas