Jest 27: incorrect afterEach order with jest-circus

[mrtnzlml]

💥 Regression Report

In Jest 26, it was possible to set up afterEach callback in a setupFilesAfterEnv script which was correctly called after the tests specific afterEach callbacks. It makes sense to behave like this because it allows us to set up callbacks that are executed after any test was completed (including their specific afterEach callbacks).

It seems that Jest 27 calls the "global" afterEach before the test-specific afterEach which breaks our codebase. It's because some of the tests are calling jest.restoreAllMocks in afterEach and later we are checking (in the global afterEach) whether all console mocks were actually restored or not.

Use case: we are capturing unexpected console logs in a similar way like React, see:

• https://github.com/facebook/react/blob/bb1c8215569f59b70276e8939bd8a59aa7f28f53/scripts/jest/setupTests.js#L159-L160
• our code: https://github.com/adeira/universe/blob/adc8bdf25d253b8bb883a040b6bf0d4bd1b66404/scripts/jest/setupTests.js#L63-L92

Test file:

afterEach(() => {
  jest.restoreAllMocks();
});

Setup file:

afterEach(() => {
  // here we check whether `restoreAllMocks` (or any equivalent which restores console mocks) was called
  // HOWEVER, if this global callback is called first then the test-specific mocks are not restored yet breaking the logic
});

Last working version

Worked up to version: 26.6.3

Stopped working in version: 27.0.1

To Reproduce

Steps to reproduce the behavior:

• setup afterEach callback in setupFilesAfterEnv script
• setup afterEach callback in some test
• check the order of execution of these callbacks (the test specific one should be run before the one from setupFilesAfterEnv)

Expected behavior

The order of afterEach callbacks is not changed from version 26 so it's still possible to register custom global afterEach callback.

Link to repl or repo (highly encouraged)

https://replit.com/@mrtnzlml/jest-afterEach-bug (try to change the Jest versions in package.json and check the difference in the order)

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS 11.2.3
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
  Binaries:
    Node: 16.2.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.14.0 - /usr/local/bin/npm
  npmPackages:
    jest: ^27.0.1 => 27.0.1 

[SimenB]

I'm not sure what makes the most sense. @jeysal?

[mrtnzlml]

@SimenB In case this issue gets dismissed, is there any other way how to have a global afterEach callback so we can achieve the same thing?

[jeysal]

Tbh I would say the previous behavior is rather unexpected (and I certainly didn't know it used to behave that way), because in any other case afterEachs would be executed in the order they are declared.
There is no drop in replacement that comes to my mind to get the previous behavior, only things that require more changes like calling something explicitly at the end of each test file, or monkey patching things. But I suppose the same is true with the old behavior if you wanted your afterEach to be run before the test afterEachs. So maybe this issue should be about giving users a way to control that easily. Or maybe there is already a good enough way? Not sure

[mockdeep]

We ran into this issue as well. In several cases we have afterEach calls in our test files that do some localized cleanup, then we have a global afterEach that does things like clearing out the DOM. Upgrading to Jest 27 breaks a lot of tests for us as our code assumes that they are run in order of locality. In other words, we expect global beforeEach and afterEach callbacks to be the first and last in the chain respectively.

Putting the global afterEach before the local one causes confusing test behavior. Previously, it was a convenience that helped smooth the developer experience between tests. They could largely ignore it. Now it means developers have to be very aware of how it behaves whenever they're writing tests.

[mboudreau]

I'm not sure if this bug I just found is related to this, but I was always under the impression that beforeEach/afterEach are always run in order of their declaration if within the same block. In my code, we have 2 global afterEach, one to do an expect, and another to do a cleanUp. Since the expect can throw, I didn't want to put it in the same block since I wanted the cleanup to run even if the expect fails:

afterEach(() => expect(nock).toBeDone());
afterEach(() => nock.cleanAll());

Here I'm expecting the expect afterEach to be called first, then the cleanAll after that, but it's the inverse. And because of this was giving false positives for our tests. 🤯

[SimenB]

In that specific case, you can probably do

afterEach(() => {
  try {
    expect(nock).toBeDone();
  } finally {
    nock.cleanAll();
  }
});

[mboudreau]

I understand that, but that doesn't "fix" the original issue which is against what the documentation mentions around order of execution.

[SimenB]

Where in the docs is this specified?

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[mrtnzlml]

do not close, please

[wu-json]

Still experiencing this issue with jest-circus... For now we have unblocked ourselves from upgrading past Jest 26 by using explicitly specifying jest-jasmine2 as our test runner.

[satanTime]

Hi @jeysal ,

I would still vote for fixing a bug instead of documenting it.

There are many testing libraries around, which use hooks under the hood, and the expectation is that the order of their execution will be respected, as it is in all other testing frameworks. Not sure why jest decided to invent own way and to cause issues.

Let's take a look at a simple example: we want to backup a global variable for a test, and restore it afterwards.
To avoid copy-pasting of tons of lines of code, we decided to write a helper function:

const backupTest = (newValue: typeof globalVar): void => {
  let backup: typeof globalVar;

  beforeEach(() => {
    backup = globalVar;
    globalVar = newValue;
  });

  afterEach(() => {
    globalVar = backup;
  });
};

The idea is simple, our test should look like that:

describe('globalVar', () => {
  backupTest(1);

  it('equals 1', () => {
    expect(globalVar).toEqual(1);
  });
});

so instead of having 10 lines of code how to backup globalVar for each test, we have just 1.

Now, let's imagine that globalVar isn't primitive, but a complex object and backupTest does only 1 modification, whereas we need an additional modification, for example, another group of stub members.

In all testing libraries it will look like that:

describe('globalVar', () => {
  backupTest(1); // one group of stub members.
  backupTest(2); // another group of stub members.

  // tests
});

and it works well, after the test, globalVar will have its initial value before the suite, because the first executed afterEach belongs to 2 and the second executed afterEach belongs to 1.

However, with the new changes in jest, globalVar is going to be 1, because of the broken order, because first jest executes afterEach for 1, and then afterEach for 2, which restores a wrong value.

let globalVar = 0;

const backupTest = (newValue: typeof globalVar): void => {
  let backup: typeof globalVar;

  beforeEach(() => {
    backup = globalVar;
    globalVar = newValue;
  });

  afterEach(() => {
    globalVar = backup;
  });
};

describe('backup', () => {
  it('equals 0 before all', () => {
    expect(globalVar).toEqual(0);
  });

  describe('globalVar', () => {
    backupTest(1); // setting globalVar to 1 and restoring it to 0 afterwards
    backupTest(2); // setting globalVar to 2 and restoring it to 1 afterwards

    it('equals 2 before each', () => {
      expect(globalVar).toEqual(2);
    });

    describe('each', () => {
      backupTest(3); // setting globalVar to 3 and restoring it to 2 afterwards
      backupTest(4); // setting globalVar to 4 and restoring it to 3 afterwards

      it('equals 4 after each', () => {
        expect(globalVar).toEqual(4);
      });
    });

    it('resets to 2 after each', () => {
      expect(globalVar).toEqual(2);
    });
  });

  it('resets to 0 after all', () => {
    expect(globalVar).toEqual(0);
  });
});

which fails on jest as

    expect(received).toEqual(expected) // deep equality

    Expected: 0
    Received: 1

      42 |
      43 |   it('resets to 0 after all', () => {
    > 44 |     expect(globalVar).toEqual(0);
         |                       ^
      45 |   });
      46 | });
      47 |

proposed fix long time ago: #12861

[frosas]

I was recently affected by this unexpected behavior too.

In my case, it was caused by different testing utilities using beforeEach/afterEach to patch console.error:

• https://github.com/ValentinH/jest-fail-on-console (code) cc @ValentinH

• https://github.com/testing-library/react-hooks-testing-library (code) cc @mpeyper

• Own code used to hide React's "Can't perform a React state update on an unmounted component" warning, which looked more or less like this:

  let originalConsoleError: typeof console.error
  
  beforeEach(() => {
    originalConsoleError = console.error
    console.error = (...args) => {
      const reactUpdateOnUnmountedWarning = `Can't perform a React state update on an unmounted component`
      if (String(args[0]).includes(reactUpdateOnUnmountedWarning)) return
      originalConsoleError.call(console, ...args)
    }
  })
  
  afterEach(() => {
    console.error = originalConsoleError
  })

I wonder what's the recommended approach to address scenarios like this.

The best approach I could come up with is to not unpatch console.error at all (not tested):

let shouldHideReactUpdateOnUnmountedWarning = false

beforeAll(() => {
  const originalConsoleError: typeof console.error
  console.error = (...args) => {
    const reactUpdateOnUnmountedWarning = `Can't perform a React state update on an unmounted component`
    if (
      shouldHideReactUpdateOnUnmountedWarning &&
      String(args[0]).includes(reactUpdateOnUnmountedWarning)
    ) {
      return
    }
    originalConsoleError.call(console, ...args)
  }
})

beforeEach(() => {
  shouldHideReactUpdateOnUnmountedWarning = true
})

afterEach(() => {
  shouldHideReactUpdateOnUnmountedWarning = false
})

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[mrtnzlml]

This is still relevant. 😎

[satanTime]

not stale