test_runner: recalculate run duration on watch restart #57786
Conversation
|
Review requested:
|
nodejs-github-bot
added
needs-ci
|
|
||
| // Reset the root start time to recalculate the duration | ||
| // of the run | ||
| opts.root.clearStartTime(); |
There was a problem hiding this comment.
Is there any reason not to put this with the other reset logic here?
There was a problem hiding this comment.
Nope, I just drafted the PR based on what I saw while working on the global setup/teardown, was not meant to be ready!
You're right, the section you highlighted is 100% the right place!
I'll push a new commit ASAP!
There was a problem hiding this comment.
@cjihrig The only issue I see is that at that point we're resetting the values for the next run, so we can't reset the time there.
By setting it to zero, the effect is that the value gets recalculated in the postRun (if I'm not mistaken).
The test doesn't go through run again and doesn't recalculate its start time.
I'll take a closer look tomorrow, but I'm afraid that, as things stand, the timer's start needs to be handled at the runner level.
There was a problem hiding this comment.
It would be good to put all of the watch reset logic together if possible. If that means moving the current logic, that seems fine too.
There was a problem hiding this comment.
I agree! As soon as I'm done fighting with Windows watch related tests, I'll take a deep dive into this one 🚀
Re-running a test using the
--watchflag keeps the duration unchanged across the various reruns.I've drafted a trivial implementation.
The test I introduced is flaky by design; I need to consider a better approach even though the number of digits is high and the likelihood of getting the same number in two consecutive runs is really low.