Dont run before and after hooks when all tests are skipped (#1283) #1570
Conversation
novemberborn
left a comment
novemberborn
left a comment
There was a problem hiding this comment.
You're on a roll @codeslikejaggars!
This change removes the before and after hooks from the run sequence when all tests in the collection are skipped. It intentionally keeps the always hook in the run sequence, since it would probably be surprising to a user if after.always didn't run.
👍
It doesn't look like there's a test asserting that after.always still runs even if all tests are skipped?
Could you update the relevant hook documentation as well?
lib/test-collection.js
Outdated
| const getUnskippedCount = tests => tests.filter(test => { | ||
| return !(test.metadata && test.metadata.skipped === true); | ||
| }).length; | ||
| return (getUnskippedCount(this.tests.serial) + getUnskippedCount(this.tests.concurrent)) > 0; |
| if (this._hasUnskippedTests()) { | ||
| const beforeHooks = new Sequence(this._buildHooks(this.hooks.before)); | ||
| const afterHooks = new Sequence(this._buildHooks(this.hooks.after)); | ||
| finalTests = new Sequence([beforeHooks, allTests, afterHooks], true); |
There was a problem hiding this comment.
You could create the allTests sequence in this branch.
There was a problem hiding this comment.
Not sure I can, see response to the comment below.
| const afterHooks = new Sequence(this._buildHooks(this.hooks.after)); | ||
| finalTests = new Sequence([beforeHooks, allTests, afterHooks], true); | ||
| } else { | ||
| finalTests = new Sequence([allTests], true); |
There was a problem hiding this comment.
This could be an empty sequence, since presumably all tests are skipped if it gets to this point. That would make the logic easier to grasp IMHO.
There was a problem hiding this comment.
I don't think that would work. If it's an empty sequence, the skipped tests won't be reported in the runner output.
codeslikejaggars
force-pushed
the
github-issue-1283-before-and-after-with-skip
branch
from
6993519 to
f2ee995
Compare
|
Updated the readme and added a test for |
|
Awesome work @codeslikejaggars 🚀 |
|
Great work @codeslikejaggars 👌✨🎉 |
codeslikejaggars
deleted the
github-issue-1283-before-and-after-with-skip
branch
3 participants
Attempting to fix #1283. This change removes the
beforeandafterhooks from the run sequence when all tests in the collection are skipped. It intentionally keeps thealwayshook in the run sequence, since it would probably be surprising to a user ifafter.alwaysdidn't run.