Skip to content

Commit 19907c2

Browse files
cjihrigMylesBorins
cjihrig
authored and
MylesBorins
committed
test: use mustCall() for simple flow tracking
Many of the tests use variables to track when callback functions are invoked or events are emitted. These variables are then asserted on process exit. This commit replaces this pattern in straightforward cases with common.mustCall(). This makes the tests easier to reason about, leads to a net reduction in lines of code, and uncovered a few bugs in tests. This commit also replaces some callbacks that should never be called with common.fail(). PR-URL: #7753 Reviewed-By: Wyatt Preul <wpreul@gmail.com> Reviewed-By: Minwoo Jung <jmwsoft@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 79d4986 commit 19907c2

212 files changed

Lines changed: 1095 additions & 2859 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎test/addons/async-hello-world/test.js‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,10 @@
11
'use strict';
2-
var common = require('../../common');
2+
const common = require('../../common');
33
var assert = require('assert');
44
const binding = require(`./build/${common.buildType}/binding`);
5-
var called = false;
65

7-
process.on('exit', function() {
8-
assert(called);
9-
});
10-
11-
binding(5, function(err, val) {
6+
binding(5, common.mustCall(function(err, val) {
127
assert.equal(null, err);
138
assert.equal(10, val);
14-
process.nextTick(function() {
15-
called = true;
16-
});
17-
});
9+
process.nextTick(common.mustCall(function() {}));
10+
}));

‎test/internet/test-http-dns-fail.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
* should trigger the error event after each attempt.
55
*/
66

7-
require('../common');
7+
const common = require('../common');
88
var assert = require('assert');
99
var http = require('http');
1010

11-
var resDespiteError = false;
1211
var hadError = 0;
1312

1413
function httpreq(count) {
@@ -19,9 +18,7 @@ function httpreq(count) {
1918
port: 80,
2019
path: '/',
2120
method: 'GET'
22-
}, function(res) {
23-
resDespiteError = true;
24-
});
21+
}, common.fail);
2522

2623
req.on('error', function(e) {
2724
console.log(e.message);
@@ -37,6 +34,5 @@ httpreq(0);
3734

3835

3936
process.on('exit', function() {
40-
assert.equal(false, resDespiteError);
4137
assert.equal(2, hadError);
4238
});

‎test/internet/test-http-https-default-ports.js‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict';
22
var common = require('../common');
3-
var assert = require('assert');
43

54
if (!common.hasCrypto) {
65
common.skip('missing crypto');
@@ -9,21 +8,11 @@ if (!common.hasCrypto) {
98
var https = require('https');
109

1110
var http = require('http');
12-
var gotHttpsResp = false;
13-
var gotHttpResp = false;
1411

15-
process.on('exit', function() {
16-
assert(gotHttpsResp);
17-
assert(gotHttpResp);
18-
console.log('ok');
19-
});
20-
21-
https.get('https://www.google.com/', function(res) {
22-
gotHttpsResp = true;
12+
https.get('https://www.google.com/', common.mustCall(function(res) {
2313
res.resume();
24-
});
14+
}));
2515

26-
http.get('http://www.google.com/', function(res) {
27-
gotHttpResp = true;
16+
http.get('http://www.google.com/', common.mustCall(function(res) {
2817
res.resume();
29-
});
18+
}));

‎test/internet/test-net-connect-timeout.js‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,12 @@
33
// https://groups.google.com/forum/#!topic/nodejs/UE0ZbfLt6t8
44
// https://groups.google.com/forum/#!topic/nodejs-dev/jR7-5UDqXkw
55

6-
require('../common');
6+
const common = require('../common');
77
var net = require('net');
88
var assert = require('assert');
99

1010
var start = new Date();
1111

12-
var gotTimeout = false;
13-
14-
var gotConnect = false;
15-
1612
var T = 100;
1713

1814
// 192.0.2.1 is part of subnet assigned as "TEST-NET" in RFC 5737.
@@ -23,22 +19,11 @@ var socket = net.createConnection(9999, '192.0.2.1');
2319

2420
socket.setTimeout(T);
2521

26-
socket.on('timeout', function() {
22+
socket.on('timeout', common.mustCall(function() {
2723
console.error('timeout');
28-
gotTimeout = true;
2924
var now = new Date();
3025
assert.ok(now - start < T + 500);
3126
socket.destroy();
32-
});
33-
34-
socket.on('connect', function() {
35-
console.error('connect');
36-
gotConnect = true;
37-
socket.destroy();
38-
});
39-
27+
}));
4028

41-
process.on('exit', function() {
42-
assert.ok(gotTimeout);
43-
assert.ok(!gotConnect);
44-
});
29+
socket.on('connect', common.fail);

‎test/internet/test-net-connect-unref.js‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,14 @@
11
'use strict';
2-
require('../common');
3-
var assert = require('assert');
2+
const common = require('../common');
43
var net = require('net');
54

6-
var client, killed = false, ended = false;
5+
var client;
76
var TIMEOUT = 10 * 1000;
87

98
client = net.createConnection(53, '8.8.8.8', function() {
109
client.unref();
1110
});
1211

13-
client.on('close', function() {
14-
ended = true;
15-
});
16-
17-
setTimeout(function() {
18-
killed = true;
19-
client.end();
20-
}, TIMEOUT).unref();
12+
client.on('close', common.fail);
2113

22-
process.on('exit', function() {
23-
assert.strictEqual(killed, false, 'A client should have connected');
24-
assert.strictEqual(ended, false, 'A client should stay connected');
25-
});
14+
setTimeout(common.fail, TIMEOUT).unref();

‎test/parallel/test-child-process-buffering.js‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@
22
var common = require('../common');
33
var assert = require('assert');
44

5-
var pwd_called = false;
6-
var childClosed = false;
7-
var childExited = false;
8-
95
function pwd(callback) {
106
var output = '';
117
var child = common.spawnPwd();
@@ -16,17 +12,14 @@ function pwd(callback) {
1612
output += s;
1713
});
1814

19-
child.on('exit', function(c) {
15+
child.on('exit', common.mustCall(function(c) {
2016
console.log('exit: ' + c);
2117
assert.equal(0, c);
22-
childExited = true;
23-
});
18+
}));
2419

25-
child.on('close', function() {
20+
child.on('close', common.mustCall(function() {
2621
callback(output);
27-
pwd_called = true;
28-
childClosed = true;
29-
});
22+
}));
3023
}
3124

3225

@@ -35,9 +28,3 @@ pwd(function(result) {
3528
assert.equal(true, result.length > 1);
3629
assert.equal('\n', result[result.length - 1]);
3730
});
38-
39-
process.on('exit', function() {
40-
assert.equal(true, pwd_called);
41-
assert.equal(true, childExited);
42-
assert.equal(true, childClosed);
43-
});

‎test/parallel/test-child-process-disconnect.js‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,16 @@ if (process.argv[2] === 'child') {
4848
var child = fork(process.argv[1], ['child']);
4949

5050
var childFlag = false;
51-
var childSelfTerminate = false;
52-
var parentEmit = false;
5351
var parentFlag = false;
5452

5553
// when calling .disconnect the event should emit
5654
// and the disconnected flag should be true.
57-
child.on('disconnect', function() {
58-
parentEmit = true;
55+
child.on('disconnect', common.mustCall(function() {
5956
parentFlag = child.connected;
60-
});
57+
}));
6158

6259
// the process should also self terminate without using signals
63-
child.on('exit', function() {
64-
childSelfTerminate = true;
65-
});
60+
child.on('exit', common.mustCall(function() {}));
6661

6762
// when child is listening
6863
child.on('message', function(obj) {
@@ -91,8 +86,5 @@ if (process.argv[2] === 'child') {
9186
process.on('exit', function() {
9287
assert.equal(childFlag, false);
9388
assert.equal(parentFlag, false);
94-
95-
assert.ok(childSelfTerminate);
96-
assert.ok(parentEmit);
9789
});
9890
}

‎test/parallel/test-child-process-exec-buffer.js‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,22 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
var assert = require('assert');
44
var exec = require('child_process').exec;
55
var os = require('os');
6-
7-
var success_count = 0;
8-
96
var str = 'hello';
107

118
// default encoding
12-
exec('echo ' + str, function(err, stdout, stderr) {
9+
exec('echo ' + str, common.mustCall(function(err, stdout, stderr) {
1310
assert.ok('string', typeof stdout, 'Expected stdout to be a string');
1411
assert.ok('string', typeof stderr, 'Expected stderr to be a string');
1512
assert.equal(str + os.EOL, stdout);
16-
17-
success_count++;
18-
});
13+
}));
1914

2015
// no encoding (Buffers expected)
2116
exec('echo ' + str, {
2217
encoding: null
23-
}, function(err, stdout, stderr) {
18+
}, common.mustCall(function(err, stdout, stderr) {
2419
assert.ok(stdout instanceof Buffer, 'Expected stdout to be a Buffer');
2520
assert.ok(stderr instanceof Buffer, 'Expected stderr to be a Buffer');
2621
assert.equal(str + os.EOL, stdout.toString());
27-
28-
success_count++;
29-
});
30-
31-
process.on('exit', function() {
32-
assert.equal(2, success_count);
33-
});
22+
}));

‎test/parallel/test-child-process-exec-cwd.js‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@ const common = require('../common');
33
var assert = require('assert');
44
var exec = require('child_process').exec;
55

6-
var success_count = 0;
7-
var error_count = 0;
8-
96
var pwdcommand, dir;
107

118
if (common.isWindows) {
@@ -16,21 +13,7 @@ if (common.isWindows) {
1613
dir = '/dev';
1714
}
1815

19-
exec(pwdcommand, {cwd: dir}, function(err, stdout, stderr) {
20-
if (err) {
21-
error_count++;
22-
console.log('error!: ' + err.code);
23-
console.log('stdout: ' + JSON.stringify(stdout));
24-
console.log('stderr: ' + JSON.stringify(stderr));
25-
assert.equal(false, err.killed);
26-
} else {
27-
success_count++;
28-
console.log(stdout);
29-
assert.ok(stdout.indexOf(dir) == 0);
30-
}
31-
});
32-
33-
process.on('exit', function() {
34-
assert.equal(1, success_count);
35-
assert.equal(0, error_count);
36-
});
16+
exec(pwdcommand, {cwd: dir}, common.mustCall(function(err, stdout, stderr) {
17+
assert.ifError(err);
18+
assert.ok(stdout.indexOf(dir) == 0);
19+
}));

‎test/parallel/test-child-process-exec-error.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,10 @@ var assert = require('assert');
44
var child_process = require('child_process');
55

66
function test(fun, code) {
7-
var errors = 0;
8-
9-
fun('does-not-exist', function(err) {
7+
fun('does-not-exist', common.mustCall(function(err) {
108
assert.equal(err.code, code);
119
assert(/does\-not\-exist/.test(err.cmd));
12-
errors++;
13-
});
14-
15-
process.on('exit', function() {
16-
assert.equal(errors, 1);
17-
});
10+
}));
1811
}
1912

2013
if (common.isWindows) {

0 commit comments

Comments
 (0)