Skip to content

Commit

Permalink
assert: align argument names
Browse files Browse the repository at this point in the history 
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.

PR-URL: nodejs#22760
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
  • Loading branch information
@BridgeAR
BridgeAR committed Sep 11, 2018
1 parent e4dd213 commit b5430d7
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 81 deletions.
41 changes: 21 additions & 20 deletions lib/assert.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,12 +555,12 @@ function expectedException(actual, expected, msg) {
return expected.call({}, actual) === true;
}

function getActual(block) {
if (typeof block !== 'function') {
throw new ERR_INVALID_ARG_TYPE('block', 'Function', block);
function getActual(fn) {
if (typeof fn !== 'function') {
throw new ERR_INVALID_ARG_TYPE('fn', 'Function', fn);
}
try {
block();
fn();
} catch (e) {
return e;
}
Expand All @@ -577,20 +577,21 @@ function checkIsPromise(obj) {
typeof obj.catch === 'function';
}

async function waitForActual(block) {
async function waitForActual(promiseFn) {
let resultPromise;
if (typeof block === 'function') {
// Return a rejected promise if `block` throws synchronously.
resultPromise = block();
if (typeof promiseFn === 'function') {
// Return a rejected promise if `promiseFn` throws synchronously.
resultPromise = promiseFn();
// Fail in case no promise is returned.
if (!checkIsPromise(resultPromise)) {
throw new ERR_INVALID_RETURN_VALUE('instance of Promise',
'block', resultPromise);
'promiseFn', resultPromise);
}
} else if (checkIsPromise(block)) {
resultPromise = block;
} else if (checkIsPromise(promiseFn)) {
resultPromise = promiseFn;
} else {
throw new ERR_INVALID_ARG_TYPE('block', ['Function', 'Promise'], block);
throw new ERR_INVALID_ARG_TYPE(
'promiseFn', ['Function', 'Promise'], promiseFn);
}

try {
Expand Down Expand Up @@ -676,20 +677,20 @@ function expectsNoError(stackStartFn, actual, error, message) {
throw actual;
}

assert.throws = function throws(block, ...args) {
expectsError(throws, getActual(block), ...args);
assert.throws = function throws(promiseFn, ...args) {
expectsError(throws, getActual(promiseFn), ...args);
};

assert.rejects = async function rejects(block, ...args) {
expectsError(rejects, await waitForActual(block), ...args);
assert.rejects = async function rejects(promiseFn, ...args) {
expectsError(rejects, await waitForActual(promiseFn), ...args);
};

assert.doesNotThrow = function doesNotThrow(block, ...args) {
expectsNoError(doesNotThrow, getActual(block), ...args);
assert.doesNotThrow = function doesNotThrow(fn, ...args) {
expectsNoError(doesNotThrow, getActual(fn), ...args);
};

assert.doesNotReject = async function doesNotReject(block, ...args) {
expectsNoError(doesNotReject, await waitForActual(block), ...args);
assert.doesNotReject = async function doesNotReject(fn, ...args) {
expectsNoError(doesNotReject, await waitForActual(fn), ...args);
};

assert.ifError = function ifError(err) {
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-assert-async.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const promises = [];
name: 'TypeError [ERR_INVALID_RETURN_VALUE]',
code: 'ERR_INVALID_RETURN_VALUE',
message: 'Expected instance of Promise to be returned ' +
'from the "block" function but got type undefined.'
'from the "promiseFn" function but got type undefined.'
}));

promise = assert.rejects(Promise.resolve(), common.mustNotCall());
Expand All @@ -62,7 +62,7 @@ promises.push(assert.rejects(
assert.rejects('fail', {}),
{
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "block" argument must be one of type ' +
message: 'The "promiseFn" argument must be one of type ' +
'Function or Promise. Received type string'
}
));
Expand All @@ -73,7 +73,7 @@ promises.push(assert.rejects(
const promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
promises.push(assert.rejects(promise, {
message: 'Expected instance of Promise to be returned ' +
'from the "block" function but got instance of Map.',
'from the "promiseFn" function but got instance of Map.',
code: 'ERR_INVALID_RETURN_VALUE',
name: 'TypeError [ERR_INVALID_RETURN_VALUE]'
}));
Expand Down Expand Up @@ -116,7 +116,7 @@ promises.push(assert.rejects(
assert.doesNotReject(123),
{
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "block" argument must be one of type ' +
message: 'The "promiseFn" argument must be one of type ' +
'Function or Promise. Received type number'
}
));
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-assert.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,15 @@ try {
}

{
// Verify that throws() and doesNotThrow() throw on non-function block.
const testBlockTypeError = (method, block) => {
// Verify that throws() and doesNotThrow() throw on non-functions.
const testBlockTypeError = (method, fn) => {
common.expectsError(
() => method(block),
() => method(fn),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "block" argument must be of type Function. Received ' +
`type ${typeof block}`
message: 'The "fn" argument must be of type Function. Received ' +
`type ${typeof fn}`
}
);
};
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-file-write-stream3.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function run_test_3() {

const run_test_4 = common.mustCall(function() {
// Error: start must be >= zero
const block = () => {
const fn = () => {
fs.createWriteStream(filepath, { start: -5, flags: 'r+' });
};
const err = {
Expand All @@ -187,7 +187,7 @@ const run_test_4 = common.mustCall(function() {
'It must be >= 0. Received {start: -5}',
type: RangeError
};
common.expectsError(block, err);
common.expectsError(fn, err);
});

run_test_1();
85 changes: 38 additions & 47 deletions test/parallel/test-net-connect-options-port.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ const net = require('net');
const hints = (dns.ADDRCONFIG | dns.V4MAPPED) + 42;
const hintOptBlocks = doConnect([{ hints }],
() => common.mustNotCall());
for (const block of hintOptBlocks) {
common.expectsError(block, {
for (const fn of hintOptBlocks) {
common.expectsError(fn, {
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: /The value "\d+" is invalid for option "hints"/
Expand Down Expand Up @@ -136,67 +136,59 @@ function doConnect(args, getCb) {
function syncFailToConnect(port, assertErr, optOnly) {
if (!optOnly) {
// connect(port, cb) and connect(port)
const portArgBlocks = doConnect([port], () => common.mustNotCall());
for (const block of portArgBlocks) {
assert.throws(block,
assertErr,
`${block.name}(${port})`);
const portArgFunctions = doConnect([port], () => common.mustNotCall());
for (const fn of portArgFunctions) {
assert.throws(fn, assertErr, `${fn.name}(${port})`);
}

// connect(port, host, cb) and connect(port, host)
const portHostArgBlocks = doConnect([port, 'localhost'],
() => common.mustNotCall());
for (const block of portHostArgBlocks) {
assert.throws(block,
assertErr,
`${block.name}(${port}, 'localhost')`);
const portHostArgFunctions = doConnect([port, 'localhost'],
() => common.mustNotCall());
for (const fn of portHostArgFunctions) {
assert.throws(fn, assertErr, `${fn.name}(${port}, 'localhost')`);
}
}
// connect({port}, cb) and connect({port})
const portOptBlocks = doConnect([{ port }],
() => common.mustNotCall());
for (const block of portOptBlocks) {
assert.throws(block,
assertErr,
`${block.name}({port: ${port}})`);
const portOptFunctions = doConnect([{ port }], () => common.mustNotCall());
for (const fn of portOptFunctions) {
assert.throws(fn, assertErr, `${fn.name}({port: ${port}})`);
}

// connect({port, host}, cb) and connect({port, host})
const portHostOptBlocks = doConnect([{ port: port, host: 'localhost' }],
() => common.mustNotCall());
for (const block of portHostOptBlocks) {
assert.throws(block,
const portHostOptFunctions = doConnect([{ port: port, host: 'localhost' }],
() => common.mustNotCall());
for (const fn of portHostOptFunctions) {
assert.throws(fn,
assertErr,
`${block.name}({port: ${port}, host: 'localhost'})`);
`${fn.name}({port: ${port}, host: 'localhost'})`);
}
}

function canConnect(port) {
const noop = () => common.mustCall();

// connect(port, cb) and connect(port)
const portArgBlocks = doConnect([port], noop);
for (const block of portArgBlocks) {
block();
const portArgFunctions = doConnect([port], noop);
for (const fn of portArgFunctions) {
fn();
}

// connect(port, host, cb) and connect(port, host)
const portHostArgBlocks = doConnect([port, 'localhost'], noop);
for (const block of portHostArgBlocks) {
block();
const portHostArgFunctions = doConnect([port, 'localhost'], noop);
for (const fn of portHostArgFunctions) {
fn();
}

// connect({port}, cb) and connect({port})
const portOptBlocks = doConnect([{ port }], noop);
for (const block of portOptBlocks) {
block();
const portOptFunctions = doConnect([{ port }], noop);
for (const fn of portOptFunctions) {
fn();
}

// connect({port, host}, cb) and connect({port, host})
const portHostOptBlocks = doConnect([{ port: port, host: 'localhost' }],
noop);
for (const block of portHostOptBlocks) {
block();
const portHostOptFns = doConnect([{ port, host: 'localhost' }], noop);
for (const fn of portHostOptFns) {
fn();
}
}

Expand All @@ -208,21 +200,20 @@ function asyncFailToConnect(port) {

const dont = () => common.mustNotCall();
// connect(port, cb) and connect(port)
const portArgBlocks = doConnect([port], dont);
for (const block of portArgBlocks) {
block().on('error', onError());
const portArgFunctions = doConnect([port], dont);
for (const fn of portArgFunctions) {
fn().on('error', onError());
}

// connect({port}, cb) and connect({port})
const portOptBlocks = doConnect([{ port }], dont);
for (const block of portOptBlocks) {
block().on('error', onError());
const portOptFunctions = doConnect([{ port }], dont);
for (const fn of portOptFunctions) {
fn().on('error', onError());
}

// connect({port, host}, cb) and connect({port, host})
const portHostOptBlocks = doConnect([{ port: port, host: 'localhost' }],
dont);
for (const block of portHostOptBlocks) {
block().on('error', onError());
const portHostOptFns = doConnect([{ port, host: 'localhost' }], dont);
for (const fn of portHostOptFns) {
fn().on('error', onError());
}
}
6 changes: 3 additions & 3 deletions test/parallel/test-net-server-listen-options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,20 @@ const listenOnPort = [

{
function shouldFailToListen(options) {
const block = () => {
const fn = () => {
net.createServer().listen(options, common.mustNotCall());
};

if (typeof options === 'object' &&
!(('port' in options) || ('path' in options))) {
common.expectsError(block,
common.expectsError(fn,
{
code: 'ERR_INVALID_ARG_VALUE',
type: TypeError,
message: /^The argument 'options' must have the property "port" or "path"\. Received .+$/,
});
} else {
common.expectsError(block,
common.expectsError(fn,
{
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
Expand Down

0 comments on commit b5430d7

Please sign in to comment.