jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.2k stars 6.46k forks source link

test.concurrent ignores beforeEach and afterEach #7997

Open sibyldawn opened 5 years ago

sibyldawn commented 5 years ago

🐛 Bug Report

When using test.concurrent with tests, all the scenarios run first and then the beforeEach and afterEach blocks run next.

To Reproduce

Write a simple test suite with a beforeEach , 3 tests using test.concurrent, and afterEach.

Expected behavior

I expect runner to execute beforeEach block, then the test, then afterEach block.

Link to repl or repo (highly encouraged)

test-concurrent bug demo

Issues without a reproduction link are likely to stall.

Run npx envinfo --preset jest

Paste the results here:

Jest v22.1.2 node v7.4.0 linux/amd64

 PASS  ./concurrent-test.js
  try running parallel by scenario level
    ✓ First Test (3ms)
    ✓ Second Test
    ✓ Third Test (1ms)

  console.log concurrent-test.js:8
    Test 1

  console.log concurrent-test.js:12
    Test 2

  console.log concurrent-test.js:16
    Test 3

  console.log concurrent-test.js:4
    BEFORE

  console.log concurrent-test.js:20
    AFTER

  console.log concurrent-test.js:4
    BEFORE

  console.log concurrent-test.js:20
    AFTER

  console.log concurrent-test.js:4
    BEFORE

  console.log concurrent-test.js:20
    AFTER

Test Suites: 1 passed, 1 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        0.993s, estimated 1s
Ran all test suites.
luxflow commented 5 years ago

https://github.com/facebook/jest/issues/4281 dupliacted issue it would be very nice if jest fix it

Mark1626 commented 4 years ago

Think this is the RCA

jest-circus

https://github.com/facebook/jest/blob/c9c8dba4dd8de34269bdb971173659399bcbfd55/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts#L84-L90

jest-jasmine https://github.com/facebook/jest/blob/c9c8dba4dd8de34269bdb971173659399bcbfd55/packages/jest-jasmine2/src/jasmineAsyncInstall.ts#L172-L176

Mark1626 commented 4 years ago

@SimenB It seems easy to change the workflow in jest-circus to fix this, I've made a POC. Let me know if this approach would be a good solution

Mark1626 commented 4 years ago

But not sure if Before and After make sense for test.concurrent, if I perform a stateful operation in the hooks then the tests will be indeterministic

csojinb commented 4 years ago

@Mark1626 beforeEach and afterEach might not make a ton of sense for test.concurrent, but beforeAll and afterAll would I think.

Mark1626 commented 4 years ago

@SimenB bump

joshuambg commented 4 years ago

Please fix.

SpadarShut commented 3 years ago

Looks like adding an empty not concurrent test before concurrent ones fixes the issue. At least with beforeAll.

UPD: no, this doesn't work :/

rhyek commented 3 years ago

Looks like adding an empty not concurrent test before concurrent ones fixes the issue. At least with beforeAll.

How did you set up your test suite? I have something like the following which is not working:

describe('...', () => {
  beforeAll(async () => {
    // do some setup
  });

  afterAll(async () => {
    // do teardown
  });

  it('noop', () => {
    expect(2+2).toBe(4);
  });

  it.concurrent('...', async () => {
    // runs before beforeAll
  });

  it.concurrent('...', async () => {
    // runs before beforeAll
  });
});
SpadarShut commented 3 years ago

Looks like adding an empty not concurrent test before concurrent ones fixes the issue. At least with beforeAll.

How did you set up your test suite? I have something like the following which is not working:

I ended up just running the async preparation code in the test file before describe block. I guess this is not ideal, though.

Another workaround is to make a promise which resolves after setup is ready and await for it in every concurrent test:

let beforeAllResolve;

const beforeAllPromise = new Promise((resolve) => {
  beforeAllResolve = resolve;
});

describe('...', () => {
  beforeAll(async () => {
    // do some setup
    // RESOLVE
    beforeAllResolve()
  });

  afterAll(async () => {
    // do teardown
  });

  it.concurrent('...', async () => {
     await beforeAllPromise;
     // do stuff
  });

  it.concurrent('...', async () => {
     await beforeAllPromise;
     // do stuff
  });
});
benminer commented 2 years ago

Has there been any progress on this? Would prefer not to rework hundreds of lines of tests 😅

igor-q-bio commented 2 years ago

The issue still persists. I regret I recommended our team to use Jest when test.concurrent does not support .beforeAll() and .afterAll() aren't working as expected. This should be a high priority issue.

RomanShmandrovskyi commented 1 year ago

I found some workaround to execute beforeAll before test.concurrent. Seems like the problem is not with test.concurrent logic but with describe. If we move beforeAll out from describe, everything works as expected:

let str;

beforeAll(() => {
   str = 'Some str with number 3';
});

describe('describe description', () => {
   test.concurrent('concurrent tests', () => {
      expect(str).toBe(`Some str with number ${Math.floor(Math.random() * 5) + 1}`);
   });
});

But beforeEach and afterEach still are ignored. Even those, that are in setupFiles or setupFilesAfterEnv configuration files...

gladykov commented 9 months ago

afterEach makes sense for us, because this is here we do screenshots for failed test.