Skip to content

test: increase test-net-GH-5504 timeout - #9444

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:argh5504
Closed

test: increase test-net-GH-5504 timeout#9444
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:argh5504

Conversation

@Trott

@TrottTrott commented Nov 3, 2016

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

test net

Description of change

test-net-GH-5504 failed on smartos14-64 in CI. The internal test timeout
expired. It is currently 2 seconds. That appears to be somewhat
arbitrary. It may be possible to rewrite the test without the timer. As
an immediate workaround, increase the timeout to 5 seconds.

@TrottTrott added net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. labels Nov 3, 2016
@Trott

Trott commented Nov 3, 2016

Copy link
Copy Markdown
MemberAuthor

@nodejs/testing

@Trott

Trott commented Nov 3, 2016

Copy link
Copy Markdown
MemberAuthor

Example failure: https://ci.nodejs.org/job/node-test-commit-smartos/5059/nodes=smartos14-64/console

not ok 1257 sequential/test-net-GH-5504
# /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/sequential/test-net-GH-5504.js:55
# throw new Error('hang');
# ^
# # Error: hang
# at Timeout._onTimeout (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/sequential/test-net-GH-5504.js:55:13)
# at ontimeout (timers.js:365:14)
# at tryOnTimeout (timers.js:237:5)
# at Timer.listOnTimeout (timers.js:207:5)
---
duration_ms: 2.975

@santigimeno

Copy link
Copy Markdown
Member

I was already running a stress test on CI: https://ci.nodejs.org/job/node-stress-single-test/nodes=smartos14-64/1050/. There's already failures,

@santigimenosantigimeno left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with stress test run green

@richardlau

richardlau commented Nov 3, 2016

Copy link
Copy Markdown
Member

Typo in commit description increate.

@TrottTrott changed the title test: increate test-net-GH-5504 timeouttest: increase test-net-GH-5504 timeoutNov 3, 2016
@Trott

Trott commented Nov 3, 2016

Copy link
Copy Markdown
MemberAuthor

Typo in commit description increate.

Fixed, thanks!

@Trott

Trott commented Nov 3, 2016

Copy link
Copy Markdown
MemberAuthor

@Trott

Trott commented Nov 4, 2016

Copy link
Copy Markdown
MemberAuthor

Increasing the timeout results in a less reliable test, judging from the stress test results. That's puzzling.

test-net-nodejsGH-5504 failed on smartos14-64 in CI. The internal test timeout
expired. It is currently 2 seconds. That appears to be somewhat
arbitrary. It may be possible to rewrite the test without the timer. As
an immediate workaround, increase the timeout to 5 seconds.
@Trott

Trott commented Nov 5, 2016

Copy link
Copy Markdown
MemberAuthor

Closing in favor of #9461

@TrottTrott closed this Nov 5, 2016
@Trott
Trott deleted the argh5504 branch January 13, 2022 22:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

netIssues and PRs related to the net subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott@santigimeno@richardlau@jasnell@cjihrig