Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert: minor assert improvements #27525

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 8 additions & 15 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ function innerFail(obj) {
function fail(actual, expected, message, operator, stackStartFn) {
const argsLen = arguments.length;

let internalMessage;
if (argsLen === 0) {
internalMessage = 'Failed';
let internalMessage = false;
if (actual == null && argsLen <= 1) {
internalMessage = true;
message = 'Failed';
} else if (argsLen === 1) {
message = actual;
actual = undefined;
Expand All @@ -118,14 +119,11 @@ function fail(actual, expected, message, operator, stackStartFn) {
actual,
expected,
operator: operator === undefined ? 'fail' : operator,
stackStartFn: stackStartFn || fail
stackStartFn: stackStartFn || fail,
message
};
if (message !== undefined) {
errArgs.message = message;
}
const err = new AssertionError(errArgs);
if (internalMessage) {
err.message = internalMessage;
err.generatedMessage = true;
}
throw err;
Expand Down Expand Up @@ -174,8 +172,8 @@ function getCode(fd, line, column) {
let lines = 0;
// Prevent blocking the event loop by limiting the maximum amount of
// data that may be read.
let maxReads = 64; // bytesPerRead * maxReads = 512 kb
const bytesPerRead = 8192;
let maxReads = 2 ** 5; // bytesPerRead * maxReads = 512 kb
const bytesPerRead = 2 ** 14;
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
// Use a single buffer up front that is reused until the call site is found.
let buffer = Buffer.allocUnsafe(bytesPerRead);
while (maxReads-- !== 0) {
Expand Down Expand Up @@ -621,11 +619,6 @@ function checkIsPromise(obj) {
// Accept native ES6 promises and promises that are implemented in a similar
// way. Do not accept thenables that use a function as `obj` and that have no
// `catch` handler.

// TODO: thenables are checked up until they have the correct methods,
// but according to documentation, the `then` method should receive
// the `fulfill` and `reject` arguments as well or it may be never resolved.

return isPromise(obj) ||
obj !== null && typeof obj === 'object' &&
typeof obj.then === 'function' &&
Expand Down
63 changes: 35 additions & 28 deletions lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@ const kReadableOperator = {
strictEqual: 'Expected values to be strictly equal:',
strictEqualObject: 'Expected "actual" to be reference-equal to "expected":',
deepEqual: 'Expected values to be loosely deep-equal:',
equal: 'Expected values to be loosely equal:',
notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
notStrictEqual: 'Expected "actual" to be strictly unequal to:',
notStrictEqualObject:
'Expected "actual" not to be reference-equal to "expected":',
notDeepEqual: 'Expected "actual" not to be loosely deep-equal to:',
notEqual: 'Expected "actual" to be loosely unequal to:',
notIdentical: 'Values identical but not reference-equal:',
notDeepEqualUnequal: 'Expected values not to be loosely deep-equal:'
};

// Comparing short primitives should just show === / !== instead of using the
// diff.
const kMaxShortLength = 10;
const kMaxShortLength = 12;

function copyError(source) {
const keys = Object.keys(source);
Expand Down Expand Up @@ -81,13 +80,12 @@ function createErrDiff(actual, expected, operator) {
let i = 0;
let indicator = '';

// In case both values are objects explicitly mark them as not reference equal
// for the `strictEqual` operator.
// In case both values are objects or functions explicitly mark them as not
// reference equal for the `strictEqual` operator.
if (operator === 'strictEqual' &&
typeof actual === 'object' &&
typeof expected === 'object' &&
actual !== null &&
expected !== null) {
((typeof actual === 'object' && actual !== null &&
typeof expected === 'object' && expected !== null) ||
(typeof actual === 'function' && typeof expected === 'function'))) {
operator = 'strictEqualObject';
}

Expand Down Expand Up @@ -153,9 +151,9 @@ function createErrDiff(actual, expected, operator) {

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
if (actualLines.length > 30) {
actualLines[26] = `${blue}...${white}`;
while (actualLines.length > 27) {
if (actualLines.length > 50) {
actualLines[46] = `${blue}...${white}`;
while (actualLines.length > 47) {
actualLines.pop();
}
}
Expand Down Expand Up @@ -282,8 +280,8 @@ function createErrDiff(actual, expected, operator) {
}
}
}
// Inspected object to big (Show ~20 rows max)
if (printedLines > 20 && i < maxLines - 2) {
// Inspected object to big (Show ~50 rows max)
if (printedLines > 50 && i < maxLines - 2) {
return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` +
`${blue}...${white}`;
}
Expand Down Expand Up @@ -348,52 +346,61 @@ class AssertionError extends Error {
let base = kReadableOperator[operator];
const res = inspectValue(actual).split('\n');

// In case "actual" is an object, it should not be reference equal.
// In case "actual" is an object or a function, it should not be
// reference equal.
if (operator === 'notStrictEqual' &&
typeof actual === 'object' &&
actual !== null) {
(typeof actual === 'object' && actual !== null ||
typeof actual === 'function')) {
base = kReadableOperator.notStrictEqualObject;
}

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
if (res.length > 30) {
res[26] = `${blue}...${white}`;
while (res.length > 27) {
if (res.length > 50) {
res[46] = `${blue}...${white}`;
while (res.length > 47) {
res.pop();
}
}

// Only print a single input.
if (res.length === 1) {
super(`${base} ${res[0]}`);
super(`${base}${res[0].length > 5 ? '\n\n' : ' '}${res[0]}`);
} else {
super(`${base}\n\n${res.join('\n')}\n`);
}
} else {
let res = inspectValue(actual);
let other = '';
let other = inspectValue(expected);
const knownOperators = kReadableOperator[operator];
if (operator === 'notDeepEqual' || operator === 'notEqual') {
res = `${kReadableOperator[operator]}\n\n${res}`;
if ((operator === 'notDeepEqual' || operator === 'notEqual') &&
res === other) {
res = `${knownOperators}\n\n${res}`;
if (res.length > 1024) {
res = `${res.slice(0, 1021)}...`;
}
super(res);
} else {
other = `${inspectValue(expected)}`;
if (res.length > 512) {
res = `${res.slice(0, 509)}...`;
}
if (other.length > 512) {
other = `${other.slice(0, 509)}...`;
}
if (operator === 'deepEqual' || operator === 'equal') {
res = `${knownOperators}\n\n${res}\n\nshould equal\n\n`;
const eq = operator === 'deepEqual' ? 'deep-equal' : 'equal';
res = `${knownOperators}\n\n${res}\n\nshould loosely ${eq}\n\n`;
} else {
other = ` ${operator} ${other}`;
const newOperator = kReadableOperator[`${operator}Unequal`];
if (newOperator) {
const eq = operator === 'notDeepEqual' ? 'deep-equal' : 'equal';
res = `${newOperator}\n\n${res}\n\nshould not loosely ${eq}\n\n`;
} else {
other = ` ${operator} ${other}`;
}
}
super(`${res}${other}`);
}
super(`${res}${other}`);
}
}

Expand Down
28 changes: 21 additions & 7 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ function assertDeepAndStrictEqual(a, b) {
function assertNotDeepOrStrict(a, b, err) {
assert.throws(
() => assert.deepEqual(a, b),
err || re`${a}\n\nshould equal\n\n${b}`
err || re`${a}\n\nshould loosely deep-equal\n\n${b}`
);
assert.throws(
() => assert.deepStrictEqual(a, b),
Expand All @@ -204,7 +204,7 @@ function assertNotDeepOrStrict(a, b, err) {

assert.throws(
() => assert.deepEqual(b, a),
err || re`${b}\n\nshould equal\n\n${a}`
err || re`${b}\n\nshould loosely deep-equal\n\n${a}`
);
assert.throws(
() => assert.deepStrictEqual(b, a),
Expand Down Expand Up @@ -651,6 +651,20 @@ assertDeepAndStrictEqual(-0, -0);
assertDeepAndStrictEqual(a, b);
}

assert.throws(
() => assert.notDeepEqual(1, true),
{
message: /1\n\nshould not loosely deep-equal\n\ntrue/
}
);

assert.throws(
() => assert.notDeepEqual(1, 1),
{
message: /Expected "actual" not to be loosely deep-equal to:\n\n1/
}
);

assert.deepEqual(new Date(2000, 3, 14), new Date(2000, 3, 14));

assert.throws(() => { assert.deepEqual(new Date(), new Date(2000, 3, 14)); },
Expand Down Expand Up @@ -779,7 +793,7 @@ assert.throws(
() => assert.notDeepStrictEqual(new Date(2000, 3, 14), new Date(2000, 3, 14)),
{
name: 'AssertionError',
message: 'Expected "actual" not to be strictly deep-equal to: ' +
message: 'Expected "actual" not to be strictly deep-equal to:\n\n' +
util.inspect(new Date(2000, 3, 14))
}
);
Expand Down Expand Up @@ -815,11 +829,11 @@ assert.throws(
message: `${defaultMsgStartFull}\n\n+ /a/m\n- /a/`
});
assert.throws(
() => assert.deepStrictEqual(/a/igm, /a/im),
() => assert.deepStrictEqual(/aa/igm, /aa/im),
{
code: 'ERR_ASSERTION',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /a/gim\n- /a/im\n ^`
message: `${defaultMsgStartFull}\n\n+ /aa/gim\n- /aa/im\n ^`
});

{
Expand Down Expand Up @@ -939,12 +953,12 @@ assert.deepStrictEqual(obj1, obj2);
}

// Strict equal with identical objects that are not identical
// by reference and longer than 30 elements
// by reference and longer than 50 elements
// E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
{
const a = {};
const b = {};
for (let i = 0; i < 35; i++) {
for (let i = 0; i < 55; i++) {
a[`symbol${i}`] = Symbol();
b[`symbol${i}`] = Symbol();
}
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-assert-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ assert.throws(
operator: 'fail',
actual: undefined,
expected: undefined,
generatedMessage: true
generatedMessage: true,
stack: /Failed/
}
);

Expand Down
24 changes: 16 additions & 8 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,20 @@ assert.throws(
assert.throws(
() => a.notStrictEqual('a '.repeat(30), 'a '.repeat(30)),
{
message: 'Expected "actual" to be strictly unequal to: ' +
message: 'Expected "actual" to be strictly unequal to:\n\n' +
`'${'a '.repeat(30)}'`,
name: 'AssertionError'
}
);

assert.throws(
() => a.notEqual(1, 1),
{
message: '1 != 1',
operator: '!='
}
);

a.notStrictEqual(2, '2');

// Testing the throwing.
Expand Down Expand Up @@ -281,12 +289,12 @@ testShortAssertionMessage('a', '"a"');
testShortAssertionMessage('foo', '\'foo\'');
testShortAssertionMessage(0, '0');
testShortAssertionMessage(Symbol(), 'Symbol()');
testShortAssertionMessage(undefined, 'undefined');
testShortAssertionMessage(-Infinity, '-Infinity');
testAssertionMessage([], '[]');
testAssertionMessage(/a/, '/a/');
testAssertionMessage(/abc/gim, '/abc/gim');
testAssertionMessage({}, '{}');
testAssertionMessage(undefined, 'undefined');
testAssertionMessage(-Infinity, '-Infinity');
testAssertionMessage([1, 2, 3], '[\n+ 1,\n+ 2,\n+ 3\n+ ]');
testAssertionMessage(function f() {}, '[Function: f]');
testAssertionMessage(function() {}, '[Function (anonymous)]');
Expand Down Expand Up @@ -579,12 +587,12 @@ assert.throws(
`${actExp} ... Lines skipped\n` +
'\n' +
' [\n' +
'+ 1,\n'.repeat(10) +
'+ 1,\n'.repeat(25) +
'...\n' +
'- 2,\n'.repeat(10) +
'- 2,\n'.repeat(25) +
'...';
assert.throws(
() => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)),
() => assert.deepEqual(Array(28).fill(1), Array(28).fill(2)),
{ message });

const obj1 = {};
Expand Down Expand Up @@ -612,8 +620,8 @@ assert.throws(
);

message = 'Expected "actual" not to be strictly deep-equal to:' +
`\n\n[${'\n 1,'.repeat(25)}\n...\n`;
const data = Array(31).fill(1);
`\n\n[${'\n 1,'.repeat(45)}\n...\n`;
const data = Array(51).fill(1);
assert.throws(
() => assert.notDeepEqual(data, data),
{ message });
Expand Down
2 changes: 1 addition & 1 deletion test/pseudo-tty/test-assert-position-indicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ assert.throws(

// Confirm that there is a position indicator.
assert.throws(
() => { assert.deepStrictEqual('aaa', 'aaaa'); },
() => { assert.deepStrictEqual('aaaa', 'aaaaa'); },
(err) => err.message.includes('^')
);