[PATCH 1/9] Avocado GitLab CI jobs: don't reset TARGETS and simplify commands

Cleber Rosa posted 9 patches 3 years, 11 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Cleber Rosa <crosa@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Antony Pavlov <antonynpavlov@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Fabien Chouteau <chouteau@adacore.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Hervé Poussineau" <hpoussin@reactos.org>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Paolo Bonzini <pbonzini@redhat.com>, Eric Auger <eric.auger@redhat.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>
[PATCH 1/9] Avocado GitLab CI jobs: don't reset TARGETS and simplify commands
Posted by Cleber Rosa 3 years, 11 months ago
The Avocado tests rely on the TARGETS variable, which is computed
based on the built targets.  The current set of commands on the
inherited scripts section will reset those, leaving TARGETS empty and
consequently the AVOCADO_CMDLINE_TAGS empty too.

This is causing the list of tests to have no filtering by tags, which
can be seen by the large number of CANCEL/SKIP statuses (because of
the lack of a matching qemu-system-$(ARCH) binary).

With this change, the TARGETS variable is properly computed, and so is
the AVOCADO_CMDLINE_TAGS.  This causes a reduction in the number of
tests attempted to be run on each job, and less noise on the test
results.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .gitlab-ci.d/buildtest-template.yml | 3 +++
 .gitlab-ci.d/buildtest.yml          | 9 ---------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index 2c7980a4f6..c038a0910f 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -64,6 +64,9 @@
         du -chs ${CI_PROJECT_DIR}/avocado-cache ;
       fi
     - export AVOCADO_ALLOW_UNTRUSTED_CODE=1
+  script:
+    - cd build
+    - make check-avocado
   after_script:
     - cd build
     - du -chs ${CI_PROJECT_DIR}/avocado-cache
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0aa70213fb..d0bed9c382 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -33,7 +33,6 @@ avocado-system-alpine:
       artifacts: true
   variables:
     IMAGE: alpine
-    MAKE_CHECK_ARGS: check-avocado
 
 build-system-ubuntu:
   extends: .native_build_job_template
@@ -66,7 +65,6 @@ avocado-system-ubuntu:
       artifacts: true
   variables:
     IMAGE: ubuntu2004
-    MAKE_CHECK_ARGS: check-avocado
 
 build-system-debian:
   extends: .native_build_job_template
@@ -98,7 +96,6 @@ avocado-system-debian:
       artifacts: true
   variables:
     IMAGE: debian-amd64
-    MAKE_CHECK_ARGS: check-avocado
 
 crash-test-debian:
   extends: .native_test_job_template
@@ -143,7 +140,6 @@ avocado-system-fedora:
       artifacts: true
   variables:
     IMAGE: fedora
-    MAKE_CHECK_ARGS: check-avocado
 
 crash-test-fedora:
   extends: .native_test_job_template
@@ -189,7 +185,6 @@ avocado-system-centos:
       artifacts: true
   variables:
     IMAGE: centos8
-    MAKE_CHECK_ARGS: check-avocado
 
 build-system-opensuse:
   extends: .native_build_job_template
@@ -221,7 +216,6 @@ avocado-system-opensuse:
       artifacts: true
   variables:
     IMAGE: opensuse-leap
-    MAKE_CHECK_ARGS: check-avocado
 
 
 # This jobs explicitly disable TCG (--disable-tcg), KVM is detected by
@@ -382,7 +376,6 @@ avocado-cfi-aarch64:
       artifacts: true
   variables:
     IMAGE: fedora
-    MAKE_CHECK_ARGS: check-avocado
 
 build-cfi-ppc64-s390x:
   extends: .native_build_job_template
@@ -424,7 +417,6 @@ avocado-cfi-ppc64-s390x:
       artifacts: true
   variables:
     IMAGE: fedora
-    MAKE_CHECK_ARGS: check-avocado
 
 build-cfi-x86_64:
   extends: .native_build_job_template
@@ -460,7 +452,6 @@ avocado-cfi-x86_64:
       artifacts: true
   variables:
     IMAGE: fedora
-    MAKE_CHECK_ARGS: check-avocado
 
 tsan-build:
   extends: .native_build_job_template
-- 
2.35.1


Re: [PATCH 1/9] Avocado GitLab CI jobs: don't reset TARGETS and simplify commands
Posted by Daniel P. Berrangé 3 years, 11 months ago
On Fri, Feb 25, 2022 at 04:01:48PM -0500, Cleber Rosa wrote:
> The Avocado tests rely on the TARGETS variable, which is computed
> based on the built targets.  The current set of commands on the
> inherited scripts section will reset those, leaving TARGETS empty and
> consequently the AVOCADO_CMDLINE_TAGS empty too.
> 
> This is causing the list of tests to have no filtering by tags, which
> can be seen by the large number of CANCEL/SKIP statuses (because of
> the lack of a matching qemu-system-$(ARCH) binary).
> 
> With this change, the TARGETS variable is properly computed, and so is
> the AVOCADO_CMDLINE_TAGS.  This causes a reduction in the number of
> tests attempted to be run on each job, and less noise on the test
> results.

This description isn't making sense to me.

AFAICT, none of the avocado-system-$DISTRO  jobs in buildtest.yml
are setting the $TARGETS variable before/after this change.

> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/buildtest-template.yml | 3 +++
>  .gitlab-ci.d/buildtest.yml          | 9 ---------
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
> index 2c7980a4f6..c038a0910f 100644
> --- a/.gitlab-ci.d/buildtest-template.yml
> +++ b/.gitlab-ci.d/buildtest-template.yml
> @@ -64,6 +64,9 @@
>          du -chs ${CI_PROJECT_DIR}/avocado-cache ;
>        fi
>      - export AVOCADO_ALLOW_UNTRUSTED_CODE=1
> +  script:
> +    - cd build
> +    - make check-avocado

The parent template has a 'script:' block we currently inherit 

    - scripts/git-submodule.sh update
        $(sed -n '/GIT_SUBMODULES=/ s/.*=// p' build/config-host.mak)
    - cd build
    - find . -type f -exec touch {} +
    # Avoid recompiling by hiding ninja with NINJA=":"
    - make NINJA=":" $MAKE_CHECK_ARGS

so replacing this is loosing the potential git submodule update
and looses the protection against recompilation.

I'm not seeing what in this old inherited is breaking the $TARGETS
variable, not least because it was never set before/after AFAICT.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|