feat(testing): Add utility for faking time #2069
Conversation
|
@KyleJune is this ready for review? |
Yes, all it needs is main to be merged back into it and Deno copyright header to be added. I could do that tonight. If there are no other issues and you decide you want to get it merged, feel free to make those changes yourself. |
Thanks! I'll give it as review. @kt3k please take a look as well. |
There was a problem hiding this comment.
Mostly looks good to me, thank you @KyleJune, this is very useful!
Could I ask you to open a PR to https://github.com/denoland/manual that updates testing chapters with this functionality?
testing/time.ts
Outdated
| import { ascend, RBTree } from "../collections/rb_tree.ts"; | ||
| import { DelayOptions } from "../async/delay.ts"; | ||
|
|
||
| export const _internals = { |
There was a problem hiding this comment.
Why do these need to be exported?
There was a problem hiding this comment.
They are used by the test coverage to verify the real methods are called or not called when they should be. I believe the tests might replace them with stubs so that the real timer functions are never called. I could move them to a separate file with _ prefix to the name so that this constant is never exposed to end users.
There was a problem hiding this comment.
I see, that makes sense, though I can see this not working properly if user's code replaced them before this file is loaded. I think this is fine as is, can you add a comment describing the purpose?
There was a problem hiding this comment.
Sure I can do that tonight and I think I'll just move them to _test_utils.ts. alternatively I could create a new file called _time.ts that could export just it instead.
There was a problem hiding this comment.
_test_utils.ts seems fine 👍
There was a problem hiding this comment.
I thought about this and think _time.ts would be better so that when people import time.ts it won't have to load _test_utils.ts too which contains unrelated internal only utilities and could potentially grow in the future.
There was a problem hiding this comment.
I moved the _internals const to _time.ts and added a comment describing its purpose.
testing/time.ts
Outdated
| } | ||
| } | ||
|
|
||
| /** The fake time replacement for se */ |
There was a problem hiding this comment.
Seems like this comment was cut-off by mistake
There was a problem hiding this comment.
I'll look into what it was supposed to say tonight. My bad.
There was a problem hiding this comment.
This comment can just be deleted. The function is never exported and the fakeSetTimeout function doesn't have a comment above it either.
There was a problem hiding this comment.
I deleted this comment.
| * Overrides the real Date object and timer functions with fake ones that can be | ||
| * controlled through the fake time instance. |
There was a problem hiding this comment.
Could you add an example in a codeblock?
There was a problem hiding this comment.
Would the example from the README.md file be fine?
There was a problem hiding this comment.
I've added the example from the README.md file.
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Looks like it doesn't have info regarding bdd.ts and mock.ts yet either. I could try updating the manual for those and time.ts this weekend. |
@KyleJune that would be awesome, much obliged 🙏 |
|
We have a few test cases which depend on some specific date time. We currently use |
I went ahead and did that. The following PR branches off of this one. |
|
I've also made the remaining requested changes. I believe this is ready to go now. |
|
@KyleJune Thanks for updating! |
Depends on: #2048
My previous PR that this one depends on adds
mock.ts. This PR adds a utility for faking time. It mocks time by replacing the Date, setTimeout, clearTimeout, setInterval, clearInterval globals with fake versions that are controlled by the user via theFakeTimeinstance.The
README.mdsection for faking time can be found here.Here are the deno docs for
time.ts.