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-http-client-timeout-option-with-agent #22041

Closed
oyyd opened this issue Jul 31, 2018 · 3 comments
Closed

Investigate flaky test-http-client-timeout-option-with-agent #22041

oyyd opened this issue Jul 31, 2018 · 3 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests.

Comments

@oyyd
Copy link
Contributor

oyyd commented Jul 31, 2018

  • Version: v11.0.0-pre(master)
  • Platform: ubuntu1604_sharedlibs_debug_x64
  • Subsystem: test

https://ci.nodejs.org/job/node-test-commit-linux-containered/5882/nodes=ubuntu1604_sharedlibs_debug_x64/console

11:15:20 not ok 833 parallel/test-http-client-timeout-option-with-agent
11:15:20   ---
11:15:20   duration_ms: 17.334
11:15:20   severity: fail
11:15:20   exitcode: 1
11:15:20   stack: |-
11:15:20     assert.js:84
11:15:20       throw new AssertionError(obj);
11:15:20       ^
11:15:20     
11:15:20     AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
11:15:20     + expected - actual
11:15:20     
11:15:20     - 4
11:15:20     + 3
11:15:20         at ClientRequest.req.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/test/parallel/test-http-client-timeout-option-with-agent.js:46:12)
11:15:20         at ClientRequest.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/test/common/index.js:467:15)
11:15:20         at ClientRequest.emit (events.js:182:13)
11:15:20         at Socket.emitRequestTimeout (_http_client.js:669:40)
11:15:20         at Object.onceWrapper (events.js:273:13)
11:15:20         at Socket.emit (events.js:182:13)
11:15:20         at Socket._onTimeout (net.js:448:8)
11:15:20         at ontimeout (timers.js:454:11)
11:15:20         at tryOnTimeout (timers.js:326:5)
11:15:20         at listOnTimeout (timers.js:300:5)
11:15:20   ...

This test asserts the elapsed time of timeout in second units. But the actual timeout may be longer in some situations.

It seems that other related tests avoid measuring the elapsed time. I would like to modify this test to make it less likely fail and keep the consistency of these tests.
@maclover7 maclover7 added test Issues and PRs related to the tests. flaky-test Issues and PRs related to the tests with unstable failures on the CI. http Issues or PRs related to the http subsystem. labels Jul 31, 2018
This was referenced Aug 2, 2018
Closed
Closed
@Trott
Copy link
Member

Trott commented Aug 3, 2018

The main cause of failure here is almost certainly that it's a debug build, which makes everything run slooooowwwwer.....

oyyd added a commit to oyyd/node that referenced this issue Aug 9, 2018
@oyyd
test: move test-http-client-timeout-option-with-agent to sequential
d30c3cf 
The timeout event cannot be precisely timed and the actual timeout
may be longer in some situations. Here we move this test into the
sequential folder to make it happens less likely.

Fixes: nodejs#22041
@gdams gdams closed this as completed in f8fda89 Aug 13, 2018
rvagg pushed a commit that referenced this issue Aug 15, 2018
@oyyd @rvagg
test: move test-http-client-timeout-option-with-agent to sequential
3804836 
The timeout event cannot be precisely timed and the actual timeout
may be longer in some situations. Here we move this test into the
sequential folder to make it happens less likely.

PR-URL: #22083
Fixes: #22041
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
@Trott
Copy link
Member

Trott commented Aug 19, 2018

Alas, still flaky.

https://ci.nodejs.org/job/node-test-commit-linux-containered/6421/nodes=ubuntu1604_sharedlibs_shared_x64/console

00:15:46 not ok 2230 sequential/test-http-client-timeout-option-with-agent
00:15:46   ---
00:15:46   duration_ms: 5.26
00:15:46   severity: fail
00:15:46   exitcode: 1
00:15:46   stack: |-
00:15:46     assert.js:84
00:15:46       throw new AssertionError(obj);
00:15:46       ^
00:15:46     
00:15:46     AssertionError [ERR_ASSERTION]: Expected inputs to be strictly equal:
00:15:46     
00:15:46     4 !== 3
00:15:46     
00:15:46         at ClientRequest.req.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_shared_x64/test/sequential/test-http-client-timeout-option-with-agent.js:46:12)
00:15:46         at ClientRequest.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_shared_x64/test/common/index.js:460:15)
00:15:46         at ClientRequest.emit (events.js:182:13)
00:15:46         at Socket.emitRequestTimeout (_http_client.js:670:40)
00:15:46         at Object.onceWrapper (events.js:273:13)
00:15:46         at Socket.emit (events.js:182:13)
00:15:46         at Socket._onTimeout (net.js:449:8)
00:15:46         at ontimeout (timers.js:454:11)
00:15:46         at tryOnTimeout (timers.js:326:5)
00:15:46         at listOnTimeout (timers.js:300:5)
00:15:46   ...

@Trott Trott reopened this Aug 19, 2018
@Trott Trott changed the title test: possible flaky test-http-client-timeout-option-with-agent Investigate flaky test-http-client-timeout-option-with-agent Aug 19, 2018
Trott added a commit to Trott/io.js that referenced this issue Aug 19, 2018
@Trott
test: fix flaky http timeout test
90f3e2f 
There is no guarantee that a timeout won't be delayed considerably due
to unrelated activity on the host. Instead of checking that the timeout
happens within a certain tolerance, simply check that it did not happen
too soon.

Fixes: nodejs#22041
@Trott
Copy link
Member

Trott commented Aug 19, 2018

Proposed fix in #22403

@Trott Trott mentioned this issue Aug 19, 2018
Closed
2 tasks
@Trott Trott closed this as completed in fd76a3d Aug 21, 2018
targos pushed a commit that referenced this issue Aug 21, 2018
@Trott @targos
test: fix flaky http timeout test
1bb0346 
There is no guarantee that a timeout won't be delayed considerably due
to unrelated activity on the host. Instead of checking that the timeout
happens within a certain tolerance, simply check that it did not happen
too soon.

Fixes: #22041

PR-URL: #22403
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Sep 3, 2018
@Trott @targos
test: fix flaky http timeout test
5aa3100 
There is no guarantee that a timeout won't be delayed considerably due
to unrelated activity on the host. Instead of checking that the timeout
happens within a certain tolerance, simply check that it did not happen
too soon.

Fixes: #22041

PR-URL: #22403
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@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
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Trott @maclover7 @oyyd