diff mbox series

[v1] gitlab-ci.yml: Speed up CI by using "meson test --no-rebuild"

Message ID 20210125090339.134019-1-thuth@redhat.com
State New
Headers show
Series [v1] gitlab-ci.yml: Speed up CI by using "meson test --no-rebuild" | expand

Commit Message

Thomas Huth Jan. 25, 2021, 9:03 a.m. UTC
Currently, our check-system-* jobs are recompiling the whole sources
again. This happens due to the fact that the jobs are checking out
the whole source tree and required submodules again, and only try
to use the "build" directory with the binaries and object files
as an artifact from the previous stage - which simply does not work
anymore (with the current version of meson). Due to some changed
time stamps, meson is always trying to rebuild the whole tree.

To fix this problem, we can use "meson test --no-rebuild" instead of
make check" to avoid rebuilding all binaries every time. However, the
iotests ("make check-block") are not run by "meson test", so we have
to execute these manually now. But instead of adding them to the same
job as "meson test", it's better to put them into a separate new job
instead, to keep things nicely running in parallel in the CI.
This saves ca. 15 - 20 minutes of precious CI cycles in each run.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 RFC -> v1: Added iotests

 .gitlab-ci.yml | 113 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 94 insertions(+), 19 deletions(-)

Comments

Paolo Bonzini Jan. 25, 2021, 9:51 a.m. UTC | #1
On 25/01/21 10:03, Thomas Huth wrote:
> Currently, our check-system-* jobs are recompiling the whole sources
> again. This happens due to the fact that the jobs are checking out
> the whole source tree and required submodules again, and only try
> to use the "build" directory with the binaries and object files
> as an artifact from the previous stage - which simply does not work
> anymore (with the current version of meson). Due to some changed
> time stamps, meson is always trying to rebuild the whole tree.
> 
> To fix this problem, we can use "meson test --no-rebuild" instead of
> make check" to avoid rebuilding all binaries every time. However, the
> iotests ("make check-block") are not run by "meson test", so we have
> to execute these manually now. But instead of adding them to the same
> job as "meson test", it's better to put them into a separate new job
> instead, to keep things nicely running in parallel in the CI.
> This saves ca. 15 - 20 minutes of precious CI cycles in each run.

The reason why we're not using "meson test" is that the reporting is 
(still) inferior to what you get from "make check", especially with 
respect to which tests are failing.  This is being tracked at 
https://github.com/mesonbuild/meson/issues/7830 and the last missing 
bits are at https://github.com/mesonbuild/meson/issues/8200 (after which 
we'll change the "meson test" command line to also include "meson test 
--verbose").

However, for CI this is a minor issue because we can let GitLab parse 
the XML testing logs.  Can you add something like this to the test jobs 
for v2?

+  artifacts:
+    when: always
+    paths:
+      - build/meson-logs/
+    reports:
+      junit: build/meson-logs/testlog.junit.xml

Another possibility could be to use "make check NINJA=:".  I am not sure 
if that works, but if it does it would be the smallest possible workaround.

Paolo

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   RFC -> v1: Added iotests
> 
>   .gitlab-ci.yml | 113 ++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 94 insertions(+), 19 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index de3a3d25b5..0834267a37 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -34,6 +34,26 @@ include:
>           make -j"$JOBS" $MAKE_CHECK_ARGS ;
>         fi
>   
> +.native_meson_test_job:
> +  stage: test
> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> +  script:
> +    - cd build
> +    - touch *
> +    - make git-submodule-update
> +    - if [ -x ../meson/meson.py ]; then
> +          ../meson/meson.py test --no-rebuild -t 5 $MESON_TEST_ARGS ;
> +      else
> +          meson test --no-rebuild -t 5 $MESON_TEST_ARGS ;
> +      fi
> +
> +.native_iotest_job:
> +  stage: test
> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> +  script:
> +    - cd build/tests/qemu-iotests/
> +    - ./check -g auto -qcow2
> +
>   .native_test_job_template: &native_test_job_definition
>     stage: test
>     image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> @@ -83,17 +103,15 @@ build-system-alpine:
>     artifacts:
>       expire_in: 2 days
>       paths:
> -      - .git-submodule-status
>         - build
>   
>   check-system-alpine:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
>     needs:
>       - job: build-system-alpine
>         artifacts: true
>     variables:
>       IMAGE: alpine
> -    MAKE_CHECK_ARGS: check
>   
>   acceptance-system-alpine:
>     <<: *native_test_job_definition
> @@ -118,13 +136,20 @@ build-system-ubuntu:
>         - build
>   
>   check-system-ubuntu:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-system-ubuntu
> +      artifacts: true
> +  variables:
> +    IMAGE: ubuntu2004
> +
> +iotest-ubuntu:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-system-ubuntu
>         artifacts: true
>     variables:
>       IMAGE: ubuntu2004
> -    MAKE_CHECK_ARGS: check
>   
>   acceptance-system-ubuntu:
>     <<: *native_test_job_definition
> @@ -149,13 +174,20 @@ build-system-debian:
>         - build
>   
>   check-system-debian:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-system-debian
> +      artifacts: true
> +  variables:
> +    IMAGE: debian-amd64
> +
> +iotest-debian:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-system-debian
>         artifacts: true
>     variables:
>       IMAGE: debian-amd64
> -    MAKE_CHECK_ARGS: check
>   
>   # No targets are built here, just tools, docs, and unit tests. This
>   # also feeds into the eventual documentation deployment steps later
> @@ -194,13 +226,20 @@ build-system-fedora:
>         - build
>   
>   check-system-fedora:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-system-fedora
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +
> +iotest-fedora:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-system-fedora
>         artifacts: true
>     variables:
>       IMAGE: fedora
> -    MAKE_CHECK_ARGS: check
>   
>   acceptance-system-fedora:
>     <<: *native_test_job_definition
> @@ -226,13 +265,20 @@ build-system-centos:
>         - build
>   
>   check-system-centos:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-system-centos
> +      artifacts: true
> +  variables:
> +    IMAGE: centos8
> +
> +iotest-centos:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-system-centos
>         artifacts: true
>     variables:
>       IMAGE: centos8
> -    MAKE_CHECK_ARGS: check
>   
>   acceptance-system-centos:
>     <<: *native_test_job_definition
> @@ -256,13 +302,20 @@ build-system-opensuse:
>         - build
>   
>   check-system-opensuse:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-system-opensuse
> +      artifacts: true
> +  variables:
> +    IMAGE: opensuse-leap
> +
> +iotest-opensuse:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-system-opensuse
>         artifacts: true
>     variables:
>       IMAGE: opensuse-leap
> -    MAKE_CHECK_ARGS: check
>   
>   acceptance-system-opensuse:
>      <<: *native_test_job_definition
> @@ -525,13 +578,20 @@ build-crypto-old-nettle:
>         - build
>   
>   check-crypto-old-nettle:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-crypto-old-nettle
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +
> +iotest-crypto-old-nettle:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-crypto-old-nettle
>         artifacts: true
>     variables:
>       IMAGE: centos7
> -    MAKE_CHECK_ARGS: check
>   
>   
>   build-crypto-old-gcrypt:
> @@ -546,13 +606,20 @@ build-crypto-old-gcrypt:
>         - build
>   
>   check-crypto-old-gcrypt:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
> +  needs:
> +    - job: build-crypto-old-gcrypt
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +
> +iotest-crypto-old-gcrypt:
> +  extends: .native_iotest_job
>     needs:
>       - job: build-crypto-old-gcrypt
>         artifacts: true
>     variables:
>       IMAGE: centos7
> -    MAKE_CHECK_ARGS: check
>   
>   
>   build-crypto-only-gnutls:
> @@ -567,13 +634,21 @@ build-crypto-only-gnutls:
>         - build
>   
>   check-crypto-only-gnutls:
> -  <<: *native_test_job_definition
> +  extends: .native_meson_test_job
>     needs:
>       - job: build-crypto-only-gnutls
>         artifacts: true
>     variables:
>       IMAGE: centos7
> -    MAKE_CHECK_ARGS: check
> +
> +iotest-crypto-only-gnutls:
> +  extends: .native_iotest_job
> +  needs:
> +    - job: build-crypto-only-gnutls
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +
>   
>   # We don't need to exercise every backend with every front-end
>   build-trace-multi-user:
>
Daniel P. Berrangé Jan. 25, 2021, 9:56 a.m. UTC | #2
On Mon, Jan 25, 2021 at 10:51:04AM +0100, Paolo Bonzini wrote:
> On 25/01/21 10:03, Thomas Huth wrote:
> > Currently, our check-system-* jobs are recompiling the whole sources
> > again. This happens due to the fact that the jobs are checking out
> > the whole source tree and required submodules again, and only try
> > to use the "build" directory with the binaries and object files
> > as an artifact from the previous stage - which simply does not work
> > anymore (with the current version of meson). Due to some changed
> > time stamps, meson is always trying to rebuild the whole tree.
> > 
> > To fix this problem, we can use "meson test --no-rebuild" instead of
> > make check" to avoid rebuilding all binaries every time. However, the
> > iotests ("make check-block") are not run by "meson test", so we have
> > to execute these manually now. But instead of adding them to the same
> > job as "meson test", it's better to put them into a separate new job
> > instead, to keep things nicely running in parallel in the CI.
> > This saves ca. 15 - 20 minutes of precious CI cycles in each run.
> 
> The reason why we're not using "meson test" is that the reporting is (still)
> inferior to what you get from "make check", especially with respect to which
> tests are failing.  This is being tracked at
> https://github.com/mesonbuild/meson/issues/7830 and the last missing bits
> are at https://github.com/mesonbuild/meson/issues/8200 (after which we'll
> change the "meson test" command line to also include "meson test
> --verbose").
> 
> However, for CI this is a minor issue because we can let GitLab parse the
> XML testing logs.  Can you add something like this to the test jobs for v2?
> 
> +  artifacts:
> +    when: always
> +    paths:
> +      - build/meson-logs/
> +    reports:
> +      junit: build/meson-logs/testlog.junit.xml
> 
> Another possibility could be to use "make check NINJA=:".  I am not sure if
> that works, but if it does it would be the smallest possible workaround.

When I suggested use of --no-rebuild, I was actally thinking that
we would change the Makefile(s) to enable to pass the --no-rebuild
arg to meson. eg

  make check MESON_ARGS=--no-rebuild

is that, or something similar possible ?


Regards,
Daniel
Paolo Bonzini Jan. 25, 2021, 10:36 a.m. UTC | #3
On 25/01/21 10:56, Daniel P. Berrangé wrote:
> When I suggested use of --no-rebuild, I was actally thinking that
> we would change the Makefile(s) to enable to pass the --no-rebuild
> arg to meson. eg
> 
>    make check MESON_ARGS=--no-rebuild
> 
> is that, or something similar possible ?

It would but "make check" is not using Meson yet.  Upon switching, you 
will indeed be able to do something like that ("make check 
MTESTARGS=--no-rebuild" in the patch I am using).

Paolo
Thomas Huth Jan. 26, 2021, 7:03 a.m. UTC | #4
On 25/01/2021 10.51, Paolo Bonzini wrote:
> On 25/01/21 10:03, Thomas Huth wrote:
>> Currently, our check-system-* jobs are recompiling the whole sources
>> again. This happens due to the fact that the jobs are checking out
>> the whole source tree and required submodules again, and only try
>> to use the "build" directory with the binaries and object files
>> as an artifact from the previous stage - which simply does not work
>> anymore (with the current version of meson). Due to some changed
>> time stamps, meson is always trying to rebuild the whole tree.
>>
>> To fix this problem, we can use "meson test --no-rebuild" instead of
>> make check" to avoid rebuilding all binaries every time. However, the
>> iotests ("make check-block") are not run by "meson test", so we have
>> to execute these manually now. But instead of adding them to the same
>> job as "meson test", it's better to put them into a separate new job
>> instead, to keep things nicely running in parallel in the CI.
>> This saves ca. 15 - 20 minutes of precious CI cycles in each run.
> 
> The reason why we're not using "meson test" is that the reporting is (still) 
> inferior to what you get from "make check", especially with respect to which 
> tests are failing.  This is being tracked at 
> https://github.com/mesonbuild/meson/issues/7830 and the last missing bits 
> are at https://github.com/mesonbuild/meson/issues/8200 (after which we'll 
> change the "meson test" command line to also include "meson test --verbose").
> 
> However, for CI this is a minor issue because we can let GitLab parse the 
> XML testing logs.  Can you add something like this to the test jobs for v2?
> 
> +  artifacts:
> +    when: always
> +    paths:
> +      - build/meson-logs/
> +    reports:
> +      junit: build/meson-logs/testlog.junit.xml

Ok, I've tried that but it also worked not quite as expected:

https://gitlab.com/huth/qemu/-/pipelines/246463068/test_report

The "check-*" jobs now show up in the test report page, but even though I've 
made some tests failing (e.g. the check-system-centos job), the failing jobs 
are showing up with "Failed: 0" there. Also the duration is always marked 
with 0.00ms.

> Another possibility could be to use "make check NINJA=:".  I am not sure if 
> that works, but if it does it would be the smallest possible workaround.

It works! So since all the other options seem to have other drawbacks right 
now, I guess that's the best way to go until we completely switch to "meson 
test". Thus I've just sent a patch for that.

  Thomas
Paolo Bonzini Jan. 26, 2021, 8:48 a.m. UTC | #5
On 26/01/21 08:03, Thomas Huth wrote:
>>
>> +  artifacts:
>> +    when: always
>> +    paths:
>> +      - build/meson-logs/
>> +    reports:
>> +      junit: build/meson-logs/testlog.junit.xml
> 
> Ok, I've tried that but it also worked not quite as expected:
> 
> https://gitlab.com/huth/qemu/-/pipelines/246463068/test_report
> 
> The "check-*" jobs now show up in the test report page, but even though 
> I've made some tests failing (e.g. the check-system-centos job), the 
> failing jobs are showing up with "Failed: 0" there.

Hmm, that's a limitation of either TAP or GitLab.  Of TAP, because it 
doesn't report tests when they start, so there is no test to attach the 
timeout to.  Of GitLab, because it reports the outcome of individual 
testcases but not the outcome of the testsuites.  Not sure how to fix it.

> Also the duration is always marked with 0.00ms.
Ok, let's wait for a new Meson release before doing that too.

Paolo
diff mbox series

Patch

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index de3a3d25b5..0834267a37 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -34,6 +34,26 @@  include:
         make -j"$JOBS" $MAKE_CHECK_ARGS ;
       fi
 
+.native_meson_test_job:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  script:
+    - cd build
+    - touch *
+    - make git-submodule-update
+    - if [ -x ../meson/meson.py ]; then
+          ../meson/meson.py test --no-rebuild -t 5 $MESON_TEST_ARGS ;
+      else
+          meson test --no-rebuild -t 5 $MESON_TEST_ARGS ;
+      fi
+
+.native_iotest_job:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  script:
+    - cd build/tests/qemu-iotests/
+    - ./check -g auto -qcow2
+
 .native_test_job_template: &native_test_job_definition
   stage: test
   image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
@@ -83,17 +103,15 @@  build-system-alpine:
   artifacts:
     expire_in: 2 days
     paths:
-      - .git-submodule-status
       - build
 
 check-system-alpine:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
   needs:
     - job: build-system-alpine
       artifacts: true
   variables:
     IMAGE: alpine
-    MAKE_CHECK_ARGS: check
 
 acceptance-system-alpine:
   <<: *native_test_job_definition
@@ -118,13 +136,20 @@  build-system-ubuntu:
       - build
 
 check-system-ubuntu:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-system-ubuntu
+      artifacts: true
+  variables:
+    IMAGE: ubuntu2004
+
+iotest-ubuntu:
+  extends: .native_iotest_job
   needs:
     - job: build-system-ubuntu
       artifacts: true
   variables:
     IMAGE: ubuntu2004
-    MAKE_CHECK_ARGS: check
 
 acceptance-system-ubuntu:
   <<: *native_test_job_definition
@@ -149,13 +174,20 @@  build-system-debian:
       - build
 
 check-system-debian:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-system-debian
+      artifacts: true
+  variables:
+    IMAGE: debian-amd64
+
+iotest-debian:
+  extends: .native_iotest_job
   needs:
     - job: build-system-debian
       artifacts: true
   variables:
     IMAGE: debian-amd64
-    MAKE_CHECK_ARGS: check
 
 # No targets are built here, just tools, docs, and unit tests. This
 # also feeds into the eventual documentation deployment steps later
@@ -194,13 +226,20 @@  build-system-fedora:
       - build
 
 check-system-fedora:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-system-fedora
+      artifacts: true
+  variables:
+    IMAGE: fedora
+
+iotest-fedora:
+  extends: .native_iotest_job
   needs:
     - job: build-system-fedora
       artifacts: true
   variables:
     IMAGE: fedora
-    MAKE_CHECK_ARGS: check
 
 acceptance-system-fedora:
   <<: *native_test_job_definition
@@ -226,13 +265,20 @@  build-system-centos:
       - build
 
 check-system-centos:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos8
+
+iotest-centos:
+  extends: .native_iotest_job
   needs:
     - job: build-system-centos
       artifacts: true
   variables:
     IMAGE: centos8
-    MAKE_CHECK_ARGS: check
 
 acceptance-system-centos:
   <<: *native_test_job_definition
@@ -256,13 +302,20 @@  build-system-opensuse:
       - build
 
 check-system-opensuse:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-system-opensuse
+      artifacts: true
+  variables:
+    IMAGE: opensuse-leap
+
+iotest-opensuse:
+  extends: .native_iotest_job
   needs:
     - job: build-system-opensuse
       artifacts: true
   variables:
     IMAGE: opensuse-leap
-    MAKE_CHECK_ARGS: check
 
 acceptance-system-opensuse:
    <<: *native_test_job_definition
@@ -525,13 +578,20 @@  build-crypto-old-nettle:
       - build
 
 check-crypto-old-nettle:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-crypto-old-nettle
+      artifacts: true
+  variables:
+    IMAGE: centos7
+
+iotest-crypto-old-nettle:
+  extends: .native_iotest_job
   needs:
     - job: build-crypto-old-nettle
       artifacts: true
   variables:
     IMAGE: centos7
-    MAKE_CHECK_ARGS: check
 
 
 build-crypto-old-gcrypt:
@@ -546,13 +606,20 @@  build-crypto-old-gcrypt:
       - build
 
 check-crypto-old-gcrypt:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
+  needs:
+    - job: build-crypto-old-gcrypt
+      artifacts: true
+  variables:
+    IMAGE: centos7
+
+iotest-crypto-old-gcrypt:
+  extends: .native_iotest_job
   needs:
     - job: build-crypto-old-gcrypt
       artifacts: true
   variables:
     IMAGE: centos7
-    MAKE_CHECK_ARGS: check
 
 
 build-crypto-only-gnutls:
@@ -567,13 +634,21 @@  build-crypto-only-gnutls:
       - build
 
 check-crypto-only-gnutls:
-  <<: *native_test_job_definition
+  extends: .native_meson_test_job
   needs:
     - job: build-crypto-only-gnutls
       artifacts: true
   variables:
     IMAGE: centos7
-    MAKE_CHECK_ARGS: check
+
+iotest-crypto-only-gnutls:
+  extends: .native_iotest_job
+  needs:
+    - job: build-crypto-only-gnutls
+      artifacts: true
+  variables:
+    IMAGE: centos7
+
 
 # We don't need to exercise every backend with every front-end
 build-trace-multi-user: