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 test-timers-blocking-callback #9198

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@Trott
Member

Trott commented Oct 20, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test timers

Description of change

This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

/cc @misterdjules @Fishrock123

test: fix flaky test-timers-blocking-callback
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.
@Fishrock123

¯_(ツ)_/¯

@misterdjules

This comment has been minimized.

Contributor

misterdjules commented Oct 20, 2016

This is definitely an improvement, so the changes look good to me. However the test would still be inherently flaky, because there's no guarantee of when the timers will fire regardless of the bug that was fixed by the change that came with this test.

For instance, one recent test failure showed that it took around 1.2 seconds for the test to fail, when in total it should take ~600ms for the test to complete if the process running that program is always on CPU.

In other words, I'd expect these changes to result in this test being less flaky, but still flaky.

@Trott

This comment has been minimized.

@Trott

This comment has been minimized.

Member

Trott commented Oct 20, 2016

OMG, CI is yellow, I'll take it!

Trott added a commit to Trott/io.js that referenced this pull request Oct 21, 2016

test: fix flaky test-timers-blocking-callback
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

PR-URL: nodejs#9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
@Trott

This comment has been minimized.

Member

Trott commented Oct 21, 2016

Landed in 5d818cc

@Trott Trott closed this Oct 21, 2016

jasnell added a commit that referenced this pull request Oct 24, 2016

test: fix flaky test-timers-blocking-callback
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

PR-URL: #9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>

MylesBorins added a commit that referenced this pull request Nov 17, 2016

test: fix flaky test-timers-blocking-callback
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

PR-URL: #9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>

MylesBorins added a commit that referenced this pull request Nov 19, 2016

test: fix flaky test-timers-blocking-callback
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

PR-URL: #9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>

@MylesBorins MylesBorins referenced this pull request Nov 22, 2016

Merged

v6.9.2 proposal #9735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment