Goddard-Fortran-Ecosystem / pFUnit

Parallel Fortran Unit Testing Framework
Other
169 stars 45 forks source link

pFUnit silently fails on Actions #423

Open n-claes opened 1 year ago

n-claes commented 1 year ago

We just noticed some very interesting behaviour in our unit testing suite (GitHub Actions). Apparently a few tests have been failing for the past month, without actually triggering a failure status. As you can see below we have 3 tests that are failing while Actions nicely exits with a green check mark, so it took us about a month to notice. I forced a proper fail by piping the output log to a file and grepping for FAILURES, which throws an exit 1 if found. Doing this fails Actions like it should, so I'm inclined to think this is a pFUnit bug. Any thoughts on this? The failing tests are two @assertExceptionRaised and one @assertTrue if that's useful.

Screenshot 2023-04-27 at 15 40 11
tclune commented 1 year ago

Hmm. That is odd. It should return a non-zero exit if any tests fail. That check does not even care about what sort of asserts were involved; just that Failures or Errors is non-zero.

Please let me know which compiler/version you are using and if relevant which MPI flavor. Note that MPI does not promise to propagate Fortran returns, so there is no standard we can rely on.

(And now hoping my various CI tests are not experiencing the same faux success.)

n-claes commented 1 year ago

We're using gfortran 11.3.0 as compiler with cmake 3.24.4, no MPI is used. We're building pFUnit with

cmake .. -DSKIP_MPI=YES -DSKIP_OPENMP=YES -DSKIP_FHAMCREST=YES

As for the version of pFUnit itself, we're restoring from cache in our CI but I can see the issue popping up in various merged PRs over the last weeks in which a fresh clone was fetched. As far as I can see our failing tests with green exit first appeared on March 2, which luckily coincides with a clean pFUnit install (so that would be commit 027393b if I'm not mistaken).

tclune commented 1 year ago

OK - I've gone to look at the relevant source now: https://github.com/Goddard-Fortran-Ecosystem/pFUnit/blob/74e6f4a0c5ff622d3e127b17c29baafe2ca45e5d/src/funit/FUnit.F90#L167-L173

I think there once was an error stop here, but with gfortran that resulted in a traceback that many found annoying.

If you use the ctest macro provided by pFUnit: https://github.com/Goddard-Fortran-Ecosystem/pFUnit/blob/main/include/add_pfunit_ctest.cmake

Then the ctest triggers on the string produced for failures. If you don't want to use that CMake macro, you'll need to grep on that string in your CI.

I could probably be talked into putting the error stop back in with some CMake setting, but would prefer to avoid the complexity.

Apologies that this is not better documented.

mathomp4 commented 1 year ago

@n-claes Can you also point me to your GitHub Action? I just engineered a pFUnit failure both in MAPL and in pFUnit itself.

The MAPL GitHub Action failed on both GNU and Intel:

https://github.com/GEOS-ESM/MAPL/actions/runs/4821478926

and the PR shows that (https://github.com/GEOS-ESM/MAPL/pull/2103)

But MAPL is running with a slightly older pFUnit, so I put in a failure into pFUnit itself and it too failed:

https://github.com/Goddard-Fortran-Ecosystem/pFUnit/actions/runs/4821717640

and the PR shows the red X:

https://github.com/Goddard-Fortran-Ecosystem/pFUnit/pull/424

mathomp4 commented 1 year ago

I just saw @tclune's post. I edited tests/funit-core/Test_Assert_Integer.pf to fail and I think that does use the macro. Let me try doing the failure in a non-macro test...

mathomp4 commented 1 year ago

Hmm. I engineered a failure in old_tests and it seems to be triggering the Red X as well:

https://github.com/Goddard-Fortran-Ecosystem/pFUnit/actions/runs/4821822710/jobs/8588226893

n-claes commented 1 year ago

@mathomp4 that's indeed puzzling... Somehow in our case it doesn't trigger an error stop: the latest fail-but-green Action log can be found here https://github.com/n-claes/legolas/actions/runs/4818904003/jobs/8581455760

@tclune thanks for the macro suggestion, I'm currently fixing the failing tests and will probably fall back on that instead of my grep-for-failure patch

ZedThree commented 2 months ago

We've just been bitten by this. What about the possibility of restoring the error stop as the simplest implementation that ensures test failures have non-zero exit code, and include in the docs how to turn off compiler backtraces (-fno-backtrace for gcc, for instance)?