Only print fail-fast when there are remaining tests #1179
Conversation
There was a problem hiding this comment.
Thanks for taking this on @jarlehansen! Reviewing the PR made me realize we're always printing the message, even if no tests failed, so this is definitely a welcome improvement.
@avajs/core any preference towards showing a "at least 5 tests have been skipped" kind of message, or perhaps "some tests have been skipped"?
| @@ -139,6 +140,7 @@ class RunStatus extends EventEmitter { | |||
| this.skipCount = sum(this.stats, 'skipCount'); | |||
| this.todoCount = sum(this.stats, 'todoCount'); | |||
| this.failCount = sum(this.stats, 'failCount'); | |||
| this.remainingCount = (this.testCount - (this.passCount + this.failCount)); | |||
There was a problem hiding this comment.
this.testCount - this.passCount - this.failCount is clearer.
You need to include other test states in this count, see the highlighted lines in https://github.com/avajs/ava/blob/033d4dcdcbdadbf665c740ff450c2a775a8373dc/lib/run-status.js#L41:L45.
The computation should be tested. That's probably best in test/run-status.js, though most of its behavior ends up being tested elsewhere.
| test('results without fail-fast if no skipped tests', function (t) { | ||
| var reporter = miniReporter(); | ||
| var runStatus = { | ||
| remainingCount: 0, |
There was a problem hiding this comment.
I think this needs failCount: 1, like with the verbose test?
|
Thanks for the comments, I will wait on #1177 to be merged and then update this PR. |
jarlehansen
force-pushed
the
print-failfast-on-skipped-tests
branch
2 times, most recently
from
550574b
to
2eaadf2
Compare
|
I have updated this PR with the changes done by @ThomasBem |
sindresorhus
changed the title
| @@ -215,7 +215,7 @@ MiniReporter.prototype.finish = function (runStatus) { | |||
| }); | |||
| } | |||
|
|
|||
| if (runStatus.failFastEnabled === true) { | |||
| if (runStatus.failFastEnabled === true && runStatus.remainingCount > 0) { | |||
There was a problem hiding this comment.
This should check if there were any failures. Otherwise the warning is also printed when .only() is used and --fail-fast is on:
$ "$(npm bin)"/ava --fail-fast
1 passed
`--fail-fast` is on. Any number of tests may have been skipped
The .only() modifier is used in some tests. 135 tests were not run.
jarlehansen
force-pushed
the
print-failfast-on-skipped-tests
branch
from
2eaadf2
to
3f5e68f
Compare
|
I have included checking runStatus.failCount, also added/updated the tests. |
|
Thank you @jarlehansen. This is excellent 👌😄 |
This PR tries to fix #1171 by calculating the remaningCount in run-status.js
And then check the value in the reporters, if there are no remaining test it will not print: