Skip to content

Commit

Permalink
assert: refactor to use more primordials
Browse files Browse the repository at this point in the history
PR-URL: #35998
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
aduh95 authored and codebytere committed Nov 22, 2020
1 parent 122797e commit 34aa0c8
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 25 deletions.
52 changes: 30 additions & 22 deletions lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict';

const {
ArrayPrototypeJoin,
ArrayPrototypePop,
Error,
ErrorCaptureStackTrace,
MathMax,
Expand All @@ -9,6 +11,10 @@ const {
ObjectGetPrototypeOf,
ObjectKeys,
String,
StringPrototypeEndsWith,
StringPrototypeRepeat,
StringPrototypeSlice,
StringPrototypeSplit,
} = primordials;

const { inspect } = require('internal/util/inspect');
Expand Down Expand Up @@ -79,8 +85,8 @@ function createErrDiff(actual, expected, operator) {
let end = '';
let skipped = false;
const actualInspected = inspectValue(actual);
const actualLines = actualInspected.split('\n');
const expectedLines = inspectValue(expected).split('\n');
const actualLines = StringPrototypeSplit(actualInspected, '\n');
const expectedLines = StringPrototypeSplit(inspectValue(expected), '\n');

let i = 0;
let indicator = '';
Expand Down Expand Up @@ -127,7 +133,7 @@ function createErrDiff(actual, expected, operator) {
if (i > 2) {
// Add position indicator for the first mismatch in case it is a
// single line and the input length is less than the column length.
indicator = `\n ${' '.repeat(i)}^`;
indicator = `\n ${StringPrototypeRepeat(' ', i)}^`;
i = 0;
}
}
Expand All @@ -144,8 +150,8 @@ function createErrDiff(actual, expected, operator) {
} else {
other = a;
}
actualLines.pop();
expectedLines.pop();
ArrayPrototypePop(actualLines);
ArrayPrototypePop(expectedLines);
if (actualLines.length === 0 || expectedLines.length === 0)
break;
a = actualLines[actualLines.length - 1];
Expand All @@ -157,18 +163,19 @@ function createErrDiff(actual, expected, operator) {
// E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
if (maxLines === 0) {
// We have to get the result again. The lines were all removed before.
const actualLines = actualInspected.split('\n');
const actualLines = StringPrototypeSplit(actualInspected, '\n');

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

return `${kReadableOperator.notIdentical}\n\n${actualLines.join('\n')}\n`;
return `${kReadableOperator.notIdentical}\n\n` +
`${ArrayPrototypeJoin(actualLines, '\n')}\n`;
}

// There were at least five identical lines at the end. Mark a couple of
Expand Down Expand Up @@ -235,9 +242,10 @@ function createErrDiff(actual, expected, operator) {
// If the lines diverge, specifically check for lines that only diverge by
// a trailing comma. In that case it is actually identical and we should
// mark it as such.
let divergingLines = actualLine !== expectedLine &&
(!actualLine.endsWith(',') ||
actualLine.slice(0, -1) !== expectedLine);
let divergingLines =
actualLine !== expectedLine &&
(!StringPrototypeEndsWith(actualLine, ',') ||
StringPrototypeSlice(actualLine, 0, -1) !== expectedLine);
// If the expected line has a trailing comma but is otherwise identical,
// add a comma at the end of the actual line. Otherwise the output could
// look weird as in:
Expand All @@ -248,8 +256,8 @@ function createErrDiff(actual, expected, operator) {
// ]
//
if (divergingLines &&
expectedLine.endsWith(',') &&
expectedLine.slice(0, -1) === actualLine) {
StringPrototypeEndsWith(expectedLine, ',') &&
StringPrototypeSlice(expectedLine, 0, -1) === actualLine) {
divergingLines = false;
actualLine += ',';
}
Expand Down Expand Up @@ -362,7 +370,7 @@ class AssertionError extends Error {
// In case the objects are equal but the operator requires unequal, show
// the first object and say A equals B
let base = kReadableOperator[operator];
const res = inspectValue(actual).split('\n');
const res = StringPrototypeSplit(inspectValue(actual), '\n');

// In case "actual" is an object or a function, it should not be
// reference equal.
Expand All @@ -377,15 +385,15 @@ class AssertionError extends Error {
if (res.length > 50) {
res[46] = `${blue}...${white}`;
while (res.length > 47) {
res.pop();
ArrayPrototypePop(res);
}
}

// Only print a single input.
if (res.length === 1) {
super(`${base}${res[0].length > 5 ? '\n\n' : ' '}${res[0]}`);
} else {
super(`${base}\n\n${res.join('\n')}\n`);
super(`${base}\n\n${ArrayPrototypeJoin(res, '\n')}\n`);
}
} else {
let res = inspectValue(actual);
Expand All @@ -394,15 +402,15 @@ class AssertionError extends Error {
if (operator === 'notDeepEqual' && res === other) {
res = `${knownOperator}\n\n${res}`;
if (res.length > 1024) {
res = `${res.slice(0, 1021)}...`;
res = `${StringPrototypeSlice(res, 0, 1021)}...`;
}
super(res);
} else {
if (res.length > 512) {
res = `${res.slice(0, 509)}...`;
res = `${StringPrototypeSlice(res, 0, 509)}...`;
}
if (other.length > 512) {
other = `${other.slice(0, 509)}...`;
other = `${StringPrototypeSlice(other, 0, 509)}...`;
}
if (operator === 'deepEqual') {
res = `${knownOperator}\n\n${res}\n\nshould loosely deep-equal\n\n`;
Expand Down Expand Up @@ -463,12 +471,12 @@ class AssertionError extends Error {

for (const name of ['actual', 'expected']) {
if (typeof this[name] === 'string') {
const lines = this[name].split('\n');
const lines = StringPrototypeSplit(this[name], '\n');
if (lines.length > 10) {
lines.length = 10;
this[name] = `${lines.join('\n')}\n...`;
this[name] = `${ArrayPrototypeJoin(lines, '\n')}\n...`;
} else if (this[name].length > 512) {
this[name] = `${this[name].slice(512)}...`;
this[name] = `${StringPrototypeSlice(this[name], 512)}...`;
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions lib/internal/assert/calltracker.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use strict';

const {
ArrayPrototypePush,
Error,
FunctionPrototype,
ReflectApply,
SafeSet,
} = primordials;

Expand All @@ -15,7 +18,7 @@ const {
validateUint32,
} = require('internal/validators');

const noop = () => {};
const noop = FunctionPrototype;

class CallTracker {

Expand Down Expand Up @@ -55,7 +58,7 @@ class CallTracker {
if (context.actual === context.exact + 1) {
callChecks.add(context);
}
return fn.apply(this, arguments);
return ReflectApply(fn, this, arguments);
};
}

Expand All @@ -67,7 +70,7 @@ class CallTracker {
const message = `Expected the ${context.name} function to be ` +
`executed ${context.exact} time(s) but was ` +
`executed ${context.actual} time(s).`;
errors.push({
ArrayPrototypePush(errors, {
message,
actual: context.actual,
expected: context.exact,
Expand Down

0 comments on commit 34aa0c8

Please sign in to comment.