Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Oct 2, 2017

test/async-hooks/test/test-tslwrap.js uses common.PORT but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove common.PORT from the test.

Refs: #14404 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@Trott Trott added the test Issues and PRs related to the tests. label Oct 2, 2017
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Oct 2, 2017
@mscdex
Copy link
Contributor

mscdex commented Oct 2, 2017

Minor nit: there's a typo in the filename in the commit message

`test/async-hooks/test/test-tlswrap.js` uses `common.PORT` but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove `common.PORT` from the test.

Refs: nodejs#14404 (comment)
@Trott
Copy link
Member Author

Trott commented Oct 2, 2017

Commit message typo fixed. Thanks.

@mscdex
Copy link
Contributor

mscdex commented Oct 2, 2017

Trott added a commit to Trott/io.js that referenced this pull request Oct 5, 2017
`test/async-hooks/test/test-tlswrap.js` uses `common.PORT` but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove `common.PORT` from the test.

PR-URL: nodejs#15742
Ref: nodejs#14404 (comment)
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member Author

Trott commented Oct 5, 2017

Landed in ddbf07a

@Trott Trott closed this Oct 5, 2017
MylesBorins pushed a commit that referenced this pull request Oct 7, 2017
`test/async-hooks/test/test-tlswrap.js` uses `common.PORT` but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove `common.PORT` from the test.

PR-URL: #15742
Ref: #14404 (comment)
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 7, 2017
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
`test/async-hooks/test/test-tlswrap.js` uses `common.PORT` but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove `common.PORT` from the test.

PR-URL: #15742
Ref: #14404 (comment)
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
`test/async-hooks/test/test-tlswrap.js` uses `common.PORT` but
async-hooks tests are run in parallel. Another test using port 0 could
result in a port collision. Remove `common.PORT` from the test.

PR-URL: nodejs/node#15742
Ref: nodejs/node#14404 (comment)
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott deleted the rm-common-port branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants