-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
"Modern" fake-timer implementation doesn't work with PromiseJS. #10221
Comments
Summary: We weren't exposing a mock for the batched bridge, which resulted in the WebSocket test failure but only in the open source copy of react-native. public Reviewed By: voideanvalue, vjeux Differential Revision: D2782151 fb-gh-sync-id: e896097dd6060bc26963bc4c23db87b7277b3eba
I have the same error with nock 12.0.3 and jest 26.1.0 The test case
Failing :
It's working when I replace
|
Hmm, I wonder if this is because we fake As an aside, one should never replace |
The test with nock is working fine after doing it. |
Yep—when I do this, it works! Looks like sinonjs/fake-timers#323 is related.
Thanks for the heads-up! Yeah, a priori, I'm not really interested in replacing |
Opened facebook/react-native#29303 to find out more about React Native's motives here. 🔍 |
Unless otherwise noted, this commit will be unnecessary when jacebook/jest#10221 is resolved.
Trying this as a workaround for jestjs/jest#10221; see https://github.com/chrisbobbe/react-native/tree/pr-0.61.5-zulip for the contents. Currently breaks `tools/test android`.
Trying this as a workaround for jestjs/jest#10221; see https://github.com/chrisbobbe/react-native/tree/pr-0.61.5-zulip for the contents. If we actually use this, we shouldn't use the fork associated with my GitHub account, but rather zulip/react-native, as we discuss in our doc: https://github.com/zulip/zulip-mobile/blob/2c1caa2a2aa5e574fca3acfb4d4fda0ca2adde5c/docs/architecture/our-rn.md Currently breaks `tools/test android`; looks like we have a doc that may help us through that, at https://github.com/zulip/zulip-mobile/blob/master/docs/howto/rn-from-git.md. The "Android" section links to an upstream doc that looks interesting for this.
After waisting a day troubleshooting why promises weren't working, this worked! Thank you. I think Jest should resort back to the way Lolex was before. See sinonjs/fake-timers#114 for more context as to why Lolex didn't fake |
Filtering out 'nextTick' works for native promises and should really be the default behaviour. Is there any workaround in the meantime for this without tinkering in node_modules to make this usable in CI/CD until a fix is out? |
@squareloop1 I'd recommend using this nifty little tool. I just used it to patch Jest to the working fake-timers code. The patch even works seamlessly when updating a package. It's unfortunate, but I've also had to use it on several other packages that I'm using in my React Native app. A complete life saver. |
@tomdaniel0x01 thanks, I think I will use fake timers from sinon for now, as they seemed to work. But I'd would rather get rid of another dependency and use it in jest. Its the only I am really missing in jest. I have to add to this issue for clarity: It does not only affect non-native global.promise replacements, it also makes native promises in Node.JS with TypeScript stop working. |
It looks like this line was introduced in 3ff3987, in 2015, and it has remained in a similar form since then. I haven't found any explanation for it. At jestjs/jest#10221 [1], a core Jest maintainer says, """ As an aside, one should never replace `global.Promise` [...]. E.g. when using `async-await` you will always get the native `Promise` regardless of the value of `global.Promise`. """ jestjs/jest#10221 is one issue this line has raised, for anyone using the latest features of Jest to test async code in their React Native projects. [1] jestjs/jest#10221 (comment) Fixes: facebook#29303
It looks like this line was introduced in 3ff3987, in 2015, and it has remained in a similar form since then. I haven't found any explanation for it. At jestjs/jest#10221 [1], a core Jest maintainer says, """ As an aside, one should never replace `global.Promise` [...]. E.g. when using `async-await` you will always get the native `Promise` regardless of the value of `global.Promise`. """ jestjs/jest#10221 is one issue this line has raised, for anyone using the latest features of Jest to test async code in their React Native projects. [1] jestjs/jest#10221 (comment) Fixes: facebook#29303
It looks like this line was introduced in 3ff3987, in 2015, and it has remained in a similar form since then. I haven't found any explanation for it. At jestjs/jest#10221 [1], a core Jest maintainer says, """ As an aside, one should never replace `global.Promise` [...]. E.g. when using `async-await` you will always get the native `Promise` regardless of the value of `global.Promise`. """ jestjs/jest#10221 is one issue this line has raised, for anyone using the latest features of Jest to test async code in their React Native projects. [1] jestjs/jest#10221 (comment) Fixes: facebook#29303
It looks like this line was introduced in 3ff3987, in 2015, and it has remained in a similar form since then. I haven't found any explanation for it. At jestjs/jest#10221 [1], a core Jest maintainer says, """ As an aside, one should never replace `global.Promise` [...]. E.g. when using `async-await` you will always get the native `Promise` regardless of the value of `global.Promise`. """ jestjs/jest#10221 is one issue this line has raised, for anyone using the latest features of Jest to test async code in their React Native projects. [1] jestjs/jest#10221 (comment) Fixes: facebook#29303
It looks like this line was introduced in 3ff3987, in 2015, and it has remained in a similar form since then. I haven't found any explanation for it. At jestjs/jest#10221 [1], a core Jest maintainer says, """ As an aside, one should never replace `global.Promise` [...]. E.g. when using `async-await` you will always get the native `Promise` regardless of the value of `global.Promise`. """ jestjs/jest#10221 is one issue this line has raised, for anyone using the latest features of Jest to test async code in their React Native projects. [1] jestjs/jest#10221 (comment) Fixes: facebook#29303
Trying this as a workaround for jestjs/jest#10221; see https://github.com/chrisbobbe/react-native/tree/pr-0.61.5-zulip for the contents. If we actually use this, we shouldn't use the fork associated with my GitHub account, but rather zulip/react-native, as we discuss in our doc: https://github.com/zulip/zulip-mobile/blob/2c1caa2a2aa5e574fca3acfb4d4fda0ca2adde5c/docs/architecture/our-rn.md Currently breaks `tools/test android`; looks like we have a doc that may help us through that, at https://github.com/zulip/zulip-mobile/blob/master/docs/howto/rn-from-git.md. The "Android" section links to an upstream doc that looks interesting for this.
This is much easier than forking React Native, as we considered in zulip/react-native#5. See jestjs/jest#10221 (comment).
I created a pull request to add the options object with an ignore property (notToFake) 100% inspired by the comment above. Any feedback is welcome and greatly appreciated. The added code is kept deliberately simple and pragmatic, it can still be changed easily to reflect any input. |
@jschoubben Thank you! This would be so great if it got in |
Is this moving along? As issue still persists. |
facing same issue. Have we fixed this issue or it's still in progress? |
What's the status? Will it be fixed? |
+1 |
Possible workaround for the time being is using https://github.com/sinonjs/fake-timers, as it was mention before. But here's the example for those who stumbles across this thread:
|
See this issue jestjs/jest#10221 for progress on support for the default 'modern' timer with react-native
Updating documentation for jest >= 27 since the 'modern' useFakeTimers does not work (yet) with react-native: jestjs/jest#10221 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [X] Updated documentation - [ ] Ensured that CI passes
Updating documentation for jest >= 27 since the 'modern' useFakeTimers does not work (yet) with react-native: jestjs/jest#10221 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [X] Updated documentation - [ ] Ensured that CI passes
I tried using this but I keep getting jest warnings: console.warn EDIT: Event after removing the waitFor it doesnt really work / advance time as it should and i get stuck on the fetch request. |
@dagadbm can you share more of your code? It's hard to see how your problem is related to this issue without more context |
https://github.com/dagadbm/load-monitoring-challenge This component is the one I wanted to do an integration test on: https://github.com/dagadbm/load-monitoring-challenge/blob/master/front-end/src/features/metrics/Metrics.tsx#L24 As you can see we have a useEffect that dispatches a fetch call and also a dispatch for a pooling mechanism using setInterval every This is the fetch code: https://github.com/dagadbm/load-monitoring-challenge/blob/master/front-end/src/features/metrics/metricsAPI.ts I basically wanted to create a test that I mock the requests with MSW and then use jest fake timers (or sinon fake timers). jest timers make the fetch to not even resolve. To have an idea of what I wanted to do you can check out this test I did where I manually do a "fake pooling" by dispatching actions manually on the store: https://github.com/dagadbm/load-monitoring-challenge/blob/master/front-end/src/features/metrics/Notification.test.tsx#L46 However, to really be integration, I would have loved for the setInterval to actually run so I dont have to manually dispatch all the actions to the store.
I hope I wasnt too overwhelming. Let me know if I can be more explicit. This is my test-utils file for the custom render function I copied from redux toolkit: https://github.com/dagadbm/load-monitoring-challenge/blob/master/front-end/src/test-utils.tsx |
@dagadbm This issue is about the fact that jest fake timers do not work with promises, but in the example code you shared it looks like you are still using jest's fake timers ( However it seems that the issue you are seeing is more related to react-testing-library. I'm assuming that the error you were seeing (
And here's an example. Code similar to yours with working tests
|
Updating documentation for jest >= 27 since the 'modern' useFakeTimers does not work (yet) with react-native: jestjs/jest#10221 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [X] Updated documentation - [ ] Ensured that CI passes
So Is this gonna be fixed any time soon so that we don't need to rely on |
Finally fixed in #12572, available in https://github.com/facebook/jest/releases/tag/v28.0.0-alpha.8. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Awesome work on #7776, thanks for that!!
🐛 Bug Report
I'm using Jest 26.1.0.
Testing async code with Promises, as described in this Jest doc, doesn't seem to work with
jest.useFakeTimers('modern');
(from #7776; documented here) if you're using the 'promise' library from NPM.For a while (starting in facebook/react-native@3ff3987), React Native has used that library as part of their Jest setup (which you get if you use
preset: 'react-native'
), so I expect this will affect most people using React Native (this is how I encountered it).Otherwise, I'm not sure how often people use that 'promise' library, but it's an official solution given in a Jest troubleshooting doc, and pops up as a workaround for some issues in this repo (e.g. here).
This is arguably a regression (or would be when 'modern' becomes the default), as this behavior isn't observed with
jest.useFakeTimers('legacy');
orjest.useRealTimers()
.To Reproduce
E.g.,
Expected behavior
I expect the test to pass. Instead, five seconds go by and I get this failure:
Link to repl or repo (highly encouraged)
In the repro linked below, I've closely imitated all the tests from the sections in the "Testing Asynchronous Code" doc that use Promises, to suggest that the failure happens in usual, well-documented ways of testing (provided you're using PromiseJS).
In its current state, you should observe these timeout errors on tests that use Promises (just run
yarn
andyarn test
).Try uncommenting
global.Promise = require('promise');
at the top offetchData.test.js
; the tests should all pass.Try doing something other than
jest.useFakeTimers('modern');
; the tests should all pass.(Incidentally, note that accidentally calling
jest.useFakeTimers
multiple times with different arguments in the same file seems disruptive and might hamper your investigation. Also, be sure to comment outjest.runAllTimers()
if you're using real timers.)https://github.com/chrisbobbe/jest-async-modern-timers-repro
envinfo
The text was updated successfully, but these errors were encountered: