Skip to content

First test running render/renderHook is slow #1423

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

Closed
KrastanD opened this issue Jun 30, 2023 · 8 comments
Closed

First test running render/renderHook is slow #1423

KrastanD opened this issue Jun 30, 2023 · 8 comments

Comments

@KrastanD
Copy link
Contributor

Describe the bug

The first test in a file that uses render/renderHook is slow. This is exacerbated when ran without jest cache.

Locally on my M1 Pro Macbook:
Without jest cache, the first test takes ~27 times as long as the second test. 1512ms vs 55ms
With jest cache, the first test takes ~3 times as long which seems reasonable. 142ms vs 56ms

On Github Actions:
Without jest cache, the first test takes ~70 times as long as the second test.

I know it's not related to the actual tests because if I change the order of them it is always the one that is first that takes significantly longer.

Expected behavior

I would expect that it takes the tests to run under a second whether they are cached or not, first or not.

Steps to Reproduce

React Query has an example repo using CRA and react testing library with some sample tests here
I recreated the repo and tests using expo and react native testing library here

When running the RTL repo tests without cache I get the following:
image
All tests take under 100 ms.

When running the RNTL repo tests without cache I get the following:
image
Both first tests take 1.6 seconds.

I also ran the RNTL repo tests on Github Actions here with the following results:
image
Interestingly its only the first renderHook that takes a long time of 4.1 seconds.

I created a branch in the RNTL repo called renderHook-isolated. There I created two new test files that don't use react-query or msw - just basic react-native and react native testing library.
Here are the results:
image
Even the very basic react hook test takes 3/4 of a second and it seems react query just makes the matter worse.

From my understanding, renderHook just calls the hook in a 'rendered Test Component so i would assume the issue is in the render` function. I won't pretend to know how it works but perhaps whatever setup jest and RNTL do before the tests is somehow leaking into the first test.

Curious to hear your thoughts and thanks in advance!

Note: This is very specific but I noticed that this issue is resolved if I first use render on any component wrapped in a TamaguiProvider in a separate test in the same file.
Perhaps rendering something as complicated as the TamaguiProvider causes a big enough delay to not leak into the other tests. 🤷

Versions

 npmPackages:
@testing-library/react-native: ^12.1.2 => 12.1.2 
react: 18.2.0 => 18.2.0 
react-native: 0.71.8 => 0.71.8 
@pierrezimmermannbam
Copy link
Collaborator

Hello @KrastanD, thanks for opening this issue! Unfortunately this is expected and I don't know if we can do anything about it. Here is what happens: when we run render for the first time, we start by running a test where we render a Text and a TextInput so that we can retrieve the component names in React Native, then we store those names. They are later used by some queries to target those specific elements. The reason we do this is because those names may change in React Native since they are not part of the public API and we try to avoid depending too much on React Native internals for compatibility reasons.

As you noticed only the first test will be impacted because when the second one runs, we already have stored the component names so it will not have a serious impact on the time it takes for your whole test suite to run. Another solution that we initially thought about was to run the component names detection when a query fail and then tell the user to add some lines to configure them correctly in a jest setup file. This would avoid having to run the detection on each test but it would add some complexity for the users so we thought it was not the best solution.

It could be avoided for renderHook though, it's still executed because renderHook uses render under the hood but we could have a common render and then a specific render for components that would do this kind of stuff that's not needed when testing hook. It's not in our top priorities for now but if you'd like to contribute this should not be too hard.

I hope this makes things clearer, let me know if you have any further questions!

@KrastanD
Copy link
Contributor Author

KrastanD commented Jul 6, 2023

@pierrezimmermannbam thanks for the explanation. The reason I even noticed this was because one of my tests times out on CI during waitFor. Do you have any recommended way to make sure that the test doesn't take too long?

As for a solution, I think the default case can be like now where the first test is slow, but allow users to configure it in the jest setup file. If they have it configured then render can grab the components from there and be quick.

@pierrezimmermannbam
Copy link
Collaborator

@KrastanD is it a waitFor timeout or a jest timeout? If the test takes longer as a whole it may result in a jest timeout but I don't see how it could make a waitFor timeout

@KrastanD
Copy link
Contributor Author

@pierrezimmermannbam It is a waitFor. It times out and returns false instead of true which fails my test. The waitFor for reference:

    await waitFor(() => expect(result.current.isSuccess).toBe(true));

@mdjastrzebski
Copy link
Member

@KrastanD should be fixed in v12.1.3. Pls check.

@KrastanD
Copy link
Contributor Author

Confirming this fixed renderHook being slow. It doesn't fix render being slow so I'll leave it up to you if you want to close this issue or not.

@mdjastrzebski
Copy link
Member

Closing as the renderHook part has been fixed.

he render part, as far as I can tell, can only be fixed by hardcoding core host component names vs. always querying on first render (as is now). The hardcoding approach would improve the speed, as we would not longer have to run detectHostComponentNames but it would also make RNTL less robust due to additional assumption about internal RN host component naming, which has been changed in the past.

@KrastanD
Copy link
Contributor Author

KrastanD commented Aug 6, 2023

@mdjastrzebski What about allowing users to run detectHostComponentNames in setup-jest? This would be optional so for users who don't care it will run as part of the first test, but for those that do it won't affect any individual test while keeping RNTL robust.

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