Show stack-trace for failed findBy or waitFor

[sregg]

Describe the Feature

Currently, if your test has a lot of await findBy, if one of them fails, it's hard to know which one from the error message.
It'd be great if the error message would include the line of the findBy that fails, the same way it does for getBy

with findBy:

  ● Testing react navigation › clicking on one item takes you to the details screen

    No instances found

with getBy:

 ● Testing react navigation › clicking on one item takes you to the details screen

    No instances found

      24 |     fireEvent(toClick, 'press');
      25 |     const newHeader = await findByText('Showing details for 5');
    > 26 |     const newBody = getByText('the number you have chosen is 6');

Possible Implementations

I'd be happy to look into the code and see if I can provide a PR if nobody is currently working on this.

[thymikee]

Happy to accept a PR with this improvement! :)

[sregg]

@thymikee has anyone looked into this already?
I don't want to spend hours to find out it's not possible if someone already went through that process 😅

[thymikee]

None that I know of

[sregg]

I spent almost a full day trying to add the stack-traces but I couldn't make it work properly.
It was actually working fine in the internal tests of the lib. Those were already showing some stack-trace, just not the right one.
By surrounding the code in waitFor with a try catch I was able to show the correct stack-trace:

  try {
    let result: T;

    //$FlowFixMe: `act` has incorrect flow typing
    await act(async () => {
      result = await waitForInternal(expectation, options);
    });
    //$FlowFixMe: either we have result or `waitFor` threw error
    return result;
  } catch (error) {
    throw new ErrorWithStack(prepareErrorMessage(error), waitFor);
  }

When I run it in my project, I don't get any stack-trace, before or after that change.

[thymikee]

So you say that "I was able to show the correct stack-trace" and then "I don't get any stack-trace, before or after that change". I don't quite understand what you mean by that 😅. Have you made this change in node_modules of your project and it still didn't work?

[sregg]

I used yalc to get the change in my project.
I validated with some console logs that I was indeed seeing the changes.
What I saw is that in my project, I wouldn't see any stack-trace at all, even without any changes in the lib.
Whereas in the lib's tests, I see a stack-trace (in waitFor) even without any changes.
That's for findBy and waitFor. I see the stack-trace perfectly fine in my project with getBy.
Have you been able to see any stack-trace in your projects for findBy and waitFor?

[thymikee]

Gotta fix it then!

[kevinbror]

Did this every get fixed for waitFor? I'm using @testing-library/[email protected] (published after the fix for findBy) and multiple expects in a waitFor print as a failure at the waitFor line obscuring which one actually failed.

i.e. where expect(queryByTestId("nada")).toBeTruthy(); throws

// renders no buttons
await waitFor(() => {
    const networkButtons = queryAllByTestId(TEST_IDS.NETWORK_BUTTON);
    expect(networkButtons).toHaveLength(0);

    expect(queryByTestId("nada")).toBeTruthy();
});

I get

  ● when loading › renders the skeleton and not the buttons

    expect(received).toBeTruthy()

    Received: null

      55 |
      56 |         // renders no buttons
    > 57 |         await waitFor(() => {
         |               ^
      58 |             const networkButtons = queryAllByTestId(TEST_IDS.NETWORK_BUTTON);
      59 |             expect(networkButtons).toHaveLength(0);
      60 |

      at call (src/components/screens/Profile/__tests__/PublicProfileHeader.test.js:57:15)

*** Update I tested on the latest version and it seems that fix has not been introduced. PRs still welcome to add this?

[AugustinLF]

Hmmm, that doesn't indeed sound ideal. In the perfect case you would be able to know what broke the waitFor, I'm unsure how easy it'd be to do. But I reckon we would be open for such a PR?

However, I've had bad experiences with putting multiple expect in a waitFor and tend to advise to a single one per waitFor. In this case that'd also make it clearer what would fail.

[kevinbror]

ok thanks. If I have some time to tinker with it I will go for a PR. But otherwise I'll go with 1:1 waitFor:expect

[mdjastrzebski]

@kevinbror KCD also recommends using only single assertion for waitFor queries in his Common mistakes with RTL article. The reasoning is as follows:

// ❌
await waitFor(() => {
  expect(window.fetch).toHaveBeenCalledWith('foo')
  expect(window.fetch).toHaveBeenCalledTimes(1)
})

// ✅
await waitFor(() => expect(window.fetch).toHaveBeenCalledWith('foo'))
expect(window.fetch).toHaveBeenCalledTimes(1)

Let's say that for the example above, window.fetch was called twice. So the waitFor call will fail, however, we'll have to wait for the timeout before we see that test failure. By putting a single assertion in there, we can both wait for the UI to settle to the state we want to assert on, and also fail faster if one of the assertions do end up failing.