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
Add options object with ignore property to modern useFakeTimers function #11661
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11661 +/- ##
=======================================
Coverage 69.01% 69.01%
=======================================
Files 312 312
Lines 16335 16336 +1
Branches 4734 4734
=======================================
+ Hits 11273 11274 +1
Misses 5034 5034
Partials 28 28
Continue to review full report at Codecov.
|
Any chance of this one getting merged? Seems pretty straightforward, and non-breaking. One suggestion would be to add documentation about the new options arg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and sorry about the wait!
This needs an integration test showing it works from jest.useFakeTimers
, not just a unit test within @jest/fake-timers
.
At that point you'll see this doesn't work as jest.useFakeTimers
takes a string (modern
or legacy
) as its first arg - you'll need to makes some changes here: https://github.com/facebook/jest/blob/7dd17d541bcdb4d17d96b53586949fb195294040/packages/jest-runtime/src/index.ts#L1904-L1912
This also needs to work when config is passed, not just configured at runtime (although runtime should override config)
It also needs docs 🙂
// This is a simple attempt to fix the issue when using fakeTimers in combination with promises | ||
// Developers can now add the nextTick function to the list timers not to mock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This is a simple attempt to fix the issue when using fakeTimers in combination with promises | |
// Developers can now add the nextTick function to the list timers not to mock |
this text is for a PR description, not the code 🙂
const toFake = Object.keys(this._fakeTimers.timers).filter( | ||
// This is a simple attempt to fix the issue when using fakeTimers in combination with promises | ||
// Developers can now add the nextTick function to the list timers not to mock | ||
timerFunc => !(options?.ignore || []).includes(timerFunc), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is sorta hard to read, what about
diff --git i/packages/jest-fake-timers/src/modernFakeTimers.ts w/packages/jest-fake-timers/src/modernFakeTimers.ts
index 59c4c49321..049982792b 100644
--- i/packages/jest-fake-timers/src/modernFakeTimers.ts
+++ w/packages/jest-fake-timers/src/modernFakeTimers.ts
@@ -12,6 +12,12 @@ import {
} from '@sinonjs/fake-timers';
import {StackTraceConfig, formatStackTrace} from 'jest-message-util';
+type ValidTimers = keyof FakeTimerWithContext['timers'];
+
+interface ModernFakeTimersOptions {
+ ignore?: Array<ValidTimers>;
+}
+
export default class FakeTimers {
private _clock!: InstalledClock;
private _config: StackTraceConfig;
@@ -93,16 +99,20 @@ export default class FakeTimers {
}
}
- useFakeTimers(): void {
+ useFakeTimers(options?: ModernFakeTimersOptions): void {
if (!this._fakingTime) {
- const toFake = Object.keys(this._fakeTimers.timers) as Array<
- keyof FakeTimerWithContext['timers']
- >;
+ const toFake = new Set(
+ Object.keys(this._fakeTimers.timers) as Array<ValidTimers>,
+ );
+
+ options?.ignore?.forEach(timer => {
+ toFake.delete(timer);
+ });
this._clock = this._fakeTimers.install({
loopLimit: this._maxLoops,
now: Date.now(),
- toFake,
+ toFake: Array.from(toFake),
});
this._fakingTime = true;
@jschoubben ping 🙂 |
It would be awesome to get this merged. What if someone from the core team handle it since it's partly done? 😃 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Since version 27, the "modern" useFakeTimers implementation became the default implementation. Unfortunately, this way doesn't support the async/await pattern. This is caused because the process.nextTick function is mocked which causes the async tests to timeout.
Issue can be found here: #10221
Test plan