On Mon, Aug 12, 2024 at 6:17 AM Thomas Huth <th...@redhat.com> wrote:
> ...
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 537804d101..545b5155f9 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -94,6 +94,9 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
> >   ifndef AVOCADO_TESTS
> >       AVOCADO_TESTS=tests/avocado
> >   endif
> > +ifndef AVOCADO_PARALLEL
> > +     AVOCADO_PARALLEL=1
> > +endif
> >   # Controls the output generated by Avocado when running tests.
> >   # Any number of command separated loggers are accepted.  For more
> >   # information please refer to "avocado --help".
> > @@ -141,7 +144,8 @@ check-avocado: check-venv $(TESTS_RESULTS_DIR) 
> > get-vm-images
> >               --show=$(AVOCADO_SHOW) run 
> > --job-results-dir=$(TESTS_RESULTS_DIR) \
> >               $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
> >                       --filter-by-tags-include-empty-key) \
> > -            $(AVOCADO_CMDLINE_TAGS) --max-parallel-tasks=1 \
> > +            $(AVOCADO_CMDLINE_TAGS) 
> > --max-parallel-tasks=$(AVOCADO_PARALLEL) \
> > +                     -p timeout_factor=$(AVOCADO_PARALLEL) \
> >               $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
> >               "AVOCADO", "tests/avocado")
>
> I think it was nicer in the previous attempt to bump the avocado version:
>
> https://gitlab.com/qemu-project/qemu/-/commit/ec5ffa0056389c3c10ea2de1e783
>
> This re-used the "-j" option from "make", so you could do "make -j$(nproc)
> check-avocado" just like with the other "check" targets.
>

Hi Thomas,

I can see why it looks better, but in practice, I'm not getting the
best behavior with such a change.

First, the fact that it enables the parallelization by default, while
there still seems to be issues with test timeout issues, and even
existing races between tests (which this series tried to address as
much as possible) will not result in the best experience IMO.  On my
12 core machine, and also on GitLab CI, having 4 tests running in
parallel gets a nice speed up (as others have reported) while still
being very stable.

I'd say making the number of parallel tests equal to `nproc` is best
kept for a future round.

Let me know if this sounds reasonable to you.

Regards,
- Cleber.

>   Thomas
>


Reply via email to