test: deflake test/pummel/test-timers.js

[jakecastelli]

This PR fixes: #55092

Timers are not guaranteed to fire exactly when specified, the flakiness of the test comes from the assumption that the timer will always fire at or after the timeout, however this is not always the case. For instance, a 1000ms timeout could be fired at anytime roughly between 98x ~ 100x due to the CPU load, platform etc.

This could be easily verified by:

let currentTime = Date.now();

setInterval(() => {
  const timeElapsed = Date.now();
  console.log(timeElapsed - currentTime);
  currentTime = Date.now();
}, 1000);

on my macOS nightly build here is the result:

1003
983
1000
1001
999
1001
1001
1000
1001
997
1000
1000

Here is a more precise timing mechanism by using process.hrtime version:

let currentTimeHR = process.hrtime();

setInterval(() => {
  const [s, n] = process.hrtime(currentTimeHR);
  console.log(s * 1000 + n / 1e6);

  currentTimeHR = process.hrtime();
}, 1000);

1001.345709
978.22325
1001.416125
1000.27375
1001.791
999.609291
1000.354209
999.61925
999.512083
1000.2595

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.24%. Comparing base (3c5ceff) to head (d9c53b0).
Report is 39 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55098      +/-   ##
==========================================
- Coverage   88.24%   88.24%   -0.01%     
==========================================
  Files         651      651              
  Lines      183877   183871       -6     
  Branches    35858    35856       -2     
==========================================
- Hits       162266   162253      -13     
  Misses      14901    14901              
- Partials     6710     6717       +7     

see 42 files with indirect coverage changes

[jakecastelli]

stress test: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/542/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62770/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62773/ 💛

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/55098
✔  Done loading data for nodejs/node/pull/55098
----------------------------------- PR info ------------------------------------
Title      test: deflake test/pummel/test-timers.js (#55098)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     jakecastelli:fix-55092 -> nodejs:main
Labels     test, author ready, needs-ci
Commits    3
 - test: deflake test/pummel/test-timers.js
 - fixup! remove unnecessary check
 - fixup! remove more checks
Committers 1
 - jakecastelli <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/55098
Fixes: https://github.com/nodejs/node/issues/55092
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55098
Fixes: https://github.com/nodejs/node/issues/55092
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 24 Sep 2024 11:54:32 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/55098#pullrequestreview-2329521139
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/55098#pullrequestreview-2329678653
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-09-25T23:05:27Z: https://ci.nodejs.org/job/node-test-pull-request/62773/
- Querying data for job/node-test-pull-request/62773/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 55098
From https://github.com/nodejs/node
 * branch                  refs/pull/55098/merge -> FETCH_HEAD
✔  Fetched commits as 668e52339261..d9c53b053e2b
--------------------------------------------------------------------------------
[main d7855da3c6] test: deflake test/pummel/test-timers.js
 Author: jakecastelli <[email protected]>
 Date: Tue Sep 24 21:15:58 2024 +0930
 1 file changed, 2 insertions(+), 2 deletions(-)
[main 15eeb19acd] fixup! remove unnecessary check
 Author: jakecastelli <[email protected]>
 Date: Tue Sep 24 21:54:40 2024 +0930
 1 file changed, 1 insertion(+), 1 deletion(-)
[main 80ea4e4e75] fixup! remove more checks
 Author: jakecastelli <[email protected]>
 Date: Wed Sep 25 11:20:02 2024 +0930
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/6)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: deflake test/pummel/test-timers.js
PR-URL: #55098

Fixes: #55092

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Antoine du Hamel <[email protected]>
[detached HEAD 2607093213] test: deflake test/pummel/test-timers.js

Author: jakecastelli <[email protected]>

Date: Tue Sep 24 21:15:58 2024 +0930

1 file changed, 2 insertions(+), 2 deletions(-)

Rebasing (3/6)

Rebasing (4/6)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

fixup! remove unnecessary check
PR-URL: #55098

Fixes: #55092

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Antoine du Hamel <[email protected]>
[detached HEAD c259a2835d] fixup! remove unnecessary check

Author: jakecastelli <[email protected]>

Date: Tue Sep 24 21:54:40 2024 +0930

1 file changed, 1 insertion(+), 1 deletion(-)

Rebasing (5/6)

Rebasing (6/6)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

fixup! remove more checks
PR-URL: #55098

Fixes: #55092

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Antoine du Hamel <[email protected]>
[detached HEAD 688d654677] fixup! remove more checks

Author: jakecastelli <[email protected]>

Date: Wed Sep 25 11:20:02 2024 +0930

1 file changed, 1 insertion(+), 1 deletion(-)
Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/11065370461

[nodejs-github-bot]

Landed in 66a2cb2

[lpinca]

It is still flaky, see #55092 (comment).