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.

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-http-client-timeout-with-data #10431

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@Trott
Copy link
Member

commented Dec 23, 2016

Description of change

test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced process.on('exit', ...) checks with
common.mustCall() and replaced usage of assert.equal() with
assert.strictEqual().

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

test http

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.
@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2016

Sample failure in CI:

https://ci.nodejs.org/job/node-test-commit-osx/6810/nodes=osx1010/console:

not ok 1302 sequential/test-http-client-timeout-with-data
  ---
  duration_ms: 0.358
  severity: fail
  stack: |-
    
    assert.js:85
      throw new assert.AssertionError({
      ^
    AssertionError: 0 == 1
        at process.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/sequential/test-http-client-timeout-with-data.js:10:10)
        at emitOne (events.js:101:20)
        at process.emit (events.js:188:7)
@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2016

(Oh, moved back to parallel because it was move to sequential along with a bunch of other tests to mitigate the issue of competing timeouts addressed here.)

@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2016

@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2016

CI stress test showing current version failing under load: https://ci.nodejs.org/job/node-stress-single-test/1094/nodes=osx1010/console

CI stress test showing the version in this PR succeeding under load: https://ci.nodejs.org/job/node-stress-single-test/1095/nodes=osx1010/console

@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2016

/cc @nodejs/testing

Also /cc @bnoordhuis to make sure I'm not undermining some subtlety in the test

@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 29, 2016

Still trying to get a review on this one. Maybe @nodejs/platform-freebsd or @nodejs/http ?

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

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: nodejs#10431
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2016

Landed in 0b33ef8

@Trott Trott closed this Dec 31, 2016

@evanlucas evanlucas referenced this pull request Jan 3, 2017

Merged

v7.4.0 release proposal #10589

evanlucas added a commit that referenced this pull request Jan 3, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

evanlucas added a commit that referenced this pull request Jan 4, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Jan 23, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Jan 23, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Jan 24, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Jan 24, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

This was referenced Jan 24, 2017

MylesBorins added a commit that referenced this pull request Jan 31, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Feb 1, 2017

test: fix flaky test-http-client-timeout-with-data
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.

Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.

Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.

PR-URL: #10431
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.