-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
test: fix flaky interval test #18140
Conversation
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.
LGTM. This version of the test fails on 9.3.0 and passes on master, which is exactly what we'd expect.
+1 to fast-tracking to unbreak CI |
Thanks everyone. Landed in 8d04323 |
PR-URL: #18140 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Alas, still flaky. https://ci.nodejs.org/job/node-test-commit-linux-containered/1416/nodes=ubuntu1604_sharedlibs_debug_x64/console 03:37:24 not ok 2014 sequential/test-timers-set-interval-excludes-callback-duration
03:37:24 ---
03:37:24 duration_ms: 1.220
03:37:24 severity: fail
03:37:24 stack: |-
03:37:24 assert.js:45
03:37:24 throw new errors.AssertionError(obj);
03:37:24 ^
03:37:24
03:37:24 AssertionError [ERR_ASSERTION]: false == true
03:37:24 at Timeout.setInterval [as _onTimeout] (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/test/sequential/test-timers-set-interval-excludes-callback-duration.js:14:5)
03:37:24 at ontimeout (timers.js:455:11)
03:37:24 at tryOnTimeout (timers.js:304:5)
03:37:24 at Timer.listOnTimeout (timers.js:267:5)
03:37:24 ... |
A test was recently introduced that consistently fails on OS X & a few other machines. This PR reliably fixes it.
CI: https://ci.nodejs.org/job/node-test-pull-request/12522/
Stress test CI: https://ci.nodejs.org/job/node-stress-single-test/1686/
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test