Skip to content

Commit

Permalink
assert: use object argument in innerFail
Browse files Browse the repository at this point in the history
Right now it is difficult to know what argument stands for what
property. By refactoring the arguments into a object it is clear
what stands for what.

PR-URL: #17582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
BridgeAR committed Dec 15, 2017
1 parent a364e7e commit 7cf569a
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 29 deletions.
118 changes: 92 additions & 26 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,13 @@ const assert = module.exports = ok;
// both the actual and expected values to the assertion error for
// display purposes.

function innerFail(actual, expected, message, operator, stackStartFunction) {
if (message instanceof Error) throw message;
function innerFail(obj) {
if (obj.message instanceof Error) throw obj.message;

throw new errors.AssertionError({
message,
actual,
expected,
operator,
stackStartFunction
});
throw new errors.AssertionError(obj);
}

function fail(actual, expected, message, operator, stackStartFunction) {
function fail(actual, expected, message, operator, stackStartFn) {
const argsLen = arguments.length;

if (argsLen === 0) {
Expand All @@ -60,7 +54,13 @@ function fail(actual, expected, message, operator, stackStartFunction) {
operator = '!=';
}

innerFail(actual, expected, message, operator, stackStartFunction || fail);
innerFail({
actual,
expected,
message,
operator,
stackStartFn: stackStartFn || fail
});
}

assert.fail = fail;
Expand All @@ -75,64 +75,121 @@ assert.AssertionError = errors.AssertionError;
// Pure assertion tests whether a value is truthy, as determined
// by !!value.
function ok(value, message) {
if (!value) innerFail(value, true, message, '==', ok);
if (!value) {
innerFail({
actual: value,
expected: true,
message,
operator: '==',
stackStartFn: ok
});
}
}
assert.ok = ok;

// The equality assertion tests shallow, coercive equality with ==.
/* eslint-disable no-restricted-properties */
assert.equal = function equal(actual, expected, message) {
// eslint-disable-next-line eqeqeq
if (actual != expected) innerFail(actual, expected, message, '==', equal);
if (actual != expected) {
innerFail({
actual,
expected,
message,
operator: '==',
stackStartFn: equal
});
}
};

// The non-equality assertion tests for whether two objects are not
// equal with !=.
assert.notEqual = function notEqual(actual, expected, message) {
// eslint-disable-next-line eqeqeq
if (actual == expected) {
innerFail(actual, expected, message, '!=', notEqual);
innerFail({
actual,
expected,
message,
operator: '!=',
stackStartFn: notEqual
});
}
};

// The equivalence assertion tests a deep equality relation.
assert.deepEqual = function deepEqual(actual, expected, message) {
if (!isDeepEqual(actual, expected)) {
innerFail(actual, expected, message, 'deepEqual', deepEqual);
innerFail({
actual,
expected,
message,
operator: 'deepEqual',
stackStartFn: deepEqual
});
}
};

// The non-equivalence assertion tests for any deep inequality.
assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
if (isDeepEqual(actual, expected)) {
innerFail(actual, expected, message, 'notDeepEqual', notDeepEqual);
innerFail({
actual,
expected,
message,
operator: 'notDeepEqual',
stackStartFn: notDeepEqual
});
}
};
/* eslint-enable */

assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
if (!isDeepStrictEqual(actual, expected)) {
innerFail(actual, expected, message, 'deepStrictEqual', deepStrictEqual);
innerFail({
actual,
expected,
message,
operator: 'deepStrictEqual',
stackStartFn: deepStrictEqual
});
}
};

assert.notDeepStrictEqual = notDeepStrictEqual;
function notDeepStrictEqual(actual, expected, message) {
if (isDeepStrictEqual(actual, expected)) {
innerFail(actual, expected, message, 'notDeepStrictEqual',
notDeepStrictEqual);
innerFail({
actual,
expected,
message,
operator: 'notDeepStrictEqual',
stackStartFn: notDeepStrictEqual
});
}
}

assert.strictEqual = function strictEqual(actual, expected, message) {
if (!Object.is(actual, expected)) {
innerFail(actual, expected, message, 'strictEqual', strictEqual);
innerFail({
actual,
expected,
message,
operator: 'strictEqual',
stackStartFn: strictEqual
});
}
};

assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
if (Object.is(actual, expected)) {
innerFail(actual, expected, message, 'notStrictEqual', notStrictEqual);
innerFail({
actual,
expected,
message,
operator: 'notStrictEqual',
stackStartFn: notStrictEqual
});
}
};

Expand Down Expand Up @@ -180,18 +237,27 @@ function innerThrows(shouldThrow, block, expected, message) {
details += ` (${expected.name})`;
}
details += message ? `: ${message}` : '.';
fail(actual, expected, `Missing expected exception${details}`, 'throws');
innerFail({
actual,
expected,
operator: 'throws',
message: `Missing expected exception${details}`,
stackStartFn: innerThrows
});
}
if (expected && expectedException(actual, expected) === false) {
throw actual;
}
} else if (actual !== undefined) {
if (!expected || expectedException(actual, expected)) {
details = message ? `: ${message}` : '.';
fail(actual,
expected,
`Got unwanted exception${details}\n${actual.message}`,
'doesNotThrow');
innerFail({
actual,
expected,
operator: 'doesNotThrow',
message: `Got unwanted exception${details}\n${actual.message}`,
stackStartFn: innerThrows
});
}
throw actual;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class AssertionError extends Error {
if (typeof options !== 'object' || options === null) {
throw new exports.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'Object');
}
var { actual, expected, message, operator, stackStartFunction } = options;
var { actual, expected, message, operator, stackStartFn } = options;
if (message) {
super(message);
} else {
Expand All @@ -155,7 +155,7 @@ class AssertionError extends Error {
this.actual = actual;
this.expected = expected;
this.operator = operator;
Error.captureStackTrace(this, stackStartFunction);
Error.captureStackTrace(this, stackStartFn);
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/message/error_exit.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Exiting with code=1
assert.js:*
throw new errors.AssertionError({
throw new errors.AssertionError(obj);
^

AssertionError [ERR_ASSERTION]: 1 strictEqual 2
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -780,3 +780,12 @@ common.expectsError(
/* eslint-enable no-restricted-properties */
assert(7);
}

common.expectsError(
() => assert.ok(null),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'null == true'
}
);

0 comments on commit 7cf569a

Please sign in to comment.