Jest: Jest should fail fast and exit early (change request for --bail)

[alycda]

🚀 Feature Proposal

Jest should not run subsequent hooks or tests associated with that hook if an exception is thrown in a hook.

Motivation

When setup/teardown is broken up into small chunks (open browser, navigate to url, login, etc) and one of those hooks fails, all subsequent hooks and tests may fail and the output is clogged.

Example

When using Jest with puppeteer, I need to do the following before running any tests:

1. open browser
2. navigate to URL
3. login
4. wait for selector/page to be loaded

If any of those steps fail, the subsequent steps and tests are likely to fail as well.

Pitch

I'm using jest because of the detailed reporter, especially when a test fails. But failure reports can become needlessly long when it's only the first item that needs to be fixed to prevent a long subsequent list of failures/errors

I do not believe this is considered a change to the default reporter, as --bail does not behave the way I expect it to (it does stop after the first failing test, but if the test has 5 hooks, it still tries to run subsequent hooks and then the test fails).

[tangjeff0]

For those that want an immediate solution to this problem: https://github.com/facebook/jest/issues/2867

[seeruk]

The suggested solution works, as long as you're not using jest-circus, you have to still be using Jasmine under the hood. Is there any word on this functionality making it's way into jest-circus as an option?

[lucasfcosta]

Hello, everyone, I'm looking forward to helping with this if needed 😊

My impression is that this is actually a bug rather than a feature request.

It's also important to separate the two issues I see here:

1. The one described by @seeruk, which is about the workaround described in #2867 not working anymore. (This might not be a fault on Jest's side, but on how the workaround expected it to work)
2. The fact that if a beforeAll/beforeEach hook fails with the --bail CLI option, the related tests still run.

I was able to reproduce (2) using [email protected].

describe("test bail", () => {
    // These will run regardless of the success of `beforeEach` or any other hooks
    beforeEach(() => { throw new Error("beforeEach hook fails!") });
    test("first", () => expect(true).toBe(true));
    test("second", () => expect(true).toBe(true));
    test("third", () => expect(true).toBe(true));
});

I think it's reasonable to expect that if beforeAll and beforeEach fail when using the --bail flag no subsequent tests in that same suite should run. IMO that's also what most people would expect to happen given the number of upvotes in this issue (but I'm happy to be proven wrong if that's the case).

For the sake of symmetry and coherence I think that the same should happen with afterAll and afterEach.

I'd be happy to send a PR for solving (2) if the maintainers agree that should be the desired behaviour 💖

[seeruk]

For the sake of symmetry and coherence I think that the same should happen with afterAll and afterEach.

It might only make sense for afterEach to fail the suite if --bail is used. afterAll is well... after all of the suite has run anyway.

I think my request is more of a feature request for circus. It'd be nice to be able to tell it to end a suite if a single test in it fails. My main use-case for this (which I'm using the workaround for with jest-jasmine currently) is E2E testing. There's not much point in continuing if an earlier test fails in a suite, because the rest will undoubtedly fail too. Then my custom reporter would spam Slack with all of the errors...

[SimenB]

If beforeEach fails, the test should not run regardless of the bail flag. That's supposed to be the behaviour of Circus, but that's apparently broken (I'm on mobile, hard to track down the link).

We'll be removing Jasmine at some point (it'll stop being the default in the next major) so we're not really adding features to it, but happy to take a PR though if you wanna do the work :)

---

And I agree bail should work on the whole test run. Possibly behind an option if you want it to only short circuit per test file (current behaviour). Or switch the behaviour and make the current one be behind a new option

[lucasfcosta]

@all @SimenB I'd love to try tackling this if possible 😊

If I understood correctly, as per your explanation here's a list of changes I intend to make if you agree they should be in:

1. If beforeEach fails, the test should not run regardless of the bail flag
2. Add a new flag for the current behaviour of bail which is to short-circuit per test file.
   This is what I'd recommend since to me it seems like bail should be general and there should be a second bailPerFile flag which encapsulates the current behaviour.

A third one would be to make #2867 work in jest-circus, but I'd like to focus on the two above first since I don't want to bite more than I can chew 😄

[seeruk]

@lucasfcosta, given the above suggestions, would --bail then stop a suite when the first test fails? That's the behaviour I'm pretty keen on. Thanks for looking into tackling these changes.

[lucasfcosta]

Just an update on this:

As I was investigating today, it seems like @SimenB was correct in regards to the tests not running if the beforeEach hook fails in jest-circus. However, in Jasmine, it still runs the tests even after beforeEach fails.

Running the following describe block without JEST_CIRCUS=1 will throw the errors inside each file:

describe('a block', () => {
  beforeEach(() => {
    throw new Error('The beforeEach hook failed.');
  });

  test('skipped first test', () => {
    throw new Error('First test should not have been run');
  });

  test('skipped second test', () => {
    throw new Error('Second test should not have been run');
  });
});

With jest-circus I get beforeEach hook fails! for each of the tests, but without it, I do get the respective error messages.

Given the explanation above, aborting tests in case the beforeEach hook fails is a fix which is only necessary for Jasmine.

I haven't yet got to the item number 2 in this comment but I'll keep updates contained in this issue (preferably editing this post).

PS: I'd rather report updates here since then I can link to the discussion in the PR and someone else can pick up the work if they need it, but if you think this is unnecessary noise in GH notifications just let me know and I'll avoid updates before the PR.

[krististepanek]

And I agree bail should work on the whole test run. Possibly behind an option if you want it to only short circuit per test file (current behaviour). Or switch the behaviour and make the current one be behind a new option

Is this change to the bail flag going to be implemented in jest-circus and is there somewhere that work is being tracked?

[aalexgabi]

I had a test case failing only when running after other test cases. I had to comment all following test cases to debug this because jest does not have an option to actually bail after 1 failed test case and not test suite.

[BorntraegerMarc]

Running npx jest --bail for the following file still prints out the console.log statements:

describe('a block', () => {
    beforeEach(() => {
        throw new Error('The beforeEach hook failed.');
    });

    test('skipped first test', () => {
        console.log('XXX skipped first test');
        // throw new Error('First test should not have been run');
        expect(true).toBe(true);
    });

    test('skipped second test', () => {
        console.log('XXX skipped second test');
        // throw new Error('Second test should not have been run');
        expect(true).toBe(true);
    });
});

Logs:

 FAIL  e2e/auth/test.e2e-spec.ts
  a block
    ✕ skipped first test (4ms)
    ✕ skipped second test (1ms)

  ● a block › skipped first test

    The beforeEach hook failed.

      1 | describe('a block', () => {
      2 |     beforeEach(() => {
    > 3 |         throw new Error('The beforeEach hook failed.');
        |               ^
      4 |     });
      5 | 
      6 |     test('skipped first test', () => {

      at Object.<anonymous> (e2e/auth/test.e2e-spec.ts:3:15)

  ● a block › skipped second test

    The beforeEach hook failed.

      1 | describe('a block', () => {
      2 |     beforeEach(() => {
    > 3 |         throw new Error('The beforeEach hook failed.');
        |               ^
      4 |     });
      5 | 
      6 |     test('skipped first test', () => {

      at Object.<anonymous> (e2e/auth/test.e2e-spec.ts:3:15)

  console.log e2e/auth/test.e2e-spec.ts:7
    XXX skipped first test

  console.log e2e/auth/test.e2e-spec.ts:13
    XXX skipped second test

This indicates to me that the tests are still run even though it says "skipped"

[MrLoh]

It would be great to have a way to mark tests as dependent on previous step for e2e test as described here.

I think the simplest implementation might be a test.step function that will skip tests if a previous step in the describe has failed. I'd be happy to look into implementing this for jest-circus, but there needs to be agreement about the design first. This issue is not very specific about whether it treats beforeAll issues or just wants a way to mark tests as dependent.

[imankovskyi]

Consider using "bail" config option:

https://jestjs.io/docs/en/configuration#bail-number--boolean

This works for cases when you'd want a fast fail in beforeAll / beforeEach setup blocks.

[MrLoh]

Not the same as already explained several times in these Threads

[wilsonpage]

@seeruk I've got a jest-circus solution I think works:

1. Define CustomEnvironment.js

const JestEnvironmentNode = require("jest-environment-node");

class CustomEnvironment extends JestEnvironmentNode {
  /**
   * @param {import('jest-circus').Event} event
   * @param {import('jest-circus').State} state
   */
  async handleTestEvent(event, state) {
    if (event.name === 'test_fn_failure' || event.name === 'hook_failure') {
      // re-throw the error to exit immediately
      throw new Error(event.error);
    }
  }
}

module.exports = CustomEnvironment;

1. Use Custom Environment in jest.config.js

module.exports = {
  testRunner: 'jest-circus/runner',
  testEnvironment: './CustomEnvironment.js',
  …
};