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

Flaky test-cluster-http-pipe #16323

Closed
joyeecheung opened this issue Oct 19, 2017 · 4 comments
Closed

Flaky test-cluster-http-pipe #16323

joyeecheung opened this issue Oct 19, 2017 · 4 comments
Labels
cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Oct 19, 2017

  • Version: master
  • Platform: ubuntu
  • Subsystem: test

Sample

not ok 246 parallel/test-cluster-http-pipe
  ---
  duration_ms: 0.516
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 0.
        at Object.exports.mustCall (/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1404-64/test/common/index.js:488:10)
        at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1404-64/test/parallel/test-cluster-http-pipe.js:37:31)
        at Module._compile (module.js:607:30)
        at Object.Module._extensions..js (module.js:618:10)
        at Module.load (module.js:526:32)
        at tryModuleLoad (module.js:489:12)
        at Function.Module._load (module.js:481:3)
        at Function.Module.runMain (module.js:648:10)
        at startup (bootstrap_node.js:191:16)
    events.js:195
          throw er; // Unhandled 'error' event
          ^
    
    Error: bind EADDRINUSE ../../../../../node-tmp/tmp.0node-test.30586.sock
        at Object._errnoException (util.js:1023:13)
        at _exceptionWithHostPort (util.js:1044:20)
        at listenOnMasterHandle (net.js:1417:16)
        at rr (internal/cluster/child.js:111:12)
        at Worker.send (internal/cluster/child.js:78:7)
        at process.onInternalMessage (internal/cluster/utils.js:42:8)
        at emitTwo (events.js:141:20)
        at process.emit (events.js:227:7)
        at emit (internal/child_process.js:790:12)
        at _combinedTickCallback (internal/process/next_tick.js:141:11)
  ...

I think we should migrate all those listen(common.PIPE) usage to listening on random pipes as what we did for common.PORT...thoguhts? @nodejs/testing

@apapirovski
Copy link
Member

Seen it fail a number of times lately (and in general, tests with common.PIPE seem to show up here and there). +1 from me.

@gibfahn
Copy link
Member

gibfahn commented Oct 19, 2017

I think we should migrate all those listen(common.PIPE) usage to listening on random pipes as what we did for common.PORT...thoughts? @nodejs/testing

SGTM, I guess we just could change common.PIPE to include the test name in the pipe name.

node/test/common/index.js

Lines 273 to 278 in bf1bace

{
const localRelative = path.relative(process.cwd(), `${exports.tmpDir}/`);
const pipePrefix = exports.isWindows ? '\\\\.\\pipe\\' : localRelative;
const pipeName = `node-test.${process.pid}.sock`;
exports.PIPE = pipePrefix + pipeName;
}

@mscdex mscdex added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Oct 20, 2017
@Trott
Copy link
Member

Trott commented Oct 21, 2017

Random pipe names are not needed. The problem is that the pipes are being created in test rathe than in the appropriate temp directory inside test due to a bug in common.PIPE instroduced in c34ae48. Fix coming soon.

Trott added a commit to Trott/io.js that referenced this issue Oct 21, 2017
`common.PIPE` is returning a path name in `test` rather than in the
`tmp` directory for each test. This is causing multiple test failures in
CI. Make the path name inside the temporary directories again. This way
the pipe is removed by `common.refreshTmpDir()` on POSIX.

The bug in `common.PIPE` was introduced in
c34ae48.

Fixes: nodejs#16290
Fixes: nodejs#16323
@Trott
Copy link
Member

Trott commented Oct 21, 2017

Should be fixed by #16364 I think,

@Trott Trott closed this as completed in 8e268c7 Oct 21, 2017
MylesBorins pushed a commit that referenced this issue Oct 23, 2017
`common.PIPE` is returning a path name in `test` rather than in the
`tmp` directory for each test. This is causing multiple test failures in
CI. Make the path name inside the temporary directories again. This way
the pipe is removed by `common.refreshTmpDir()` on POSIX.

The bug in `common.PIPE` was introduced in
c34ae48.

PR-URL: #16364
Fixes: #16290
Fixes: #16323
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this issue Oct 26, 2017
`common.PIPE` is returning a path name in `test` rather than in the
`tmp` directory for each test. This is causing multiple test failures in
CI. Make the path name inside the temporary directories again. This way
the pipe is removed by `common.refreshTmpDir()` on POSIX.

The bug in `common.PIPE` was introduced in
c34ae48.

PR-URL: nodejs/node#16364
Fixes: nodejs/node#16290
Fixes: nodejs/node#16323
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this issue Dec 7, 2017
`common.PIPE` is returning a path name in `test` rather than in the
`tmp` directory for each test. This is causing multiple test failures in
CI. Make the path name inside the temporary directories again. This way
the pipe is removed by `common.refreshTmpDir()` on POSIX.

The bug in `common.PIPE` was introduced in
c34ae48.

PR-URL: nodejs/node#16364
Fixes: nodejs/node#16290
Fixes: nodejs/node#16323
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants