Skip to content

Commit 0178001

Browse files
TrottMyles Borins
authored and
Myles Borins
committed
test: make test-cluster-disconnect-leak reliable
Previously, test-cluster-disconnect-leak had two issues: * Magic numbers: How many times to spawn a worker was determined through empirical experimentation. This means that as new platforms and new CPU/RAM configurations are tested, the magic numbers require more and more refinement. This brings us to... * Non-determinism: The test *seems* to fail all the time when the bug it tests for is present, but it's really a judgment based on sampling. "Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try 16..." This revised version of the test takes a different approach. The fix for the bug that the test was written for means that the `disconnect` event will fire reliably for a single worker. So we check for that and the test still fails when the fix is not in the code base and succeeds when it is. Advantages of this approach include: * The test runs much faster. * The test now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: #4674 PR-URL: #4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>
1 parent cb5986d commit 0178001

File tree

1 file changed

+14
-32
lines changed

1 file changed

+14
-32
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,29 @@
11
'use strict';
2-
// Flags: --expose-internals
2+
3+
// Test fails in Node v5.4.0 and passes in v5.4.1 and newer.
34

45
const common = require('../common');
5-
const assert = require('assert');
66
const net = require('net');
77
const cluster = require('cluster');
8-
const handles = require('internal/cluster').handles;
9-
const os = require('os');
108

11-
if (common.isWindows) {
12-
console.log('1..0 # Skipped: This test does not apply to Windows.');
13-
return;
14-
}
9+
const noop = () => {};
1510

1611
cluster.schedulingPolicy = cluster.SCHED_NONE;
1712

1813
if (cluster.isMaster) {
19-
const cpus = os.cpus().length;
20-
const tries = cpus > 8 ? 128 : cpus * 16;
21-
22-
const worker1 = cluster.fork();
23-
worker1.on('message', common.mustCall(() => {
24-
worker1.disconnect();
25-
for (let i = 0; i < tries; ++ i) {
26-
const w = cluster.fork();
27-
w.on('online', common.mustCall(w.disconnect));
28-
}
29-
}));
30-
31-
cluster.on('exit', common.mustCall((worker, code) => {
32-
assert.strictEqual(code, 0, 'worker exited with error');
33-
}, tries + 1));
34-
35-
process.on('exit', () => {
36-
assert.deepEqual(Object.keys(cluster.workers), []);
37-
assert.strictEqual(Object.keys(handles).length, 0);
38-
});
14+
const worker = cluster.fork();
15+
16+
// This is the important part of the test: Confirm that `disconnect` fires.
17+
worker.on('disconnect', common.mustCall(noop));
18+
19+
// These are just some extra stuff we're checking for good measure...
20+
worker.on('exit', common.mustCall(noop));
21+
cluster.on('exit', common.mustCall(noop));
3922

23+
cluster.disconnect();
4024
return;
4125
}
4226

43-
var server = net.createServer();
27+
const server = net.createServer();
4428

45-
server.listen(common.PORT, function() {
46-
process.send('listening');
47-
});
29+
server.listen(common.PORT);

0 commit comments

Comments
 (0)