Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add failFast flag to abort test suite after first failure #387

Merged
merged 8 commits into from
Jun 4, 2015

Conversation

PhilVargas
Copy link
Contributor

Addresses issue #127 to add a flag to abort the test suite after the first failing test file occurs.

  • invoke the fail-fast flag by calling jest with the --bail options or -b alias.
  • if the flag is present and a test fails, exit the worker process from the DefaultTestReporter
    after calling the cleanup method onRunComplete in order to print the test results before abort
  • calculate the overall runTime by saving the startTime (instead of final runTime) in aggregatedResults. this allows us to calculate the runTime anywhere aggregatedResults is available. I.e. to print the overall run time during any individual test, instead of after all the tests have run.

I'm currently having an issue on jest 0.4.5 (from facebook/jest/master) that is forcing noHighlight to always be called. I believe its related to pull #364, but i'm unsure if this is a persistent issue or just my machine. Its happening by default and passing --noHighlight false does not remove it.
Looking forward to your feedback! @amasad

examples:
Fail fast
Fail fast

Fail fast, verbose
Fail fast, verbose

Fail fast, no highlight
Fail fast, no highlight

If a test fails, call the reporter#onRunComplete and exit the worker process

Store startTime in `aggregatedResults` instead of runTime in order to calculate
total runTime at any point in time after the test has begun
Extract redundancies in failure message and header creation

Remove unused runTime variable in aggregatedResults
@amasad
Copy link
Contributor

amasad commented Jun 2, 2015

I like it! nit: can we call it --bail? inspired from mocha.
otherwise I'll let @DmitrySoshnikov merge

@PhilVargas
Copy link
Contributor Author

Sure thing! I'll push the changes up soon

this.log(failureMessage);
}

if (config.bail){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: space before {

@DmitrySoshnikov
Copy link
Contributor

This is looks great, thanks. Let's fix the nits, and it's good to go.

@PhilVargas
Copy link
Contributor Author

np. changes pushed

@@ -62,6 +62,10 @@ function(config, testResult, aggregatedResults) {
testRunTimeString = this._formatMsg(testRunTimeString, FAIL_COLOR);
}

var resultHeader = this._getResultHeader(allTestsPassed, pathStr, [
testRunTimeString
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be:

var resultHeader = this._getResultHeader(allTestsPassed, pathStr, [
  testRunTimeString
]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right.
just pushed up the fix

@DmitrySoshnikov
Copy link
Contributor

Thanks!

DmitrySoshnikov added a commit that referenced this pull request Jun 4, 2015
Add failFast flag to abort test suite after first failure
@DmitrySoshnikov DmitrySoshnikov merged commit 3022579 into jestjs:master Jun 4, 2015
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants