Skip to content

Commit

Permalink
tools,test: throw if common.PORT used in parallel tests
Browse files Browse the repository at this point in the history
common.PORT should not be used in parallelized tests. (There can be a
port collision if another tests requests an arbitrary open port from the
operating system and ends up getting common.PORT before a test that uses
common.PORT uses the port.) In such a situation, throw an error.

PR-URL: nodejs#17559
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
  • Loading branch information
Trott committed Dec 12, 2017
1 parent 3674bee commit 800ce94
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
11 changes: 10 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,16 @@ const noop = () => {};
// gets tools to ignore it by default or by simple rules, especially eslint.
let tmpDirName = '.tmp';

exports.PORT = +process.env.NODE_COMMON_PORT || 12346;
Object.defineProperty(exports, 'PORT', {
get: () => {
if (+process.env.TEST_PARALLEL) {
throw new Error('common.PORT cannot be used in a parallelized test');
}
return +process.env.NODE_COMMON_PORT || 12346;
},
enumerable: true
});


exports.isWindows = process.platform === 'win32';
exports.isWOW64 = exports.isWindows &&
Expand Down
3 changes: 2 additions & 1 deletion tools/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,8 @@ def Run(self):

try:
result = self.RunCommand(self.GetCommand(), {
"TEST_THREAD_ID": "%d" % self.thread_id
"TEST_THREAD_ID": "%d" % self.thread_id,
"TEST_PARALLEL" : "%d" % self.parallel
})
finally:
# Tests can leave the tty in non-blocking mode. If the test runner
Expand Down

0 comments on commit 800ce94

Please sign in to comment.