Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Dec 20, 2017

Backport of #17466

common.hasMultiLocalhost() uses common.PORT under the hood. This is
problematic in parallel tests because another test using port 0 to
get an arbitrary open port may end up getting common.PORT before the
test using common.PORT gets it.

Therefore, change common.PORT to port 0 in common.hasMultiLocalhost().

PR-URL: #17466
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: Colin Ihrig [email protected]
Reviewed-By: Jon Moss [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Michael Dawson [email protected]
Reviewed-By: Ruben Bridgewater [email protected]

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

test

common.hasMultiLocalhost() uses common.PORT under the hood. This is
problematic in parallel tests because another test using port 0 to
get an arbitrary open port may end up getting common.PORT before the
test using common.PORT gets it.

Therefore, change common.PORT to port 0 in common.hasMultiLocalhost().

PR-URL: nodejs#17466
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v6.x labels Dec 20, 2017
@Trott
Copy link
Member Author

Trott commented Dec 20, 2017

@gibfahn gibfahn assigned MylesBorins and unassigned gibfahn Dec 20, 2017
MylesBorins pushed a commit that referenced this pull request Dec 20, 2017
common.hasMultiLocalhost() uses common.PORT under the hood. This is
problematic in parallel tests because another test using port 0 to
get an arbitrary open port may end up getting common.PORT before the
test using common.PORT gets it.

Therefore, change common.PORT to port 0 in common.hasMultiLocalhost().

Backport-PR-URL: #17771
PR-URL: #17466
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins
Copy link
Contributor

landed in a528d57

@Trott Trott deleted the backport-17466-6.x branch January 13, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants