Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Investigate flaky test-timers-blocking-callback #8041

Closed
Trott opened this issue Aug 9, 2016 · 10 comments
Closed

Investigate flaky test-timers-blocking-callback #8041

Trott opened this issue Aug 9, 2016 · 10 comments
Labels
test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@Trott
Copy link
Member

Trott commented Aug 9, 2016

https://ci.nodejs.org/job/node-test-commit-freebsd/3639/nodes=freebsd10-64/console

not ok 922 parallel/test-timers-blocking-callback
# 
# assert.js:89
#   throw new assert.AssertionError({
#   ^
# AssertionError: false == true
#     at Timeout.blockingCallback [as _onTimeout] (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/parallel/test-timers-blocking-callback.js:52:5)
#     at tryOnTimeout (timers.js:232:11)
#     at Timer.listOnTimeout (timers.js:202:5)
  ---
  duration_ms: 0.659
@Trott Trott added timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. test Issues and PRs related to the tests. freebsd Issues and PRs related to the FreeBSD platform. labels Aug 9, 2016
@Trott
Copy link
Member Author

Trott commented Sep 21, 2016

Also flaky on CentOS 5 (32-bit):

https://ci.nodejs.org/job/node-test-commit-linux/5269/nodes=centos5-32/console

not ok 950 parallel/test-timers-blocking-callback
# 
# assert.js:85
#   throw new assert.AssertionError({
#   ^
# AssertionError: false == true
#     at Timeout.blockingCallback [as _onTimeout] (/home/iojs/build/workspace/node-test-commit-linux/nodes/centos5-32/test/parallel/test-timers-blocking-callback.js:52:5)
#     at tryOnTimeout (timers.js:232:11)
#     at Timer.listOnTimeout (timers.js:202:5)
  ---
  duration_ms: 1.226

@Trott Trott changed the title Investigate flaky test-timers-blocking-callback on FreeBSD Investigate flaky test-timers-blocking-callback Sep 21, 2016
@Trott Trott removed the freebsd Issues and PRs related to the FreeBSD platform. label Sep 21, 2016
@Trott
Copy link
Member Author

Trott commented Sep 21, 2016

/cc @whitlockjc

@Trott
Copy link
Member Author

Trott commented Sep 21, 2016

/cc @Fishrock123, @misterdjules

@misterdjules
Copy link

Thanks for the heads up @Trott! This test is definitely inherently flaky. My apologies for that, as I participated in its design.

I'm looking into finding a way to make it non-flaky, I'll keep you posted.

@mscdex mscdex mentioned this issue Oct 15, 2016
Merged
@MylesBorins
Copy link
Contributor

another failure https://ci.nodejs.org/job/node-test-commit-freebsd/4890/nodes=freebsd10-64/tapTestReport/

@misterdjules misterdjules mentioned this issue Oct 20, 2016
Closed
2 tasks
@Trott
Copy link
Member Author

Trott commented Oct 26, 2016

Three more freebsd10-64 on CI today:

@misterdjules
Copy link

misterdjules commented Oct 26, 2016 •
edited
Loading

@Trott As indicated in one of my recents comments, I expected more failures after the latest changes in that test, so in some ways it's good to have that confirmation recorded here, thanks for that!

@Trott
Copy link
Member Author

Trott commented Oct 26, 2016

Yes, you definitely called it.

(I think I might have a solution soon, but I've definitely thought that before and been wrong, so don't hold me to it.)

@Trott
Copy link
Member Author

Trott commented Oct 26, 2016

Still not sure about a fix, but there's a bug in the test. PR momentarily...

@Trott Trott mentioned this issue Oct 26, 2016
Closed
2 tasks
@Trott
Copy link
Member Author

Trott commented Oct 27, 2016

PR to fix in #9317

Trott added a commit to Trott/io.js that referenced this issue Oct 29, 2016
@Trott
test: fix freebsd10-64 CI failures
8efee55 
Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.

This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.

In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.

Fixes: nodejs#8041
Fixes: nodejs#9227
@Trott Trott closed this as completed in 6ef636c Oct 29, 2016
evanlucas pushed a commit that referenced this issue Nov 3, 2016
@Trott @evanlucas
test: fix freebsd10-64 CI failures
411b133 
Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.

This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.

In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.

Fixes: #8041
Fixes: #9227
PR-URL: #9317
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 22, 2016
@Trott
test: fix freebsd10-64 CI failures
ce05b70 
Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.

This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.

In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.

Fixes: #8041
Fixes: #9227
PR-URL: #9317
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@MylesBorins @Trott @misterdjules