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

Calling jest.resetAllMocks() disables the check #10

Closed
codyzu opened this issue Jan 5, 2022 · 10 comments
Closed

Calling jest.resetAllMocks() disables the check #10

codyzu opened this issue Jan 5, 2022 · 10 comments

Comments

@codyzu
Copy link

codyzu commented Jan 5, 2022

A pattern common in my tests is to call jest.resetAllMocks() in a beforeEach block to help ensure that tests are truly independent and don't depend on any state from other tests.

I found that this disables the magic from this package. This seems obvious looking at the source since it depends on jest spyOn mocks.

To re-enable the magic, I found that I can call failOnConsole() after any beforeEach block that called jest.resetAllMocks().

  1. Does this seem like the sane thing to do? I can't think of any better fix since this package just uses jest to spy on the console, but I'm interested if anyone else has any better ideas.
  2. Should a note be added to the readme to save anyone else some time debugging?
  3. 👆 If yes, I can create PR... let me know
@ValentinH
Copy link
Owner

Thanks for reporting this issue.

This might have been added by #5 , could you please try with v2.0.4 to confirm?

Regarding using jest.resetAllMocks(), I usually use the restoreMocks option:: . Did you try it?

@codyzu
Copy link
Author

codyzu commented Jan 24, 2022

@ValentinH Sorry for the slow turnaround.

I have some updates...

  1. I can call jest.resetAllMocks() in a beforeEach() block in version v2.0.4 without disabling jest-fail-on-console. So your theory about it being introduced by We should only override console methods before each test #5 appears correct.
  2. using the restoreMocks or resetMocks config options works for both versions (v.2.0.4 and v2.1.1). Unfortunately, I prefer to be able to call resetAllMocks explicitly in my test so it's obvious. In the repo where this problem occurred, I think there are some tests that depend on some common mock implementations so it would require a bit of refactoring to use the config options.

I created a repo that tests both versions v.2.0.4 and v2.1.1. You can see it, clone, and test from here: https://github.com/codyzu/test-jest-fail-on-console

@ValentinH
Copy link
Owner

Thanks for the details answer.

I think we could partially revert what has been changed in #5. If I understand correctly this PR did 2 things:

  • enabling the custom console logic in a beforeEach and removing it in an afterEach
  • using jest.spyOn() to set the custom logic

if we only revert the second one, meaning setting the custom logic "manually", it should fix this issue while still fixing the original issue of #5.

What do you think ?

@swavans
Copy link

swavans commented Feb 2, 2022

My jest setup looks llike this:
`import failOnConsole from 'jest-fail-on-console'

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

failOnConsole();`
And it works fine!
When the failOnConsole() was before the beforeEach it didn't though.

@codyzu
Copy link
Author

codyzu commented Feb 7, 2022

Thanks for the details answer.

I think we could partially revert what has been changed in #5. If I understand correctly this PR did 2 things:

  • enabling the custom console logic in a beforeEach and removing it in an afterEach
  • using jest.spyOn() to set the custom logic

if we only revert the second one, meaning setting the custom logic "manually", it should fix this issue while still fixing the original issue of #5.

What do you think ?

@ValentinH Yes, I agree... if we just get rid of the spy usage it should be ok.

While appreciating the author's intentions to use the native spy functionality, it could lead to unexpected conflicts (as it did for me).

Do you need help with this?

@ValentinH
Copy link
Owner

No it's ok, I'll do it. I just need to get back my admin access. I'll probably have it this week.

@ValentinH
Copy link
Owner

The fix was published in v2.2.1

@codyzu
Copy link
Author

codyzu commented Feb 9, 2022 via email

@codyzu
Copy link
Author

codyzu commented Feb 10, 2022

Worked like a charm. Thanks!

@ValentinH
Copy link
Owner

@codyzu thanks for checking 👍 Turns out this version caused issue with older versions of Jest 🙈 Could you please try the version mentioned in this PR to check if it also works for your setup: #14 ? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants