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

waitFor doesn't advance time when using jest fake timers without injecting globals #1286

Open
signorettif opened this issue Jan 19, 2024 · 5 comments

Comments

@signorettif
Copy link

signorettif commented Jan 19, 2024

Relevant Code or Config:

A minimal sandbox to reproduce the issue is available on codesandbox. You can execute tests by opening a terminal and running the yarn test command.

Actions Undertaken:

  • Configured jest to not inject globals (--injectGlobals=false); refer to the package.json command from the sandbox above.
  • Created tests that rely on waitFor or waitForElementToBeRemoved.

Outcome:

  • waitFor never advances timers by time (the default 50ms interval of the loop or any other interval), causing the tests to exceed their timeout.

Reproduction:

You can reproduce this issue via the following codesandbox link.

Problem Description:

When using jest, but without injecting globals, the waitFor logic:

  • Fails to identify jest's environment in the jestFakeTimersAreEnabled predicate.
  • Assuming the above was passing correctly, it would reference the jest global object, which is undefined when not injecting globals. This would lead to an error when attempting to invoke jest.advanceTimersByTime.

On a broader scope (although I want to keep this issue's focus narrow for the sake of actionability), there might be other unintended implicit assumptions about depending on (a) jest and / or (b) a test environment that injects globals. The following issues are related, although they focus on different manifested problems:

Suggested Solution:

  • The first part of the issue (i.e., jest environment detection) can be resolved by adding an OR condition to the jestFakeTimersAreEnabled predicate. In my experience, we should add a check for process.env.JEST_WORKER_ID to not be undefined.
  • The second issue could be addressed by dynamically attempting to import jest from @jest/globals when (a) we detect a JEST_WORKER_ID and (b) we know fake timers are enabled.
    • I don't believe dom-testing-library should depend on @jest/globals.
    • I'm not completely satisfied with the tentative dynamic import of an optional dependency @jest/globals in this case, but I think it might provide the best developer experience for end users.

Depending on the appetite for this issue to be fixed, I might work on a PR to address the above. I am somewhat surprised this hasn't come up before, but I guess not many people tend to set up jest without injecting globals.

@krutoo
Copy link

krutoo commented Feb 16, 2024

@signorettif May this ussue be relative to this problem?
testing-library/user-event#1173

@signorettif
Copy link
Author

Yes! I think that's a symptom of the same underlying problem

@krutoo
Copy link

krutoo commented May 23, 2024

@signorettif is there some updates?

@MatanBobi
Copy link
Member

In the end, it all boils down to whether TL should be open to dependency injection of the advance timers function by our users.
There's been some talk about this here:
#939

I didn't get the change to work on that lately.

@nekitboy
Copy link

Now we have api, how to share jest.advanceTimersByTime in user-event library. https://testing-library.com/docs/user-event/options#advancetimers

It would be nice to at least have similar setup option in testing library

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

4 participants