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: refactor to use more primordials #35998

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 30 additions & 22 deletions lib/internal/assert/assertion_error.js
@@ -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
@@ -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