Repository navigation
fix(pause): only pause on fail when the test fails - #5736
Open
luantaraschi wants to merge 1 commit into
Open
luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
The fail mode of the pause plugin (and the pauseOnFail alias) set its flag on step.failed, so a step that failed inside tryTo, hopeThat or retryTo opened the interactive shell even though the test passed. Track test.failed instead. Hook failures still pause because they are reported through test.failed before test.after. Fixes codeceptjs#4516
This branch has not been deployed
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.
Motivation/Description of the PR
Fixes #4516.
With
-p pauseOnFail(or-p pause:on=fail), a scenario that passes still drops into the interactive shell if one of its steps failed insidetryTo. The fail mode inlib/plugin/pause.jsset its flag onevent.step.failed, and that event fires for every failed step, including the onestryTo,hopeThatandretryToswallow afterwards. Attest.afterthe plugin then paused without looking at how the test ended.The flag is now set on
event.test.failed. Hook failures still pause, because they reach the test astest.failedbeforetest.after.I checked it in a small sandbox with the FileSystem helper and a handler that only logs when the pause would open:
tryTohopeThatretryTothat passes on the 2nd attemptBefore/AfterBeforeSuite/AfterSuitethrowor JS assertion, no failed stepThe last row is a behaviour change. A test that fails without a failed step used to slip through, and now it pauses, which matches
docs/plugins/pauseOnFail.md("Starts an interactive pause when a test fails"). If you'd rather keep the old behaviour there, I can require both a failed step and a failed test instead.The new
test/unit/plugin/pause_test.jscovers these cases (8 tests). On 4.x, 4 of them fail:tryTo,hopeThat,retryToand the plain throw. I also updated the JSDoc of thefailmode and the line indocs/debugging.md.I targeted
4.xbecause the same logic on 3.x lives inlib/plugin/pauseOnFail.jsand that branch has only been getting dependency updates.Applicable helpers:
Applicable plugins:
Type of change
Checklist:
npm run docs)npm run lint)npm test)I ran the whole unit suite (
npx mocha test/unit --recursive) on Windows: the same 11 failures as 4.x without the change, all path-related (workers, sharding, trace), plus the new tests passing. The runner tests don't run on Windows, so I didn't runnpm test. I haven't tried workers orScenario().retrywith the plugin.