Re: [PATCH v2 9/9] Avocado tests: allow for parallel execution of tests

[Thomas Huth <thuth@redhat.com>]

On 15/08/2024 16.08, Cleber Rosa wrote:
> On Mon, Aug 12, 2024 at 6:17 AM Thomas Huth <thuth@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.

  Hi Cleber,

that patch that I linked did not set the default number of parallel tests to 
$(nproc), it just used the value of the "-j" option of make. So if you just 
run "make check-avocado" there, you only get single threaded execution as 
before. You explicitely have to run "make -jX check-avocado" to get X 
parallel threads. IMHO using "-j" is more intuitive than using yet another 
environment variable.

  Thomas