Skip to content

Conversation

@gibfahn
Copy link
Member

They tend to hang if they happen to run in parallel with another test that uses common.PORT.

I missed these when looking at #13580, looks like two of these tests were competing with each other in the last run I did.

I've done a diff against the list in #12376, and these are the parallel debugger tests that contain common.PORT and are no longer in master:

  • test-debug-brk-no-arg.js
  • test-debug-brk.js (test: move test-debug-brk to sequential #13580)
  • test-debug-no-context.js
  • test-debug-port-cluster.js
  • test-debug-port-from-cmdline.js
  • test-debug-port-numbers.js
  • test-debug-signal-cluster.js
  • test-debugger-util-regression.js
grep results:
➜ parallel git:(v6.x-staging) ❯ rg common.PORT `fnd test-debug`~/wrk/com/node/test/parallel test-debug-no-context.js 7:const args = ['--debug', `--debug-port=${common.PORT}`, '--interactive']; test-debug-brk-no-arg.js 6:const child = spawn(process.execPath, ['--debug-brk=' + common.PORT, '-i']); test-debug-port-cluster.js 6:const PORT_MIN = common.PORT + 1; // The fixture uses common.PORT. test-debug-signal-cluster.js 8:const port = common.PORT; test-debug-brk.js 18: const procArgs = [`--debug-brk=${common.PORT}`].concat(extraArgs); 25: const agentArgs = ['debug', `localhost:${common.PORT}`]; test-debug-port-from-cmdline.js 6:const debugPort = common.PORT; test-debug-port-numbers.js 16: const port = common.PORT + i; test-debugger-util-regression.js 16: `--port=${common.PORT}`,
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@gibfahngibfahn added the test Issues and PRs related to the tests. label Jun 10, 2017
@gibfahngibfahn requested a review from TrottJune 10, 2017 00:47
@nodejs-github-botnodejs-github-bot added test Issues and PRs related to the tests. v6.x labels Jun 10, 2017
@gibfahn
Copy link
MemberAuthor

Interested to hear any other method of solving this. I don't think we need to worry too much about these, they'll go away when 6.x goes away. Just trying to avoid any more flakes in CI.

Failures that prompted this:

not ok 920 parallel/test-debug-signal-cluster --- duration_ms: 60.14 severity: fail stack: |- timeout ... not ok 900 parallel/test-debug-brk --- duration_ms: 60.109 severity: fail stack: |- timeout ... 

Copy link
Member

@TrottTrott left a comment

Choose a reason for hiding this comment

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

LGTM

They tend to hang if they happen to run in parallel with another test that uses common.PORT. PR-URL: nodejs#13592 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
gibfahn added a commit to gibfahn/node that referenced this pull request Jun 17, 2017
They tend to hang if they happen to run in parallel with another test that uses common.PORT. PR-URL: nodejs#13592 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@gibfahn
Copy link
MemberAuthor

Landed in 7a22964

@gibfahngibfahn closed this Jun 17, 2017
@gibfahngibfahn deleted the seq-debug-sig branch June 17, 2017 21:46
gibfahn added a commit that referenced this pull request Jun 20, 2017
They tend to hang if they happen to run in parallel with another test that uses common.PORT. PR-URL: #13592 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
They tend to hang if they happen to run in parallel with another test that uses common.PORT. PR-URL: #13592 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 18, 2017
BethGriggs added a commit to BethGriggs/node that referenced this pull request Mar 1, 2018
Remove 'flaky' in parallel.status for test-debug-signal-cluster as the test was moved to sequential. Refs: nodejs#13592
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
Remove 'flaky' in parallel.status for test-debug-signal-cluster as the test was moved to sequential. Refs: #13592 PR-URL: #19069 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 28, 2018
Remove 'flaky' in parallel.status for test-debug-signal-cluster as the test was moved to sequential. Refs: #13592 PR-URL: #19069 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2018
Remove 'flaky' in parallel.status for test-debug-signal-cluster as the test was moved to sequential. Refs: #13592 PR-URL: #19069 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@gibfahn@Trott@lpinca@nodejs-github-bot