Skip to content

Commit

Permalink
test: refactor/cleanup a number of cluster tests
Browse files Browse the repository at this point in the history 
* Move shared code into common
* Favor use of strictEqual
* Add some missing common.mustCalls
* Other general cleanup

PR-URL: #8261
Reviewed-By: Santiago Gimeno <[email protected]>
  • Loading branch information
@jasnell
jasnell committed Sep 1, 2016
1 parent 2168432 commit baa0ffd
Show file tree
Hide file tree
Showing 14 changed files with 278 additions and 360 deletions.
9 changes: 9 additions & 0 deletions test/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,3 +498,12 @@ exports.busyLoop = function busyLoop(time) {
var stopTime = startTime + time;
while (Timer.now() < stopTime) {}
};

exports.isAlive = function isAlive(pid) {
try {
process.kill(pid, 'SIGCONT');
return true;
} catch (e) {
return false;
}
};
91 changes: 45 additions & 46 deletions test/parallel/test-cluster-basic.js
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var cluster = require('cluster');
const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');

assert.equal('NODE_UNIQUE_ID' in process.env, false,
'NODE_UNIQUE_ID should be removed on startup');
assert.strictEqual('NODE_UNIQUE_ID' in process.env, false,
'NODE_UNIQUE_ID should be removed on startup');

function forEach(obj, fn) {
Object.keys(obj).forEach(function(name, index) {
Object.keys(obj).forEach((name, index) => {
fn(obj[name], name, index);
});
}


if (cluster.isWorker) {
var http = require('http');
const http = require('http');
http.Server(function() {

}).listen(common.PORT, '127.0.0.1');
} else if (cluster.isMaster) {

var checks = {
const checks = {
cluster: {
events: {
fork: false,
Expand Down Expand Up @@ -57,13 +57,13 @@ if (cluster.isWorker) {
};

var worker;
var stateNames = Object.keys(checks.worker.states);
const stateNames = Object.keys(checks.worker.states);

//Check events, states, and emit arguments
forEach(checks.cluster.events, function(bool, name, index) {
forEach(checks.cluster.events, (bool, name, index) => {

//Listen on event
cluster.on(name, function(/* worker */) {
cluster.on(name, common.mustCall(function(/* worker */) {

//Set event
checks.cluster.events[name] = true;
Expand All @@ -74,28 +74,26 @@ if (cluster.isWorker) {
//Check state
var state = stateNames[index];
checks.worker.states[state] = (state === worker.state);
});
}));
});

//Kill worker when listening
cluster.on('listening', function() {
cluster.on('listening', common.mustCall(() => {
worker.kill();
});
}));

//Kill process when worker is killed
cluster.on('exit', function() {
process.exit(0);
});
cluster.on('exit', common.mustCall(() => {}));

//Create worker
worker = cluster.fork();
assert.equal(worker.id, 1);
assert.ok(worker instanceof cluster.Worker,
'the worker is not a instance of the Worker constructor');
assert.strictEqual(worker.id, 1);
assert(worker instanceof cluster.Worker,
'the worker is not a instance of the Worker constructor');

//Check event
forEach(checks.worker.events, function(bool, name, index) {
worker.on(name, function() {
worker.on(name, common.mustCall(function() {
//Set event
checks.worker.events[name] = true;

Expand All @@ -104,56 +102,57 @@ if (cluster.isWorker) {

switch (name) {
case 'exit':
assert.equal(arguments[0], worker.process.exitCode);
assert.equal(arguments[1], worker.process.signalCode);
assert.equal(arguments.length, 2);
assert.strictEqual(arguments[0], worker.process.exitCode);
assert.strictEqual(arguments[1], worker.process.signalCode);
assert.strictEqual(arguments.length, 2);
break;

case 'listening':
assert.equal(arguments.length, 1);
var expect = { address: '127.0.0.1',
port: common.PORT,
addressType: 4,
fd: undefined };
assert.strictEqual(arguments.length, 1);
const expect = { address: '127.0.0.1',
port: common.PORT,
addressType: 4,
fd: undefined };
assert.deepStrictEqual(arguments[0], expect);
break;

default:
assert.equal(arguments.length, 0);
assert.strictEqual(arguments.length, 0);
break;
}
});
}));
});

//Check all values
process.once('exit', function() {
process.once('exit', () => {
//Check cluster events
forEach(checks.cluster.events, function(check, name) {
assert.ok(check, 'The cluster event "' + name + '" on the cluster ' +
'object did not fire');
forEach(checks.cluster.events, (check, name) => {
assert(check,
`The cluster event "${name}" on the cluster object did not fire`);
});

//Check cluster event arguments
forEach(checks.cluster.equal, function(check, name) {
assert.ok(check, 'The cluster event "' + name + '" did not emit ' +
'with correct argument');
forEach(checks.cluster.equal, (check, name) => {
assert(check,
`The cluster event "${name}" did not emit with correct argument`);
});

//Check worker states
forEach(checks.worker.states, function(check, name) {
assert.ok(check, 'The worker state "' + name + '" was not set to true');
forEach(checks.worker.states, (check, name) => {
assert(check,
`The worker state "${name}" was not set to true`);
});

//Check worker events
forEach(checks.worker.events, function(check, name) {
assert.ok(check, 'The worker event "' + name + '" on the worker object ' +
'did not fire');
forEach(checks.worker.events, (check, name) => {
assert(check,
`The worker event "${name}" on the worker object did not fire`);
});

//Check worker event arguments
forEach(checks.worker.equal, function(check, name) {
assert.ok(check, 'The worker event "' + name + '" did not emit with ' +
'corrent argument');
forEach(checks.worker.equal, (check, name) => {
assert(check,
`The worker event "${name}" did not emit with correct argument`);
});
});

Expand Down
16 changes: 8 additions & 8 deletions test/parallel/test-cluster-bind-privileged-port.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var cluster = require('cluster');
var net = require('net');
const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');
const net = require('net');

if (common.isWindows) {
common.skip('not reliable on Windows.');
Expand All @@ -15,14 +15,14 @@ if (process.getuid() === 0) {
}

if (cluster.isMaster) {
cluster.fork().on('exit', common.mustCall(function(exitCode) {
assert.equal(exitCode, 0);
cluster.fork().on('exit', common.mustCall((exitCode) => {
assert.strictEqual(exitCode, 0);
}));
} else {
var s = net.createServer(common.fail);
s.listen(42, common.fail.bind(null, 'listen should have failed'));
s.on('error', common.mustCall(function(err) {
assert.equal(err.code, 'EACCES');
s.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'EACCES');
process.disconnect();
}));
}
74 changes: 30 additions & 44 deletions test/parallel/test-cluster-bind-twice.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,88 +18,74 @@
//
// See https://github.com/joyent/node/issues/2721 for more details.

var common = require('../common');
var assert = require('assert');
var cluster = require('cluster');
var fork = require('child_process').fork;
var http = require('http');
const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');
const fork = require('child_process').fork;
const http = require('http');

var id = process.argv[2];
const id = process.argv[2];

if (!id) {
var a = fork(__filename, ['one']);
var b = fork(__filename, ['two']);
const a = fork(__filename, ['one']);
const b = fork(__filename, ['two']);

a.on('exit', function(c) {
a.on('exit', common.mustCall((c) => {
if (c) {
b.send('QUIT');
throw new Error('A exited with ' + c);
}
});
}));

b.on('exit', function(c) {
b.on('exit', common.mustCall((c) => {
if (c) {
a.send('QUIT');
throw new Error('B exited with ' + c);
}
});
}));


a.on('message', function(m) {
a.on('message', common.mustCall((m) => {
if (typeof m === 'object') return;
assert.equal(m, 'READY');
assert.strictEqual(m, 'READY');
b.send('START');
});
}));

let ok = false;

b.on('message', function(m) {
if (typeof m === 'object') return; // ignore system messages
assert.equal(m, 'EADDRINUSE');
ok = true;
b.on('message', common.mustCall((m) => {
assert.strictEqual(m, 'EADDRINUSE');
a.send('QUIT');
b.send('QUIT');
});
}));

process.on('exit', function() {
assert(ok);
});
} else if (id === 'one') {
if (cluster.isMaster) return startWorker();

http.createServer(common.fail).listen(common.PORT, function() {
http.createServer(common.fail).listen(common.PORT, common.mustCall(() => {
process.send('READY');
});
}));

process.on('message', function(m) {
process.on('message', common.mustCall((m) => {
if (m === 'QUIT') process.exit();
});
}));
} else if (id === 'two') {
if (cluster.isMaster) return startWorker();

let ok = false;
process.on('exit', function() {
assert(ok);
});

var server = http.createServer(common.fail);
process.on('message', function(m) {
if (typeof m === 'object') return; // ignore system messages
const server = http.createServer(common.fail);
process.on('message', common.mustCall((m) => {
if (m === 'QUIT') process.exit();
assert.equal(m, 'START');
assert.strictEqual(m, 'START');
server.listen(common.PORT, common.fail);
server.on('error', function(e) {
assert.equal(e.code, 'EADDRINUSE');
server.on('error', common.mustCall((e) => {
assert.strictEqual(e.code, 'EADDRINUSE');
process.send(e.code);
ok = true;
});
});
}));
}, 2));
} else {
assert(0); // bad command line argument
}

function startWorker() {
var worker = cluster.fork();
const worker = cluster.fork();
worker.on('exit', process.exit);
worker.on('message', process.send.bind(process));
process.on('message', worker.send.bind(worker));
Expand Down
Loading

1 comment on commit baa0ffd

@mhdawson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the AIX runs I've seen a few failures of: parallel/test-cluster-dgram-1

I wonder if its related to this change ?

Please sign in to comment.