Skip to content

Commit

Permalink
test: remove common.fail()
Browse files Browse the repository at this point in the history
common.fail() was added to paste over issues with assert.fail() function
signature. assert.fail() has been updated to accept a single argument so
common.fail() is no longer necessary.

PR-URL: nodejs#12293
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Trott committed Sep 29, 2017
1 parent 7c10194 commit e5b11f8
Show file tree
Hide file tree
Showing 45 changed files with 95 additions and 89 deletions.
9 changes: 2 additions & 7 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ process.on('exit', function() {
if (!exports.globalCheck) return;
const leaked = leakedGlobals();
if (leaked.length > 0) {
fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
}
});

Expand Down Expand Up @@ -473,14 +473,9 @@ exports.fileExists = function(pathname) {
}
};

function fail(msg) {
assert.fail(null, null, msg);
}
exports.fail = fail;

exports.mustNotCall = function(msg) {
return function mustNotCall() {
fail(msg || 'function should not have been called');
assert.fail(msg || 'function should not have been called');
};
};

Expand Down
2 changes: 1 addition & 1 deletion test/inspector/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ TestSession.prototype.sendInspectorCommands = function(commands) {
};
this.sendAll_(commands, () => {
timeoutId = setTimeout(() => {
common.fail(`Messages without response: ${
assert.fail(`Messages without response: ${
Object.keys(this.messages_).join(', ')}`);
}, TIMEOUT);
});
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dgram-send-cb-quelches-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function callbackOnly(err) {
}

function onEvent(err) {
common.fail('Error should not be emitted if there is callback');
assert.fail('Error should not be emitted if there is callback');
}

function onError(err) {
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ TEST(function test_lookup_all_mixed(done) {
else if (isIPv6(ip.address))
assert.strictEqual(ip.family, 6);
else
assert(false);
assert.fail('unexpected IP address');
});

done();
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-tls-add-ca-cert.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ tls.connect(opts, fail).on('error', common.mustCall((err) => {
}));

function fail() {
assert(false, 'should fail to connect');
assert.fail('should fail to connect');
}

// New secure contexts have the well-known root CAs.
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-beforeexit-event-exit.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');

process.on('beforeExit', function() {
common.fail('exit should not allow this to occur');
assert.fail('exit should not allow this to occur');
});

process.exit();
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-fork-and-spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ switch (process.argv[2] || '') {
case 'spawn':
break;
default:
common.fail();
assert.fail();
}

function checkExit(statusCode) {
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-child-process-stdout-flush-exit.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ if (process.argv[2] === 'child') {

child.stderr.setEncoding('utf8');
child.stderr.on('data', function(data) {
console.log('parent stderr: ' + data);
assert.ok(false);
assert.fail(`Unexpected parent stderr: ${data}`);
});

// check if we receive both 'hello' at start and 'goodbye' at end
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-send-handle-twice.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ if (cluster.isMaster) {
setTimeout(function() { client.end(); }, 50);
}).on('error', function(e) {
console.error(e);
common.fail('server.listen failed');
assert.fail('server.listen failed');
cluster.worker.disconnect();
});
}
11 changes: 6 additions & 5 deletions test/parallel/test-domain-uncaught-exception.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,16 @@ if (process.argv[2] === 'child') {
test.expectedMessages.forEach(function(expectedMessage) {
if (test.messagesReceived === undefined ||
test.messagesReceived.indexOf(expectedMessage) === -1)
assert(false, `test ${test.fn.name} should have sent message: ${
expectedMessage} but didn't`);
assert.fail('test ' + test.fn.name + ' should have sent message: ' +
expectedMessage + ' but didn\'t');
});

if (test.messagesReceived) {
test.messagesReceived.forEach(function(receivedMessage) {
if (!test.expectedMessages.includes(receivedMessage)) {
assert(false, `test ${test.fn.name} should not have sent message: ${
receivedMessage} but did`);
if (test.expectedMessages.indexOf(receivedMessage) === -1) {
assert.fail('test ' + test.fn.name +
' should not have sent message: ' + receivedMessage +
' but did');
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-event-emitter-add-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ const EventEmitter = require('events');
});

ee.on('hello', hello);
ee.once('foo', common.fail);
ee.once('foo', assert.fail);
assert.deepStrictEqual(['hello', 'foo'], events_new_listener_emited);
assert.deepStrictEqual([hello, common.fail], listeners_new_listener_emited);
assert.deepStrictEqual([hello, assert.fail], listeners_new_listener_emited);

ee.emit('hello', 'a', 'b');
}
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-event-emitter-listeners-side-effects.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');

const EventEmitter = require('events').EventEmitter;
Expand All @@ -14,12 +14,12 @@ assert.strictEqual(fl.length, 0);
assert(!(e._events instanceof Object));
assert.deepStrictEqual(Object.keys(e._events), []);

e.on('foo', common.fail);
e.on('foo', assert.fail);
fl = e.listeners('foo');
assert.strictEqual(e._events.foo, common.fail);
assert.strictEqual(e._events.foo, assert.fail);
assert(Array.isArray(fl));
assert.strictEqual(fl.length, 1);
assert.strictEqual(fl[0], common.fail);
assert.strictEqual(fl[0], assert.fail);

e.listeners('bar');

Expand All @@ -28,12 +28,12 @@ fl = e.listeners('foo');

assert(Array.isArray(e._events.foo));
assert.strictEqual(e._events.foo.length, 2);
assert.strictEqual(e._events.foo[0], common.fail);
assert.strictEqual(e._events.foo[0], assert.fail);
assert.strictEqual(e._events.foo[1], assert.ok);

assert(Array.isArray(fl));
assert.strictEqual(fl.length, 2);
assert.strictEqual(fl[0], common.fail);
assert.strictEqual(fl[0], assert.fail);
assert.strictEqual(fl[1], assert.ok);

console.log('ok');
2 changes: 1 addition & 1 deletion test/parallel/test-event-emitter-once.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ e.emit('hello', 'a', 'b');
e.emit('hello', 'a', 'b');

function remove() {
common.fail('once->foo should not be emitted');
assert.fail('once->foo should not be emitted');
}

e.once('foo', remove);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-event-emitter-remove-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ function listener2() {}
const ee = new EventEmitter();

function remove1() {
common.fail('remove1 should not have been called');
assert.fail('remove1 should not have been called');
}

function remove2() {
common.fail('remove2 should not have been called');
assert.fail('remove2 should not have been called');
}

ee.on('removeListener', common.mustCall(function(name, cb) {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-exception-handler2.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const common = require('../common');
const assert = require('assert');

process.on('uncaughtException', function(err) {
console.log(`Caught exception: ${err}`);
Expand All @@ -11,4 +12,4 @@ setTimeout(common.mustCall(function() {

// Intentionally cause an exception, but don't catch it.
nonexistentFunc(); // eslint-disable-line no-undef
common.fail('This will not run.');
assert.fail('This will not run.');
2 changes: 1 addition & 1 deletion test/parallel/test-fs-stat.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) {
try {
stats = fs.fstatSync(fd);
} catch (err) {
common.fail(err);
assert.fail(err);
}
if (stats) {
console.dir(stats);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-write-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ common.refreshTmpDir();
const stream = fs.createWriteStream(file);

stream.on('drain', function() {
common.fail('\'drain\' event must not be emitted before ' +
assert.fail('\'drain\' event must not be emitted before ' +
'stream.write() has been called at least once.');
});
stream.destroy();
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-global-console-exists.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');
const EventEmitter = require('events');
const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/;
Expand All @@ -26,7 +26,7 @@ process.stderr.write = (data) => {
if (write_calls === 0)
assert.ok(leak_warning.test(data));
else
common.fail('stderr.write should be called only once');
assert.fail('stderr.write should be called only once');

write_calls++;
};
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-createConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const server = http.createServer(common.mustCall(function(req, res) {
res.resume();
fn = common.mustCall(createConnectionError);
http.get({ createConnection: fn }, function(res) {
common.fail('Unexpected response callback');
assert.fail('Unexpected response callback');
}).on('error', common.mustCall(function(err) {
assert.strictEqual(err.message, 'Could not create socket');
server.close();
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-invalid-path-chars.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');
const http = require('http');

Expand All @@ -8,7 +8,7 @@ const theExperimentallyDeterminedNumber = 39;

function fail(path) {
assert.throws(() => {
http.request({ path }, common.fail);
http.request({ path }, assert.fail);
}, expectedError);
}

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http-localaddress-bind-error.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

const invalidLocalAddress = '1.2.3.4';
Expand All @@ -22,7 +23,7 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
method: 'GET',
localAddress: invalidLocalAddress
}, function(res) {
common.fail('unexpectedly got response from server');
assert.fail('unexpectedly got response from server');
}).on('error', common.mustCall(function(e) {
console.log(`client got error: ${e.message}`);
server.close();
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http-mutable-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ const s = http.createServer(function(req, res) {
res.setHeader('x-foo', 'keyboard cat');
res.writeHead(200, { 'x-foo': 'bar', 'x-bar': 'baz' });
break;
default:
assert.fail('Unknown test');
}

res.statusCode = 201;
Expand Down Expand Up @@ -120,7 +122,7 @@ function nextTest() {
break;

default:
throw new Error('?');
assert.fail('Unknown test');
}

response.setEncoding('utf8');
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-response-multi-content-length.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const server = http.createServer((req, res) => {
res.writeHead(200, {'content-length': [1, 2]});
break;
default:
common.fail('should never get here');
assert.fail('should never get here');
}
res.end('ok');
});
Expand All @@ -35,7 +35,7 @@ server.listen(0, common.mustCall(() => {
http.get(
{port: server.address().port, headers: {'x-num': n}},
(res) => {
assert(false, 'client allowed multiple content-length headers.');
assert.fail('client allowed multiple content-length headers.');
}
).on('error', common.mustCall((err) => {
assert(/^Parse Error/.test(err.message));
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-response-splitting.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const common = require('../common');
require('../common');
const http = require('http');
const net = require('net');
const url = require('url');
Expand Down Expand Up @@ -38,7 +38,7 @@ const server = http.createServer((req, res) => {
test(res, 200, {'foo': y});
break;
default:
common.fail('should not get to here.');
assert.fail('should not get to here.');
}
if (count === 3)
server.close();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-response-status-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ testCases.findByPath = function(path) {
return testCase.path === path;
});
if (matching.length === 0) {
throw 'failed to find test case with path ' + path;
assert.fail(`failed to find test case with path ${path}`);
}
return matching[0];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ server.listen(0, () => {
client.on('data', (data) => {
// Should not get to this point because the server should simply
// close the connection without returning any data.
common.fail('no data should be returned by the server');
assert.fail('no data should be returned by the server');
});
client.on('end', common.mustCall(() => {}));
});
2 changes: 1 addition & 1 deletion test/parallel/test-http-unix-socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ server.listen(common.PIPE, common.mustCall(function() {
}));

req.on('error', function(e) {
common.fail(e.stack);
assert.fail(e.stack);
});

req.end();
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-https-localaddress-bind-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const fs = require('fs');
const https = require('https');

Expand Down Expand Up @@ -32,7 +33,7 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
method: 'GET',
localAddress: invalidLocalAddress
}, function(res) {
common.fail('unexpectedly got response from server');
assert.fail('unexpectedly got response from server');
}).on('error', common.mustCall(function(e) {
console.log(`client got error: ${e.message}`);
server.close();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-error-twice.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const srv = net.createServer(function onConnection(conn) {
conn.on('error', function(err) {
errs.push(err);
if (errs.length > 1 && errs[0] === errs[1])
assert(false, 'We should not be emitting the same error twice');
assert.fail('Should not emit the same error twice');
});
conn.on('close', function() {
srv.unref();
Expand Down
Loading

0 comments on commit e5b11f8

Please sign in to comment.