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

test-debug-port-cluster or test-debug-signal-cluster hanging in background on freebsd #6989

Closed
rvagg opened this issue May 26, 2016 · 6 comments
Closed

test-debug-port-cluster or test-debug-signal-cluster hanging in background on freebsd #6989

rvagg opened this issue May 26, 2016 · 6 comments
Labels
cluster Issues and PRs related to the cluster subsystem. freebsd Issues and PRs related to the FreeBSD platform. test Issues and PRs related to the tests.

Comments

@rvagg
Copy link
Member

rvagg commented May 26, 2016

found ~550 (not a typo) instances of test/fixtures/clustered-server/app.js running on one of the freebsd CI machines, this is used by both test-debug-port-cluster and test-debug-signal-cluster and I'm not sure which one was parent. Typical entries look like:

iojs          11971   0.0  0.2  620776  3556  -  I    16May16      0:00.09 /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/node --debug=13683 /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/fixtures/clustered-server/app.js

@nodejs/testing
@Trott
Copy link
Member

Trott commented May 26, 2016

Not sure I have a fully coherentl explanation as to how this could plausibly cause the issue but the +1000 in this line seems like a code smell to me. Ought to just be common.PORT. Will submit a PR momentarily...

Trott added a commit to Trott/io.js that referenced this issue May 26, 2016
@Trott
test: remove modifcation to common.PORT
46ae822 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

Refs: nodejs#6989
@Trott Trott mentioned this issue May 26, 2016
Closed
2 tasks
@mscdex mscdex added cluster Issues and PRs related to the cluster subsystem. debugger test Issues and PRs related to the tests. freebsd Issues and PRs related to the FreeBSD platform. labels May 26, 2016
@cjihrig
Copy link
Contributor

cjihrig commented May 26, 2016

Could we build something into the CI or test runner to 1) detect tests that leave orphans running. 2) log that information somewhere. 3) kill the orphans.

@Trott
Copy link
Member

Trott commented May 26, 2016

@cjihrig I imagine the main challenge would be distinguishing between orphans and other tests that are running in parallel.

@cjihrig
Copy link
Contributor

cjihrig commented May 26, 2016

From a little Googling, it seems like identifying orphans is possible. I just don't know that we could associate that back to individual tests.

Trott added a commit to Trott/io.js that referenced this issue May 29, 2016
@Trott
test: remove modifcation to common.PORT
bddfe63 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: nodejs#6990
Refs: nodejs#6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue May 30, 2016
@Trott @Fishrock123
test: remove modifcation to common.PORT
4e826f4 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: nodejs#6990
Refs: nodejs#6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
rvagg pushed a commit that referenced this issue Jun 2, 2016
@Trott @rvagg
test: remove modifcation to common.PORT
0a258e5 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@cjihrig
Copy link
Contributor

cjihrig commented Jun 2, 2016

@Trott can this be closed?

@Trott
Copy link
Member

Trott commented Jun 2, 2016

Yes. Closing.

@Trott Trott closed this as completed Jun 2, 2016
MylesBorins pushed a commit that referenced this issue Jul 11, 2016
@Trott
test: remove modifcation to common.PORT
182a609 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott @MylesBorins
test: remove modifcation to common.PORT
ed586ec 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@Trott
test: remove modifcation to common.PORT
cd99a3e 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
test: remove modifcation to common.PORT
58a3852 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@Trott
test: remove modifcation to common.PORT
cde3014 
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.

PR-URL: #6990
Refs: #6989
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cluster Issues and PRs related to the cluster subsystem. freebsd Issues and PRs related to the FreeBSD platform. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mscdex @rvagg @Trott @cjihrig