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

Investigate flaky test test-debug-signal-cluster #2476

Closed
joaocgreis opened this issue Aug 20, 2015 · 6 comments · May be fixed by B020239/node#12
Closed

Investigate flaky test test-debug-signal-cluster #2476

joaocgreis opened this issue Aug 20, 2015 · 6 comments · May be fixed by B020239/node#12
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Comments

@joaocgreis
Copy link
Member

Examples of failures:

@joaocgreis joaocgreis added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. labels Aug 20, 2015
@jbergstroem
Copy link
Member

here's example output:

not ok 150 test-debug-signal-cluster.js
# got pids [98570,98575,98576]
# 
# assert.js:89
# throw new assert.AssertionError({
# ^
# AssertionError: test timed out.
# at testTimedOut [as _onTimeout] (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-32/test/parallel/test-debug-signal-cluster.js:54:3)
# at Timer.unrefdHandle (timers.js:301:14)
# > all workers are running
# > Starting debugger agent.
# > Debugger listening on port 12388
# > Starting debugger agent.
# > Starting debugger agent.
# > Debugger listening on port 12389Debugger listening on port 12390

@jbergstroem
Copy link
Member

Looks out some output buffering issue..

@Trott
Copy link
Member

Trott commented Oct 30, 2015

When this happens, the line Debugger listening on port 12389Debugger listening on port 12390 has two line feeds at the end. So, somehow, a \n is being misplaced to the end of both lines rather than between the 9 and the D...

@Trott
Copy link
Member

Trott commented Oct 31, 2015

In lib/_debugAgent.js, changing this...

process._rawDebug('Debugger listening on port %d', addr.port);

...to this...

console.error('Debugger listening on port %d', addr.port);

makes the test pass 1000 times in a row. Previous runs got 50-100 failures every 1000 runs.

Here it is running 1000 times in a row successfully:
https://ci.nodejs.org/job/node-test-commit-smartos/182/

And here is a more typical run with the current code base. 1000 times in a row with around a dozen failures on each SmartOS:
https://ci.nodejs.org/job/node-test-commit-smartos/170/

Running tests right now to see if this change blows up any of the other tests and it looks good. PR coming in a few minutes...

@Trott
Copy link
Member

Trott commented Oct 31, 2015

Update: Looks like it blows up on Windows. Easy enough to deal with. PR still coming in a little bit...

@Trott
Copy link
Member

Trott commented Oct 31, 2015

PR: #3615

Trott added a commit to Trott/io.js that referenced this issue Nov 4, 2015
SmartOS sometimes munges output from `process._rawDebug()`
but Windows can't use `console.error()` in the debug agent.
So inject logic to chose the right thing based on platform.

Fixes: nodejs#2476
Trott added a commit to Trott/io.js that referenced this issue Nov 9, 2015
SmartOS does not line buffer stderr by default, or at least that is the
behavior on the Node project Jenkins server. Force line buffering. This
resolves the flakiness observed on SmartOS for
test-debug-signal-cluster.

Fixes: nodejs#2476
Refs: nodejs#3615
@Trott Trott closed this as completed in 0966ab9 Nov 11, 2015
Trott added a commit that referenced this issue Nov 11, 2015
SmartOS does not line buffer stderr by default, or at least that is the
behavior on the Node project Jenkins server. Force line buffering. This
resolves the flakiness observed on SmartOS for
test-debug-signal-cluster.

PR-URL: #3701
Fixes: #2476
Refs: #3615
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 22, 2016
SmartOS does not line buffer stderr by default, or at least that is the
behavior on the Node project Jenkins server. Force line buffering. This
resolves the flakiness observed on SmartOS for
test-debug-signal-cluster.

PR-URL: #3701
Fixes: #2476
Refs: #3615
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 22, 2016
SmartOS does not line buffer stderr by default, or at least that is the
behavior on the Node project Jenkins server. Force line buffering. This
resolves the flakiness observed on SmartOS for
test-debug-signal-cluster.

PR-URL: #3701
Fixes: #2476
Refs: #3615
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 2, 2016
SmartOS does not line buffer stderr by default, or at least that is the
behavior on the Node project Jenkins server. Force line buffering. This
resolves the flakiness observed on SmartOS for
test-debug-signal-cluster.

PR-URL: #3701
Fixes: #2476
Refs: #3615
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
3 participants