Uh oh!
There was an error while loading. Please reload this page.
test_runner: introduces a new MockTimers API - #47775
Conversation
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
Review requested:
|
Hey message me (wherever) I am a maintainer of sinon/jest's fake timers and would be happy to give feedback. I'll also leave some on this PR |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
benjamingr
commented
Apr 30, 2023
I'm wondering what the right layer for this is:
It is worth mentioning that places that need fake time/timers like HTTP server date headers does |
benjamingr
commented
Apr 30, 2023
Also cc some sinon folks @SimenB@fatso83@mroderick Also note you can use our tests (super simple) https://github.com/sinonjs/fake-timers/blob/main/test/fake-timers-test.js |
ErickWendel
commented
Apr 30, 2023
That's great @benjamingr gonna check it out soon! I loved the concerns and suggestions surely it's helpful and will help making this feature a complete solution! |
tniessen
commented
May 1, 2023
In that case, I am wondering if there is any advantage of having it in core, besides convenience. If we commit to something like this at the lower levels that you mention, and not just through some monkey-patching in the test runner, we should evaluate spec-compliance and various side-effects. Timers are notoriously tricky in Node.js. |
cjihrig
left a comment
There was a problem hiding this comment.
A few comments:
- I don't think this should be a separate API inside of the test runner, but rather a component of the existing mock functionality.
- I don't think this should change the code of any parts of core other than the test runner.
Uh oh!
There was an error while loading. Please reload this page.
benjamingr
commented
May 1, 2023
Other than convenience, the motivation in several cases is that Node dates/timers use primordials which makes it impossible to mock things like http date headers impossible in userland without hacks. |
7a71461 to
278893aCompareSigned-off-by: Erick Wendel <erick.workspace@gmail.com>
278893a to
ac0fab3Compareljharb
commented
May 1, 2023
@benjamingr wouldn't that defeat the purpose of primordials tho? there shouldn't be a way for later-run code to compromise first-run code. |
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
ed3a873 to
3670374CompareSigned-off-by: Erick Wendel <erick.workspace@gmail.com>
3670374 to
71ffd10CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
407644b to
a0f8afdCompareUh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
Signed-off-by: Erick Wendel <erick.workspace@gmail.com> PR-URL: #47775 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) #47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) #45190 PR-URL: TODO
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) nodejs#48416 doc: * add vmoroz to collaborators (Vladimir Morozov) nodejs#48527 * add kvakil to collaborators (Keyhan Vakil) nodejs#48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) nodejs#48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs#47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs#45190 PR-URL: nodejs#48643
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) #47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) #45190 PR-URL: #48643
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) nodejs#48416 doc: * add vmoroz to collaborators (Vladimir Morozov) nodejs#48527 * add kvakil to collaborators (Keyhan Vakil) nodejs#48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) nodejs#48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs#47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs#45190 PR-URL: nodejs#48643
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) #47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) #45190 PR-URL: #48643
Signed-off-by: Erick Wendel <erick.workspace@gmail.com> PR-URL: nodejs#47775 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) nodejs#48416 doc: * add vmoroz to collaborators (Vladimir Morozov) nodejs#48527 * add kvakil to collaborators (Keyhan Vakil) nodejs#48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) nodejs#48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs#47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs#45190 PR-URL: nodejs#48643
Signed-off-by: Erick Wendel <erick.workspace@gmail.com> PR-URL: nodejs#47775 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) nodejs#48416 doc: * add vmoroz to collaborators (Vladimir Morozov) nodejs#48527 * add kvakil to collaborators (Keyhan Vakil) nodejs#48449 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) nodejs#48518 test_runner: * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs#47775 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs#45190 PR-URL: nodejs#48643
ruyadorno
commented
Sep 10, 2023
This commit has a broken test when cherry-picked into |
Signed-off-by: Erick Wendel <erick.workspace@gmail.com> PR-URL: nodejs#47775 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Signed-off-by: Erick Wendel <erick.workspace@gmail.com> PR-URL: nodejs#47775 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
This PR introduces a new FakeTimers API for the native Node.js test runner.
I'm opening it for review as I've implemented all features for this initial version (it's still missing docs but I think we can start moving forward while I write them)
Backlog
.resetfunction to clear the state.releaseAllTimersfunction to trigger all functions.resetcalls itself after an.ittestAPI:
For the next version:
MockTimers Design API
Usage example: