wave icon indicating copy to clipboard operation
wave copied to clipboard

DOM behaviour is not always properly emulated when testing

Open martimalek opened this issue 4 years ago • 4 comments

When doing integration tests that use userEvent we are not waiting for the DOM "update" but directly expecting behaviour.

  • @freenow/wave version: 1.14.0

Relevant code (this is just a pseudo-code example)

In some cases we are using userEvent like this

// Simulate an input change
userEvent.type(input, 'some text');

expect(something).toHaveHappened();

We should instead do this in order to correctly emulate the behaviour the user would see in the browser

// Simulate an input change
userEvent.type(input, 'some text');

await waitFor(() => expect(something).toHaveHappened());

What was expected to happen?

When running tests this warning appears Warning: An update to X inside a test was not wrapped in act(...).

After the PR for this issue is merged we should not see this warning anymore while running tests

Depends on #163

martimalek avatar Sep 22 '21 15:09 martimalek

I will probably be able to create a PR for this issue in 1-2 weeks, but if somebody wants to do it before feel free to do it

martimalek avatar Sep 22 '21 15:09 martimalek

Hello folks, after a testing session with @christopherrolfe198 we found that the root cause of this warning is in the useLocaleObject hook (which is being used in DatepickerSingleInput and in DatepickerRangeInput, which are the two tests that produced this warning).

Basically (Chris correct me if I'm wrong) the issue is that there is an async action (importing a file) which when finished sets some state and this state is trying to be set no matter if the component is mounted or not.

The async action:

import(`date-fns/locale/${currentLocale}/index.js`)
     .then((importedLocaleObject: { default: Locale }) => setLocaleObject(importedLocaleObject.default))

Some options we came up with:

  • Creating a new loading state for the DatePickers that is true while this async action is happening
  • Mocking the useLocaleObject hook in our tests (I don't think this really solves the issue but it does get rid of the warning)
  • Creating a flag in the useLocaleObject hook based on if the component is mounted or not and only make changes in the state if it is (this seems more of a workaround rather than a fix)

Let me know what you think, this were just some possible solutions that we came up with on the fly so there are probably better options. @nlopin @snapsnapturtle @jonotrujillo

martimalek avatar Oct 08 '21 12:10 martimalek

@martimalek any progress with this topic?

rafael-sepeda avatar Oct 28 '21 09:10 rafael-sepeda

@rafael-sepeda My idea was to start a conversation in this issue to decide which option is the best for us, I'm waiting for other's inputs

martimalek avatar Nov 02 '21 11:11 martimalek