| Message ID | 20240806173119.582857-10-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bump Avocado to 103.0 LTS and update tests for compatibility and new features | expand |
On 06/08/2024 19.31, Cleber Rosa wrote: > The updated Avocado version allows for the execution of tests in > parallel. > > While on a CI environment it may not be a good idea to increase the > parallelization level in a single runner, developers may leverage that > on specific CI runners or on their development environments. > > This also multiplies the timeout for each test accordingly. The > reason is that more concurrency can lead to less resources, and less > resources can lead to some specific tests taking longer to complete > and then time out. The timeout factor being used here is very > conservative (being equal to the amount of parallel tasks). The worst > this possibly oversized timeout value can do is making users wait a > bit longer for the job to finish if a test hangs. > > Overall, users can expect a much quicker turnaround on most systems > with a value such as 8 on a 12 core machine. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- ... > 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. Thomas
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. Regards, - Cleber. > Thomas >
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
On 8/16/24 02:02, Thomas Huth wrote: > 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. Seconded. r~
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index af73d3d64f..97ebc8211f 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -998,6 +998,18 @@ of Avocado or ``make check-avocado``, and can also be queried using: pyvenv/bin/avocado list tests/avocado +To run tests in parallel, the ``AVOCADO_PARALLEL`` environment +variable can be defined with a value different than ``1`` (its default +value). Example: + + .. code:: + + make check-avocado AVOCADO_PARALLEL=4 + +Please exercise care when using parallel execution with the QEMU +Avocado tests as a higher system load can cause time sensitive tests +to timeout and be interrupted. + Manual Installation ~~~~~~~~~~~~~~~~~~~ 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")
The updated Avocado version allows for the execution of tests in parallel. While on a CI environment it may not be a good idea to increase the parallelization level in a single runner, developers may leverage that on specific CI runners or on their development environments. This also multiplies the timeout for each test accordingly. The reason is that more concurrency can lead to less resources, and less resources can lead to some specific tests taking longer to complete and then time out. The timeout factor being used here is very conservative (being equal to the amount of parallel tasks). The worst this possibly oversized timeout value can do is making users wait a bit longer for the job to finish if a test hangs. Overall, users can expect a much quicker turnaround on most systems with a value such as 8 on a 12 core machine. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- docs/devel/testing.rst | 12 ++++++++++++ tests/Makefile.include | 6 +++++- 2 files changed, 17 insertions(+), 1 deletion(-)