fix(runner): run the file teardown before a bad annotation aborts - #1334
Merged
Conversation
A malformed @timeout or @Retry aborted the run with `exit 1` from inside `call_test_functions`. `set_up_before_script` had already run, so whatever it acquired was leaked: the file's `tear_down_after_script` never got a frame. `annotations_validate` now reports instead of exiting, `call_test_functions` returns non-zero, and the caller that owns the file's teardown runs it before honouring the abort. Same shape as the bench runner's fix for #1322. The error text and the non-zero exit code are unchanged. Closes #1329
`call_test_functions ... || file_status=$?` put the whole test run in a context where bash ignores errexit, and bash carries that down into every function and subshell the command calls. The `set -e` that aborts a failing `set_up` stopped firing, so a broken hook reported as passing: three acceptance tests failed on every Linux job while passing on bash 3.2. Read `$?` on the next line instead. This shell runs with errexit off, so a non-zero return does not abort it and the file's teardown still runs.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #1329
A malformed
# @timeoutor# @retryaborted the run from insidecall_test_functions.set_up_before_scripthad already run, so the file'stear_down_after_scriptnever got a frame and whatever the setup acquired was leaked. Sequential and--parallelboth leaked it.💡 Changes
bashunit benchaborts on a malformed annotation without runningtear_down_after_script#1322.@timeoutand@retry, each under--no-paralleland--parallel.--parallela malformed annotation next to a passing file still exits 0. That is a pre-existing exit-code defect, not this teardown leak, and it gets its own issue.