diff mbox series

[RFC,PATCH-for-5.2?,4/5] tests/acceptance: Only run tests tagged 'gating-ci' on GitLab CI

Message ID 20201102144245.2134077-5-philmd@redhat.com
State New
Headers show
Series [PATCH-for-5.2?,1/5] tests/acceptance: Restrict virtio_check_params tests to X86 target | expand

Commit Message

Philippe Mathieu-Daudé Nov. 2, 2020, 2:42 p.m. UTC
To avoid breaking our CI each time a test is added, switch from the
"run all but disable some" to "only run the tagged tests on CI".
This way we can add a test to the repository, and promote it to
'gating-ci' once it is proven stable enough.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---

TODO: where to add documentation?

- docs/devel/testing.rst (too big, split?)
- tests/acceptance/README.rst

---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Nov. 2, 2020, 2:59 p.m. UTC | #1
On 11/2/20 3:42 PM, Philippe Mathieu-Daudé wrote:
> To avoid breaking our CI each time a test is added, switch from the

> "run all but disable some" to "only run the tagged tests on CI".

> This way we can add a test to the repository, and promote it to

> 'gating-ci' once it is proven stable enough.

> 

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---

> 

> TODO: where to add documentation?

> 

> - docs/devel/testing.rst (too big, split?)

> - tests/acceptance/README.rst

> 

> ---

>  tests/Makefile.include | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/tests/Makefile.include b/tests/Makefile.include

> index 3a0524ce740..f39ba760c17 100644

> --- a/tests/Makefile.include

> +++ b/tests/Makefile.include

> @@ -126,7 +126,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images

>              $(TESTS_VENV_DIR)/bin/python -m avocado \

>              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \

>              --filter-by-tags-include-empty --filter-by-tags-include-empty-key \

> -            $(AVOCADO_TAGS) \

> +            $(AVOCADO_TAGS) -t gating-ci \


This doesn't work as expected, since we have:

AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter
%-softmmu,$(TARGET_DIRS)))

And avocado '-t' works as logical OR, not logical AND.

OTOH it seems this variable predate the auto-skip feature
(when a binary is not present).

So I'll test this instead, which is simpler:

-- >8 --
@@ -90,7 +90,7 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
 # Any number of command separated loggers are accepted.  For more
 # information please refer to "avocado --help".
 AVOCADO_SHOW=app
-AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter
%-softmmu,$(TARGET_DIRS)))
+AVOCADO_TAGS=-t gating-ci

 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
        $(call quiet-command, \
---
Cleber Rosa Nov. 2, 2020, 4:12 p.m. UTC | #2
On Mon, Nov 02, 2020 at 03:59:12PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/2/20 3:42 PM, Philippe Mathieu-Daudé wrote:

> > To avoid breaking our CI each time a test is added, switch from the

> > "run all but disable some" to "only run the tagged tests on CI".

> > This way we can add a test to the repository, and promote it to

> > 'gating-ci' once it is proven stable enough.

> > 

> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> > ---

> > 

> > TODO: where to add documentation?

> > 

> > - docs/devel/testing.rst (too big, split?)

> > - tests/acceptance/README.rst

> > 

> > ---

> >  tests/Makefile.include | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > 

> > diff --git a/tests/Makefile.include b/tests/Makefile.include

> > index 3a0524ce740..f39ba760c17 100644

> > --- a/tests/Makefile.include

> > +++ b/tests/Makefile.include

> > @@ -126,7 +126,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images

> >              $(TESTS_VENV_DIR)/bin/python -m avocado \

> >              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \

> >              --filter-by-tags-include-empty --filter-by-tags-include-empty-key \

> > -            $(AVOCADO_TAGS) \

> > +            $(AVOCADO_TAGS) -t gating-ci \

> 

> This doesn't work as expected, since we have:

> 

> AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter

> %-softmmu,$(TARGET_DIRS)))

> 

> And avocado '-t' works as logical OR, not logical AND.

> 

> OTOH it seems this variable predate the auto-skip feature

> (when a binary is not present).

> 

> So I'll test this instead, which is simpler:

> 

> -- >8 --

> @@ -90,7 +90,7 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results

>  # Any number of command separated loggers are accepted.  For more

>  # information please refer to "avocado --help".

>  AVOCADO_SHOW=app

> -AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter

> %-softmmu,$(TARGET_DIRS)))

> +AVOCADO_TAGS=-t gating-ci

>


But you can *add* to the resulting list with the arches, and then you get a
logical AND.  So it'd end up as:

 -t arch:x86_64,gating-ci -t arch:aarch64,gating-ci [...]

To avoid having only "gating-ci" tests running on other environments that
call "make check-acceptance", we can look at the environment variable that
GitLab CI sets and optionally include the "gating-ci" tag.

Regards,
- Cleber.
Philippe Mathieu-Daudé Oct. 25, 2021, 2:06 p.m. UTC | #3
On 11/2/20 15:42, Philippe Mathieu-Daudé wrote:
> To avoid breaking our CI each time a test is added, switch from the

> "run all but disable some" to "only run the tagged tests on CI".

> This way we can add a test to the repository, and promote it to

> 'gating-ci' once it is proven stable enough.

> 

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---

> 

> TODO: where to add documentation?

> 

> - docs/devel/testing.rst (too big, split?)

> - tests/acceptance/README.rst

> 

> ---

>  tests/Makefile.include | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/tests/Makefile.include b/tests/Makefile.include

> index 3a0524ce740..f39ba760c17 100644

> --- a/tests/Makefile.include

> +++ b/tests/Makefile.include

> @@ -126,7 +126,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images

>              $(TESTS_VENV_DIR)/bin/python -m avocado \

>              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \

>              --filter-by-tags-include-empty --filter-by-tags-include-empty-key \

> -            $(AVOCADO_TAGS) \

> +            $(AVOCADO_TAGS) -t gating-ci \

>              $(if $(GITLAB_CI),,--failfast) tests/acceptance, \

>              "AVOCADO", "tests/acceptance")


self-NAck, this has to be added in acceptance_test_job_template
in .gitlab-ci.d/buildtest-template.yml, not in tests/Makefile.include.
diff mbox series

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3a0524ce740..f39ba760c17 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -126,7 +126,7 @@  check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
             $(TESTS_VENV_DIR)/bin/python -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
             --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
-            $(AVOCADO_TAGS) \
+            $(AVOCADO_TAGS) -t gating-ci \
             $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
             "AVOCADO", "tests/acceptance")