testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.26k stars 466 forks source link

Lots of new act warnings since 9.3.1 #1251

Closed jgarplind closed 1 year ago

jgarplind commented 1 year ago

Relevant code or config:

  it("should group items", async () => {
    renderWithReactQueryAndReactRouter();

    const user = userEvent.setup();
    await user.selectOptions(
      await screen.findByRole("combobox", { name: "group" }),
      ["property"],
    );
    expect(
      screen.getByRole("columnheader", { name: "John Doe" }),
    ).toBeInTheDocument();
  });

What you did:

Re-installed packages for the first time since @testing-library/dom@9.3.1 was released, triggering an upgrade since I use @testing-library/user-event which has a peer dependency: "@testing-library/dom": ">=7.21.4"

This is the different in my pnpm-lock.yaml:

  /@testing-library/dom@9.3.0:
    resolution: {integrity: sha512-Dffe68pGwI6WlLRYR2I0piIkyole9cSBH5jGQKCGMRpHW5RHCqAUaqc2Kv0tUyd4dU4DLPKhJIjyKOnjv4tuUw==}
    engines: {node: '>=14'}
    dependencies:
      '@babel/code-frame': 7.21.4
      '@babel/runtime': 7.22.3
      '@types/aria-query': 5.0.1
      aria-query: 5.1.3
      chalk: 4.1.2
      dom-accessibility-api: 0.5.16
      lz-string: 1.5.0
      pretty-format: 27.5.1

  /@testing-library/dom@9.3.1:
    resolution: {integrity: sha512-0DGPd9AR3+iDTjGoMpxIkAsUihHZ3Ai6CneU6bRRrffXMgzCdlNk43jTrD2/5LT6CBb3MWTP8v510JzYtahD2w==}
    engines: {node: '>=14'}
    dependencies:
      '@babel/code-frame': 7.22.10
      '@babel/runtime': 7.22.10
      '@types/aria-query': 5.0.1
      aria-query: 5.1.3
      chalk: 4.1.2
      dom-accessibility-api: 0.5.16
      lz-string: 1.5.0
      pretty-format: 27.5.1

What happened:

New behaviors, including errors and warnings.

In particular, a lot of new act warnings:

When testing, code that causes React state updates should be wrapped into act(...): act(() => { / fire events that update state / }); / assert on the output / This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act at stacktrace pointing to component

It is really quite pervasive, and could be due to user error, but I am a fairly experience testing library user and have never seen anything like this.

Reproduction:

Seeking to understand if I am being reasonable or barking up the wrong tree before spending time on that, sorry.

Problem description:

Our trust in our tests is significantly reduced since the warnings indicate that they don't operate correctly.

Suggested solution:

Help me understand how 9.3.1, really only pinning aria-query to a slightly older version, could cause all this trouble?

MatanBobi commented 1 year ago

Hi @jgarplind, thanks for opening this. Since act is only related to RTL, the issue you're raising isn't relevant to this library. We do have an issue about this which is related to RTL 14: https://github.com/testing-library/react-testing-library/issues/1216, is that what you're experiencing? If so, I believe we can close this as duplicated and continue the discussion there.

jgarplind commented 1 year ago

Thanks for getting back so soon.

Right, of course act only exists in the React ecosystem, so I can see why this issue would be better handled over there.

Before closing, I just wanted to mention the error that I have worked around by simplifying the test, but which was also caused by the 9.3.0 -> 9.3.1 upgrade:

AssertionError: expected [ <span …(1)></span>, …(2) ] to have a length of 5 but got 3

- Expected
+ Received

- 5
+ 3

Now, this is just one test, and it is non-trivial (including usage of mswjs.io, zod, etc.), but I was still surprised to see it stop working all out of sudden:

  it("can finish activity", async () => {
    renderScheduleWithReactQueryAndReactRouter();
    const user = userEvent.setup();
    const finishButton = await screen.findByRole("button", {
      name: "xxx",
    });
    const numberOfFinishedBefore = screen.getAllByText("finished").length;
    const numberOfUnfinishedBefore = occurrences.filter(
      ({ status }) =>
        status !== occurrenceState.Values.finished,
    ).length;

    server.use(
      rest.get(`*/occurrences`, (req, res, ctx) => {
        return res(
          ctx.body(
            JSON.stringify(
              occurrences.map((a) => {
                if (a.category === category.Values.Task) {
                  return {
                    ...a,
                    status: occurrenceState.Values.finished,
                  };
                }
                return a;
              }),
            ),
          ),
        );
      }),
    );
    await user.click(finishButton);
    expect(await screen.findAllByText("finished")).toHaveLength(
      numberOfFinishedBefore + numberOfUnfinishedPatientTasksBefore,
    );
  });

I suppose this doesn't change much - the issue is better handled where you linked me to, but now at least I have shared most of the information available to me!

jgarplind commented 1 year ago

Then again.. I've been on user-event 14.4.3 and react 18 since before, so I'm not sure the issues are the same, but probably related.

MatanBobi commented 1 year ago

The point is @testing-library/dom 9.3.1 only change was pinning the version of aria-query to a specific version. Is there any way to know what was the previous version of aria-query you were using (probably can be found in the package-lock file if you have one. I don't think those issues are related anyways :)

jgarplind commented 1 year ago

You can see in my initial post that it was the same (5.1.3) which just causes me even more confusion.

But I can consistently "toggle" the warnings just by flipping between 9.3.0 and 9.3.1.

... and when cross-checking again (after playing around a bit with pnpm hooks and whatnot), I now see a different version for 9.3.0, namely aria-query@5.3.0:

  /@testing-library/dom@9.3.0:
    resolution: {integrity: sha512-Dffe68pGwI6WlLRYR2I0piIkyole9cSBH5jGQKCGMRpHW5RHCqAUaqc2Kv0tUyd4dU4DLPKhJIjyKOnjv4tuUw==}
    engines: {node: '>=14'}
    dependencies:
      '@babel/code-frame': 7.22.10
      '@babel/runtime': 7.22.10
      '@types/aria-query': 5.0.1
      aria-query: 5.3.0
      chalk: 4.1.2
      dom-accessibility-api: 0.5.16
      lz-string: 1.5.0
      pretty-format: 27.5.1

  /@testing-library/dom@9.3.1:
    resolution: {integrity: sha512-0DGPd9AR3+iDTjGoMpxIkAsUihHZ3Ai6CneU6bRRrffXMgzCdlNk43jTrD2/5LT6CBb3MWTP8v510JzYtahD2w==}
    engines: {node: '>=14'}
    dependencies:
      '@babel/code-frame': 7.22.10
      '@babel/runtime': 7.22.10
      '@types/aria-query': 5.0.1
      aria-query: 5.1.3
      chalk: 4.1.2
      dom-accessibility-api: 0.5.16
      lz-string: 1.5.0
      pretty-format: 27.5.1

I'm really stumped, but for now I think I will just enforce 9.3.0 using a pnpm hook*, since I believe (and hope for) these warnings to be misplaced.

*For anyone interested, looks like this:

function readPackage(pkg) {
  // This will change any packages using @testing-library/dom as a peer dependency to use @testing-library/dom@9.3.0
  if (pkg.peerDependencies["@testing-library/dom"]) {
    pkg.peerDependencies["@testing-library/dom"] = "9.3.0";
  }

  return pkg;
}

module.exports = {
  hooks: {
    readPackage,
  },
};

as explained here: https://pnpm.io/pnpmfile#hooksreadpackagepkg-context-pkg--promisepkg

domos4 commented 1 year ago

I'm experiencing the very same issue. I'm using vitest and all the libraries listed by OP at the same versions.

jgarplind commented 1 year ago

Just updated to the latest releases of this package as well as user-event, and my workaround is no longer necessary.

Closing.

jgarplind commented 8 months ago

Turns out the issue is with multiple versions.

It hit us again now with 9.3.3 and 9.3.4.

So indeed the original tip to check https://github.com/testing-library/react-testing-library/issues/1216 was relevant, since that discussion has now turned us there.