Skip to content

Commit c322fca

Browse files
BridgeARjasnell
authored andcommitted
test: improve common.expectsError
The output is now improved by showing most properties all at once. Besides that this adds a warning to use `assert.throws` instead due to a better output. PR-URL: #19797 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 6949543 commit c322fca

File tree

3 files changed

+55
-30
lines changed

3 files changed

+55
-30
lines changed

test/common/index.js

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -690,52 +690,79 @@ Object.defineProperty(exports, 'hasSmallICU', {
690690
}
691691
});
692692

693+
class Comparison {
694+
constructor(obj, keys) {
695+
for (const key of keys) {
696+
if (key in obj)
697+
this[key] = obj[key];
698+
}
699+
}
700+
}
701+
693702
// Useful for testing expected internal/error objects
694703
exports.expectsError = function expectsError(fn, settings, exact) {
695704
if (typeof fn !== 'function') {
696705
exact = settings;
697706
settings = fn;
698707
fn = undefined;
699708
}
709+
700710
function innerFn(error) {
701711
const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
702712
assert.strictEqual(descriptor.enumerable,
703713
false, 'The error message should be non-enumerable');
714+
715+
let innerSettings = settings;
704716
if ('type' in settings) {
705717
const type = settings.type;
706718
if (type !== Error && !Error.isPrototypeOf(type)) {
707719
throw new TypeError('`settings.type` must inherit from `Error`');
708720
}
709-
assert(error instanceof type,
710-
`${error.name} is not instance of ${type.name}`);
711-
let typeName = error.constructor.name;
712-
if (typeName === 'NodeError' && type.name !== 'NodeError') {
713-
typeName = Object.getPrototypeOf(error.constructor).name;
721+
let constructor = error.constructor;
722+
if (constructor.name === 'NodeError' && type.name !== 'NodeError') {
723+
constructor = Object.getPrototypeOf(error.constructor);
714724
}
715-
assert.strictEqual(typeName, type.name);
725+
// Add the `type` to the error to properly compare and visualize it.
726+
if (!('type' in error))
727+
error.type = constructor;
716728
}
717-
if ('info' in settings) {
718-
assert.deepStrictEqual(error.info, settings.info);
719-
}
720-
if ('message' in settings) {
721-
const message = settings.message;
722-
if (typeof message === 'string') {
723-
assert.strictEqual(error.message, message);
724-
} else {
725-
assert(message.test(error.message),
726-
`${error.message} does not match ${message}`);
727-
}
729+
730+
if ('message' in settings &&
731+
typeof settings.message === 'object' &&
732+
settings.message.test(error.message)) {
733+
// Make a copy so we are able to modify the settings.
734+
innerSettings = Object.create(
735+
settings, Object.getOwnPropertyDescriptors(settings));
736+
// Visualize the message as identical in case of other errors.
737+
innerSettings.message = error.message;
728738
}
729739

730740
// Check all error properties.
731741
const keys = Object.keys(settings);
732742
for (const key of keys) {
733-
if (key === 'message' || key === 'type' || key === 'info')
734-
continue;
735-
const actual = error[key];
736-
const expected = settings[key];
737-
assert.strictEqual(actual, expected,
738-
`${key}: expected ${expected}, not ${actual}`);
743+
if (!util.isDeepStrictEqual(error[key], innerSettings[key])) {
744+
// Create placeholder objects to create a nice output.
745+
const a = new Comparison(error, keys);
746+
const b = new Comparison(innerSettings, keys);
747+
748+
const tmpLimit = Error.stackTraceLimit;
749+
Error.stackTraceLimit = 0;
750+
const err = new assert.AssertionError({
751+
actual: a,
752+
expected: b,
753+
operator: 'strictEqual',
754+
stackStartFn: assert.throws
755+
});
756+
Error.stackTraceLimit = tmpLimit;
757+
758+
throw new assert.AssertionError({
759+
actual: error,
760+
expected: settings,
761+
operator: 'common.expectsError',
762+
message: err.message
763+
});
764+
}
765+
739766
}
740767
return true;
741768
}

test/parallel/test-console-assign-undefined.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,17 @@
66
const tmp = global.console;
77
global.console = 42;
88

9-
const common = require('../common');
9+
require('../common');
1010
const assert = require('assert');
1111

1212
// Originally the console had a getter. Test twice to verify it had no side
1313
// effect.
1414
assert.strictEqual(global.console, 42);
1515
assert.strictEqual(global.console, 42);
1616

17-
common.expectsError(
17+
assert.throws(
1818
() => console.log('foo'),
19-
{
20-
type: TypeError
21-
}
19+
{ name: 'TypeError' }
2220
);
2321

2422
global.console = 1;

test/parallel/test-internal-errors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ common.expectsError(() => {
7777
}, { code: 'TEST_ERROR_1', type: RangeError });
7878
}, {
7979
code: 'ERR_ASSERTION',
80-
message: /^.+ is not instance of \S/
80+
message: /- type: \[Function: TypeError]\n\+ type: \[Function: RangeError]/
8181
});
8282

8383
common.expectsError(() => {
@@ -89,7 +89,7 @@ common.expectsError(() => {
8989
}, {
9090
code: 'ERR_ASSERTION',
9191
type: assert.AssertionError,
92-
message: /.+ does not match \S/
92+
message: /- message: 'Error for testing purposes: a'\n\+ message: \/\^Error/
9393
});
9494

9595
// Test ERR_INVALID_FD_TYPE

0 commit comments

Comments
 (0)