[CI] Correctly fail failing tests by fixing interpolation of variable#5136
[CI] Correctly fail failing tests by fixing interpolation of variable#5136
Conversation
| echo $test_status | ||
| exit $test_status |
There was a problem hiding this comment.
My hunch is that $test_status is empty because buildkite variable interpolation is terrible, and we need $$test_status instead. Let's see.
There was a problem hiding this comment.
Yup, test_status is empty: https://buildkite.com/clima/oceananigans/builds/28411#019baddc-161a-4624-8c70-a98d0b5fc7f4/L93
giordano
force-pushed
the
mg/buildkite-test-failures
branch
from
eed4714 to
63c8cd0
Compare
| # Preserve the original test result for Buildkite | ||
| exit $test_status | ||
| echo "test_status=$${test_status}" | ||
| exit "$${test_status}" |
There was a problem hiding this comment.
Tests are correctly failing now: https://buildkite.com/clima/oceananigans/builds/28412#019badeb-8b2e-4582-a13c-2eb92ca1c09e/L80
🥳
giordano
changed the title
giordano
force-pushed
the
mg/buildkite-test-failures
branch
from
3d45db2 to
4f416b5
Compare
|
There are test failures in Enzyme and Reactant, but because of the bug this PR is addressing they didn't show up before: they were failing already in https://buildkite.com/clima/oceananigans/builds/28384#019ba58a-8ea2-4399-9808-3a0e6dc5daa5/L661 (7e4ffb6, merge commit of #5108), but it wasn't evident. For the record, @wsmoses enzyme tests are failing with https://buildkite.com/clima/oceananigans/builds/28414#019badf0-d0dd-4925-b734-4e1b8bada9b0/L666 while Reactant has a Edit: resolved, the real problem is that #5108 unintentionally changed the Julia version used for Enzyme/Reactant tests from v1.10 to v1.12. |
| # Run tests (but don't exit immediately so we can upload coverage even if tests fail) | ||
| set +e | ||
| julia +$JULIA_VERSION -O0 --color=yes --project --code-coverage=user -e 'using Pkg; Pkg.test(coverage="user")' | ||
| julia +$JULIA_VERSION_ENZYME -O0 --color=yes --project --code-coverage=user -e 'using Pkg; Pkg.test(coverage="user")' |
There was a problem hiding this comment.
This was accidentally changed in #5108, and because tests failures weren't being reported anymore this wasn't spotted at all.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5136 +/- ##
==========================================
- Coverage 70.86% 67.74% -3.13%
==========================================
Files 383 391 +8
Lines 21401 20921 -480
==========================================
- Hits 15165 14172 -993
- Misses 6236 6749 +513
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
navidcy
left a comment
navidcy
left a comment
There was a problem hiding this comment.
Thanks for catching and fixing this!
2 participants
Spotted in #5133 (comment).