Console messages emitted from Jest's beforeAll hook do not cause failOnConsole to fail the test

[rszalski]

HI! First of all, thank you for this library, it was a godsend for us when cleaning up our messy unit test report :)

I think I have found a bug that caught us off guard, though. It seems that if the console is used in beforeAll (it might be other Jest hooks too, I just have not confirmed) it doesn't cause the tests to fail.

Our configuration:

failOnConsole({
  shouldFailOnWarn: true,
  shouldFailOnError: true,
  shouldFailOnAssert: true,
  shouldFailOnDebug: true,
  shouldFailOnInfo: true,
  shouldFailOnLog: true,
  silenceMessage: m => {
    // This error is removed in React 18 because it is misleading, so suppressing until we upgrade: https://github.com/reactwg/react-18/discussions/82
    if (m.includes("Can't perform a React state update on an unmounted component.")) {
      return true;
    }
    return false;
  }
});

An example which will emit the following error:

  beforeEach(() => {
    jest.useFakeTimers();
  });

  // `beforeAll` is the cause of the warning, it should be `afterEach` but this illustrates the issue
  beforeAll(() => {
    // Emitted here, but test PASS
    jest.runOnlyPendingTimers();
    jest.useRealTimers();
  });

  it('Emits a warning which correctly fails the test run', () => {
   // This is the same sequence of commands and they also result in a Jest warning. This time tests will FAIL
    jest.runOnlyPendingTimers();
    jest.useRealTimers();
    jest.useFakeTimers();
  });

Please take a look at this code sandbox: https://codesandbox.io/s/jest-fail-on-console-hooks-7rut79?file=/src/index.test.js
Unfortunately, even though Codesandbox supports Jest and the tests run, it doesn't seem to allow us to use any of the fake-timer controlling features due to this bug: codesandbox/codesandbox-client#513. Hopefully, this example is easy enough to reproduce locally.

[ValentinH]

Thanks for the detailed issue report.

Unfortunately, I'm not sure that we can do something about this 😬
Indeed, we are using beforeEach/afterEach to setup/teardown this plugin logic for each test:
https://github.com/ValentinH/jest-fail-on-console/blob/main/index.js#L85-L99

As far as I know, they run after beforeAll so too late in your case.

Conceptually, I'm not sure we could make a test fail because this run before all tests. Therefore, which test should be failed in that case?

That being said, we could at least update the README to explain this limitation.

[rszalski]

@ValentinH in this case I would be ok with not knowing which test failed. The most important thing would be that it fails at all. But I agree it might not be possible with the current architecture.

[flakey-bit]

A related use-case - we're relying on silenceMessage to silence some annoying warning mesages. Sometimes, those warning messages are emited during beforeAll - at the moment, jest-fail-on-console is unable to silence those messages for us.

It would be great if that aspect (at least) could be fixed 🤞

[ValentinH]

Silencing might work if we also register our interceptors in a beforeAll call. Today, we only do it in a beforeEach. Would you be willing to test this and submit a PR if it works?

However, I'm still not sure if failing tests if we see that a non-silenced console is called could be done for the reasons I shared above.

[flakey-bit]

@ValentinH registering the inceptors in beforeAll did help 👍 - see #39

I had issues running the tests locally on main after a yarn install (i.e. before making changes) so I'm unable to check whether the changes impact any tests.

yarn run v1.22.5
$ jest tests/index.test.js
 FAIL  tests/index.test.js
  jest-fail-on-console
    × errors when console.error() is called (27 ms)
    × does not error when console.error() is called but shouldFailOnError is false (23 ms)
    × does not error when console.error() is called and skip test by test file path returns true (23 ms)
    × does not error when console.error() is called and skip test by test name returns true (22 ms)
    × errors when console.assert() is called with a failing assertion (22 ms)
    × does not error when console.assert() is called with a passing assertion (22 ms)
    × does not error if message is silenced with `silenceMessage` (23 ms)
    × does not error if message is silenced with `silenceMessage` based on console method (23 ms)
    × does not error if message is silenced with `silenceMessage` based on group context (23 ms)
    × does not error if message is silenced with `silenceMessage` based on nested group context (23 ms)

  ● jest-fail-on-console › errors when console.error() is called

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

    Expected: StringContaining "Expected test not to call console.error()."
    Received: "'.' is not recognized as an internal or external command,·
    operable program or batch file.·
    "

      21 |     const { stderr } = await runFixture('error')
      22 |
    > 23 |     expect(stderr).toEqual(expect.stringContaining('Expected test not to call console.error().'))
         |                    ^
      24 |   })
      25 |
      26 |   it('does not error when console.error() is called but shouldFailOnError is false', async () => {

      at Object.<anonymous> (tests/index.test.js:23:20)

  ● jest-fail-on-console › does not error when console.error() is called but shouldFailOnError is false

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

    Expected: StringContaining "PASS tests/fixtures/error-disabled/index.test.js"
    Received: "'.' is not recognized as an internal or external command,·
    operable program or batch file.·
    "

[ValentinH]

Thank you for taking the time. That's unfortunate that the tests are not runnable, are you on Windows?

[flakey-bit]

Thank you for taking the time. That's unfortunate that the tests are not runnable, are you on Windows?

Yep

yarn --version
1.22.5
node --version
v18.13.0

[ValentinH]

Ok that's means that some code in the tests system are not compatible with Windows unfortunately.

[mikedidomizio]

We've just hit this issue as well. We merged a PR that didn't catch the React errors that were being logged before the tests started. Now, we're seeing numerous errors during test runs.

The errors occurred before Jest executes beforeEach, which resets the errors it has caught. Then when it runs the afterEach, no errors are reported.

The most important thing would be that it fails at all.

I agree with this, I'd rather have no errors than caring if it's tied to a specific test case. While we'll resolve our current React/jest issues, I'd like to prevent this happening again altogether.

I have a branch that seems to resolve the issue (no tests yet). The way it works is it overrides each console implementation immediately once setup, then in the beforeAll it flushes the console calls out, then business as usual.

Does this change work for you @ValentinH?

[ValentinH]

If the tests are still passing, I'm fine with this change.

[mikedidomizio]

PR is up with a test.

[ValentinH]

Fixed in https://github.com/ValentinH/jest-fail-on-console/releases/tag/v3.3.1