False Positives: Tests Should Fail if Build Fails

[ax3l]

When build steps fail, e.g., for tools

building tools...
   building particle_compare...
   make -j2 AMREX_HOME=/tmp/ci-31YWhskdkz/amrex/   DEBUG=FALSE USE_MPI=FALSE  COMP=g++ TEST=TRUE USE_ASSERTION=TRUE WarpxBinDir= 
   WARNING: build failed
   unable to continue, tools not able to be built
Finishing: Build & test

Then the overall return status should be non-zero.
Otherwise, CI using the regression testing scripts silently passes (permanent false negative).

Seen today by @RemiLehe and @EZoni on WarpX CI.

[ax3l]

Seen again today, disabling our tests with an always false-ok:

building tools...
   building particle_compare...
   make -j2 AMREX_HOME=/tmp/ci-kXuvJgn5SL/amrex/   DEBUG=FALSE USE_MPI=FALSE  COMP=g++ TEST=TRUE USE_ASSERTION=TRUE WarpxBinDir= 
   WARNING: build failed
   unable to continue, tools not able to be built
Finishing: Build & test

x-ref: ECP-WarpX#17

[WeiqunZhang]

https://github.com/AMReX-Codes/regression_testing/blob/main/suite.py#L1009

This particular issue was fixed 2 years ago. But warpx's regression testing repo was not updated.

[ax3l]

Thanks! Wuhu! Same as ECP-WarpX#17 indeed!

I leave this issue open since we still need to implement error handling in case any of the build steps fail. Currently they write an error and return a zero error code, letting CI pass. (The rc in your link needs to be propagated in some way, I think.)

[WeiqunZhang]

https://github.com/AMReX-Codes/regression_testing/blob/main/test_util.py#L242

Seem that we just need to make this return an error code sys.exit(1) instead of the default 0.

[ax3l]

Yes!
Huh, I did not expect that the logger also exits - should have checked. Will PR that in, great idea!