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

manually flush microtasks in afterEach #514

Merged

Conversation

LaurensBosscher
Copy link
Contributor

@LaurensBosscher LaurensBosscher commented Oct 30, 2019

This is the rebase of #440. I've squashed both commits into the original commit.

Original description:

This PR replaces calling async act() in afterEach, with a version that still flushes microtasks, but doesn't wrap with act().

Explanation: If there are any hanging microtasks, that means a test ran without asserting on a valid state. Any async portion that causes updates should be asserted on, or atleast resolved within the scope of a test. This PR should still fire missing act warnings for a test, but won't let it leak to the next one.

This PR replaces calling async act() in afterEach, with a version that still flushes microtasks, but doesn't wrap with act().

Explanation: If there are any hanging microtasks, that means a test ran without asserting on a valid state. Any async portion that causes updates should be asserted on, or atleast resolved within the scope of a test. This PR should still fire missing act warnings for a test, but won't let it leak to the next one.
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a review from one other maintainer, but I think this is acceptable.

@kentcdodds kentcdodds merged commit c48b501 into testing-library:master Oct 30, 2019
@kentcdodds
Copy link
Member

@all-contributors please add @LaurensBosscher code

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @LaurensBosscher! 🎉

@kentcdodds
Copy link
Member

🎉 This PR is included in version 9.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@evoyy
Copy link

evoyy commented Nov 4, 2019

I just upgraded to 9.3.1 and this is causing my test suite to log the following error:

ERROR: true, 'This browser does not have a MessageChannel implementation, so enqueuing tasks via await act(async () => ...) will fail. Please file an issue at https://github.com/facebook/react/issues if you encounter this warning.'

Is this something I need to worry about? I'm running my tests in Chrome using Karma.

@kentcdodds
Copy link
Member

cc @threepointone

@donaldpipowitch
Copy link

This was the only change from v9.3.0 to v9.3.1, right? When I switch between those versions I get An update to Formik inside a test was not wrapped in act(...). in every case where a fireEvent.change is not followed by an await. fireEvent.click seems to be fine. Is this intended behavior?

@eps1lon
Copy link
Member

eps1lon commented Nov 8, 2019

@donaldpipowitch Please open a new issue with a reproducible example. Without code it's hard to tell whether this is intended or not.

@donaldpipowitch
Copy link

@eps1lon I created the bug report: #535

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

Successfully merging this pull request may close these issues.

7 participants