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

Tests are run even when beforeAll throws error #2713

Closed
dirkmc opened this issue Jan 26, 2017 · 25 comments
Closed

Tests are run even when beforeAll throws error #2713

dirkmc opened this issue Jan 26, 2017 · 25 comments

Comments

@dirkmc
Copy link

dirkmc commented Jan 26, 2017

When beforeAll throws an error, or returns a Promise that rejects, the tests are still run. I would expect the tests to not even run at all.

See example repo:
https://github.com/dirkmc/jest-before-all-error-handling

And the output:

> jest __tests__/test.js

 FAIL  __tests__/test.js
  ● test › tests 1 === 1

    My error
      
      at __tests__/test.js:5:14
      at Object.<anonymous> (__tests__/test.js:4:12)

  ● test › tests 2 === 2

    My error
      
      at __tests__/test.js:5:14
      at Object.<anonymous> (__tests__/test.js:4:12)

  test
    ✕ tests 1 === 1 (2ms)
    ✕ tests 2 === 2 (1ms)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 2 total
Snapshots:   0 total
Time:        0.154s, estimated 1s
Ran all test suites matching "__tests__/test.js".
  console.log __tests__/test.js:3
    before

  console.log __tests__/test.js:12
    test 1

  console.log __tests__/test.js:17
    test 2

  console.log __tests__/test.js:9
    after

npm ERR! Test failed.  See above for more details.
@cpojer
Copy link
Member

cpojer commented Jan 26, 2017

This is how jasmine behaves, I'm afraid this is a wontfix for now.

@cpojer cpojer closed this as completed Jan 26, 2017
@cpojer
Copy link
Member

cpojer commented Jan 26, 2017

btw cc @DmitriiAbramov who wants this changed.

@tamlyn
Copy link
Contributor

tamlyn commented Aug 2, 2017

Is there any clean way to abort a test run other than process.exit(1)?

@aaronabramov
Copy link
Contributor

@tamlyn do you mean --bail?

@tamlyn
Copy link
Contributor

tamlyn commented Aug 2, 2017

No, I mean from within a test suite, I have a condition which means the rest of the file should not run so I want to skip all remaining tests or terminate the process or something. Like the behaviour which the OP expected when an exception is thrown in beforeAll.

My specific use case is I'm writing some integration tests which hit the DB but I want don't want to run them if there's already data in there.

@aaronabramov
Copy link
Contributor

@tamlyn oh. in this case i don't think there's a clean way of doing this. you can use --runInBand together with process.exit.
without --runInBand, process.exit will only terminate one of the child worker processes and won't exit the main jest process

@tamlyn
Copy link
Contributor

tamlyn commented Aug 2, 2017

Yes, exiting just one worker is what I want as they operate independently. It works with process.exit so I'll stick with that. Thanks!

@sladec
Copy link

sladec commented Oct 26, 2017

Problem with process.exit is that I don't get any console error messages before exit.

@mike-marcacci
Copy link

Ya, I agree. I have to say this is an astonishingly bad developer experience for cases that rely on complex setup procedures. We have several projects with integration tests that need to add fixtures to a real database, and this issue makes it very difficult to differentiate between setup errors and failed tests, especially since setup runs independently for each test file.

It looks like this may be a better place to direct our comments, though.

@WangHansen
Copy link

Is this going to be fixed, since both jasmine and mocha have this feature? And how should I use the process.exit for the work around now?
Here is my code:

  beforeAll(() => {
    mongoose.connect('mongodb://localhost/test').then(()=>{
      mongoose.connection.dropDatabase('test')
    }, err => {
      process.exit
    })
  })

doesn't seem to work

@melroy89
Copy link

stopSpecOnExpectationFailure option?

@SimenB
Copy link
Member

SimenB commented May 18, 2018

PR very much welcome porting the fix from Jasmine: jasmine/jasmine@585287b

@SimenB SimenB reopened this May 18, 2018
@SimenB
Copy link
Member

SimenB commented May 18, 2018

@Danger89 stopSpecOnExpectationFailure seems interesting, kinda related to #5823

@g-harel
Copy link
Contributor

g-harel commented Jun 22, 2018

Wouldn't it be possible to imitate this behavior with return?

arcanis pushed a commit to yarnpkg/yarn that referenced this issue Sep 3, 2018
Previously if the tests were run without first having run `yarn build`,
these three test suites would fail with over 1000 lines of unclear
console output.

Now, later tests are stopped from running to reduce verbosity, and a
more contributor-friendly error message displayed instead.

We unfortunately can't handle this case in `beforeAll()`, since even
if it throws, later tests in the same file are still run:
jestjs/jest#2713
ebemunk added a commit to ebemunk/node-uci that referenced this issue Sep 25, 2018
@SimenB
Copy link
Member

SimenB commented Oct 16, 2018

This is fixed in Jest circus, we won't be fixing it for Jasmine. If somebody sends a PR that's fine, but I'm going to close this as we won't get to it

@rattrayalex-stripe
Copy link

Sorry, what's the relation with jasmine here? For people who are not using jasmine, failures in beforeAll et al still seem to require a try/catch + process.exit.

@rickhanlonii
Copy link
Member

@rattrayalex-stripe the old test runner for Jest (which is still currently the default) was powered by a jasmine fork called jest-jasmine2. We've written our own runner called jest-circus, which we're migrating everything to

We fixed this issue in the jest-circus runner, and wont be fixing it in the jest-jasmine2 runner (my understanding is that it's a pretty tough one to fix in the jasmine runner)

@rattrayalex-stripe
Copy link

Ah, gotcha, thanks – guess if I'd been more patient on my request for a jest-circus readme I'd have found that out 😅 thanks @rickhanlonii !

@SimenB
Copy link
Member

SimenB commented Oct 18, 2018

Yup! Also see #6695 (comment)

@paulmelnikow
Copy link

paulmelnikow commented Nov 16, 2018

If you're looking for information about when Jest Circus is going to be released, see #6295. 🎪

We fixed this issue in the jest-circus runner, and wont be fixing it in the jest-jasmine2 runner (my understanding is that it's a pretty tough one to fix in the jasmine runner)

In what version and PR was it fixed? I've tried installing jest-circus from npm and setting JEST_CIRCUS=1, but the behavior is the same: an exception in beforeAll does not prevent the tests from running.

paulmelnikow added a commit to metabolize/apollo-resolver-gcs that referenced this issue Nov 16, 2018
I'm getting bit really hard by these issues:

- jestjs/jest#2713
- jasmine/jasmine#577
- jasmine/jasmine#529
paulmelnikow added a commit to metabolize/apollo-resolver-gcs that referenced this issue Nov 16, 2018
I'm getting bit really hard by these issues:

- jestjs/jest#2713
- jasmine/jasmine#577
- jasmine/jasmine#529
randytarampi added a commit to randytarampi/react-native-appium that referenced this issue Apr 21, 2019
randytarampi added a commit to randytarampi/react-native-appium that referenced this issue Apr 22, 2019
dbjorge added a commit to microsoft/axe-pipelines-samples that referenced this issue May 23, 2019
…or (#12)

Updates jest.config.js to recommend use of the jest-circus runner to enable tests to early-exit when beforeAll/beforeEach methods fail, per jestjs/jest#2713
@adrienjoly
Copy link

It turns out that you can expect that behavior if:

  • you install the jest-circus npm package
  • you set the JEST_CIRCUS=1 environment variable when running jest

I'm surprised and bummed that this behavior is not applied by default in Jest.

@glasser
Copy link

glasser commented Nov 20, 2019

jest-circus stops tests from being run if a beforeAll throws, but it doesn't stop subsequent beforeAlls from running! eg, changing the reproduction code to:

describe('test', () => {
  beforeAll(() => {
    console.log('before');
    return new Promise((resolve, reject) => {
      reject(new Error('My error'))
    })
  })
  beforeAll(() => {
    console.log('before 2');
  })
  
  afterAll(() => console.log('after'))

  it('tests 1 === 1', () => {
    console.log('test 1')
    expect(1).toBe(1)
  })

  it('tests 2 === 2', () => {
    console.log('test 2')
    expect(2).toBe(2)
  })
})

With jest-circus, before 2 prints.

It's pretty surprising that there's a difference between:

beforeAll(() => {x(); y();});

and

beforeAll(() => { x(); });
beforeAll(() => { y(); });

I can understand why you would maybe want this behavior, but it would at least be nice if beforeAll() functions can check a flag like "are we actually going to run tests or have we already failed"?

@cstroe
Copy link

cstroe commented Jun 26, 2020

Seems like transitioning to jest-circus is ongoing: #6295

@dandv
Copy link
Contributor

dandv commented Feb 8, 2021

Seems like transitioning to jest-circus is ongoing: #6295

jest-circus is shipping with Jest 27, which is now @ next.2, and the milestone is 80% complete.

I couldn't test because I used ts-jest, and upgrading to 27 breaks imports.

Using jest and jest-circus 26.6.3 (with ts-jest 26.5.0), I still experience the issue described by the OP: if beforeAll() throws an error, all test still execute, and the error is reported for each.

@github-actions
Copy link

This issue 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 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests