From 724e7e1acf34929213f47b518b3d7d3ca808be7b Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Tue, 11 Jul 2017 10:03:19 -0400 Subject: [PATCH] test: make common.PIPE process unique * includes a tiny bit of refactoring in adjacent lines. * fixes 1 test and 1 benchmark that depended on PIPE being constant. PR-URL: https://github.com/nodejs/node/pull/14168 Fixes: https://github.com/nodejs/node/issues/14128 Reviewed-By: James M Snell --- benchmark/http/_chunky_http_client.js | 3 +- .../http/http_server_for_chunky_client.js | 20 ++++----- test/common/index.js | 27 ++++++------ test/fixtures/listen-on-socket-and-exit.js | 39 ----------------- test/parallel/test-cluster-eaccess.js | 42 +++++++++---------- 5 files changed, 45 insertions(+), 86 deletions(-) delete mode 100644 test/fixtures/listen-on-socket-and-exit.js diff --git a/benchmark/http/_chunky_http_client.js b/benchmark/http/_chunky_http_client.js index e9ba435f11c454..d7c4e193c8c3c9 100644 --- a/benchmark/http/_chunky_http_client.js +++ b/benchmark/http/_chunky_http_client.js @@ -3,7 +3,6 @@ // test HTTP throughput in fragmented header case var common = require('../common.js'); var net = require('net'); -var test = require('../../test/common'); var bench = common.createBenchmark(main, { len: [1, 4, 8, 16, 32, 64, 128], @@ -56,7 +55,7 @@ function main(conf) { var mult = 17; var add = 11; var count = 0; - var PIPE = test.PIPE; + var PIPE = process.env.PIPE_NAME; var socket = net.connect(PIPE, function() { bench.start(); WriteHTTPHeaders(socket, 1, len); diff --git a/benchmark/http/http_server_for_chunky_client.js b/benchmark/http/http_server_for_chunky_client.js index 04d5a2c4a02530..cc632c02ec4222 100644 --- a/benchmark/http/http_server_for_chunky_client.js +++ b/benchmark/http/http_server_for_chunky_client.js @@ -1,24 +1,22 @@ 'use strict'; var assert = require('assert'); -var path = require('path'); var http = require('http'); var fs = require('fs'); -var fork = require('child_process').fork; +var { fork } = require('child_process'); var common = require('../common.js'); -var test = require('../../test/common'); -var pep = `${path.dirname(process.argv[1])}/_chunky_http_client.js`; -var PIPE = test.PIPE; +const { PIPE, tmpDir } = require('../../test/common'); +process.env.PIPE_NAME = PIPE; try { - fs.accessSync(test.tmpDir, fs.F_OK); + fs.accessSync(tmpDir, fs.F_OK); } catch (e) { - fs.mkdirSync(test.tmpDir); + fs.mkdirSync(tmpDir); } var server; try { - fs.unlinkSync(PIPE); + fs.unlinkSync(process.env.PIPE_NAME); } catch (e) { /* ignore */ } server = http.createServer(function(req, res) { @@ -33,10 +31,12 @@ server = http.createServer(function(req, res) { server.on('error', function(err) { throw new Error(`server error: ${err}`); }); - server.listen(PIPE); -var child = fork(pep, process.argv.slice(2)); +const child = fork( + `${__dirname}/_chunky_http_client.js`, + process.argv.slice(2) +); child.on('message', common.sendResult); child.on('close', function(code) { server.close(); diff --git a/test/common/index.js b/test/common/index.js index 3caed88e203158..d74123c525b11e 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -246,7 +246,7 @@ Object.defineProperty(exports, 'opensslCli', {get: function() { Object.defineProperty(exports, 'hasCrypto', { get: function() { - return process.versions.openssl ? true : false; + return Boolean(process.versions.openssl); } }); @@ -256,22 +256,21 @@ Object.defineProperty(exports, 'hasFipsCrypto', { } }); -if (exports.isWindows) { - exports.PIPE = '\\\\.\\pipe\\libuv-test'; - if (process.env.TEST_THREAD_ID) { - exports.PIPE += `.${process.env.TEST_THREAD_ID}`; - } -} else { - exports.PIPE = `${exports.tmpDir}/test.sock`; +{ + const pipePrefix = exports.isWindows ? '\\\\.\\pipe\\' : `${exports.tmpDir}/`; + const pipeName = `node-test.${process.pid}.sock`; + exports.PIPE = pipePrefix + pipeName; } -const ifaces = os.networkInterfaces(); -const re = /lo/; -exports.hasIPv6 = Object.keys(ifaces).some(function(name) { - return re.test(name) && ifaces[name].some(function(info) { - return info.family === 'IPv6'; +{ + const iFaces = os.networkInterfaces(); + const re = /lo/; + exports.hasIPv6 = Object.keys(iFaces).some(function(name) { + return re.test(name) && iFaces[name].some(function(info) { + return info.family === 'IPv6'; + }); }); -}); +} /* * Check that when running a test with diff --git a/test/fixtures/listen-on-socket-and-exit.js b/test/fixtures/listen-on-socket-and-exit.js deleted file mode 100644 index aeb6cb482b715d..00000000000000 --- a/test/fixtures/listen-on-socket-and-exit.js +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -// child process that listens on a socket, allows testing of an EADDRINUSE condition - -const common = require('../common'); -const net = require('net'); - -common.refreshTmpDir(); - -var server = net.createServer().listen(common.PIPE, function() { - console.log('child listening'); - process.send('listening'); -}); - -function onmessage() { - console.log('child exiting'); - server.close(); -} - -process.once('message', onmessage); diff --git a/test/parallel/test-cluster-eaccess.js b/test/parallel/test-cluster-eaccess.js index bc2ffddab80dbe..c7bb46b2844add 100644 --- a/test/parallel/test-cluster-eaccess.js +++ b/test/parallel/test-cluster-eaccess.js @@ -20,19 +20,22 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; +const common = require('../common'); + // Test that errors propagated from cluster workers are properly // received in their master. Creates an EADDRINUSE condition by forking // a process in child cluster and propagates the error to the master. -const common = require('../common'); const assert = require('assert'); const cluster = require('cluster'); const fork = require('child_process').fork; -const fs = require('fs'); const net = require('net'); -if (cluster.isMaster) { - const worker = cluster.fork(); +if (cluster.isMaster && process.argv.length !== 3) { + // cluster.isMaster + common.refreshTmpDir(); + const PIPE_NAME = common.PIPE; + const worker = cluster.fork({PIPE_NAME}); // makes sure master is able to fork the worker cluster.on('fork', common.mustCall()); @@ -43,27 +46,16 @@ if (cluster.isMaster) { worker.on('message', common.mustCall(function(err) { // disconnect first, so that we will not leave zombies worker.disconnect(); - - console.log(err); assert.strictEqual('EADDRINUSE', err.code); })); - - process.on('exit', function() { - console.log('master exited'); - try { - fs.unlinkSync(common.PIPE); - } catch (e) { - } - }); - -} else { - common.refreshTmpDir(); - const cp = fork(`${common.fixturesDir}/listen-on-socket-and-exit.js`, - { stdio: 'inherit' }); +} else if (process.argv.length !== 3) { + // cluster.worker + const PIPE_NAME = process.env.PIPE_NAME; + const cp = fork(__filename, [PIPE_NAME], { stdio: 'inherit' }); // message from the child indicates it's ready and listening cp.on('message', common.mustCall(function() { - const server = net.createServer().listen(common.PIPE, function() { + const server = net.createServer().listen(PIPE_NAME, function() { // message child process so that it can exit cp.send('end'); // inform master about the unexpected situation @@ -71,12 +63,20 @@ if (cluster.isMaster) { }); server.on('error', function(err) { - console.log('parent error, ending'); // message to child process tells it to exit cp.send('end'); // propagate error to parent process.send(err); }); + })); +} else if (process.argv.length === 3) { + // child process (of cluster.worker) + const PIPE_NAME = process.argv[2]; + const server = net.createServer().listen(PIPE_NAME, common.mustCall(() => { + process.send('listening'); })); + process.once('message', common.mustCall(() => server.close())); +} else { + assert.fail('Impossible state'); }