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: https://github.com/nodejs/node/pull/19797
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This commit is contained in:
Ruben Bridgewater 2018-04-04 17:03:58 +02:00
parent c1078c4a3b
commit f85d5996db
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
3 changed files with 55 additions and 30 deletions

View File

@ -690,6 +690,15 @@ Object.defineProperty(exports, 'hasSmallICU', {
} }
}); });
class Comparison {
constructor(obj, keys) {
for (const key of keys) {
if (key in obj)
this[key] = obj[key];
}
}
}
// Useful for testing expected internal/error objects // Useful for testing expected internal/error objects
exports.expectsError = function expectsError(fn, settings, exact) { exports.expectsError = function expectsError(fn, settings, exact) {
if (typeof fn !== 'function') { if (typeof fn !== 'function') {
@ -697,45 +706,63 @@ exports.expectsError = function expectsError(fn, settings, exact) {
settings = fn; settings = fn;
fn = undefined; fn = undefined;
} }
function innerFn(error) { function innerFn(error) {
const descriptor = Object.getOwnPropertyDescriptor(error, 'message'); const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
assert.strictEqual(descriptor.enumerable, assert.strictEqual(descriptor.enumerable,
false, 'The error message should be non-enumerable'); false, 'The error message should be non-enumerable');
let innerSettings = settings;
if ('type' in settings) { if ('type' in settings) {
const type = settings.type; const type = settings.type;
if (type !== Error && !Error.isPrototypeOf(type)) { if (type !== Error && !Error.isPrototypeOf(type)) {
throw new TypeError('`settings.type` must inherit from `Error`'); throw new TypeError('`settings.type` must inherit from `Error`');
} }
assert(error instanceof type, let constructor = error.constructor;
`${error.name} is not instance of ${type.name}`); if (constructor.name === 'NodeError' && type.name !== 'NodeError') {
let typeName = error.constructor.name; constructor = Object.getPrototypeOf(error.constructor);
if (typeName === 'NodeError' && type.name !== 'NodeError') {
typeName = Object.getPrototypeOf(error.constructor).name;
} }
assert.strictEqual(typeName, type.name); // Add the `type` to the error to properly compare and visualize it.
if (!('type' in error))
error.type = constructor;
} }
if ('info' in settings) {
assert.deepStrictEqual(error.info, settings.info); if ('message' in settings &&
} typeof settings.message === 'object' &&
if ('message' in settings) { settings.message.test(error.message)) {
const message = settings.message; // Make a copy so we are able to modify the settings.
if (typeof message === 'string') { innerSettings = Object.create(
assert.strictEqual(error.message, message); settings, Object.getOwnPropertyDescriptors(settings));
} else { // Visualize the message as identical in case of other errors.
assert(message.test(error.message), innerSettings.message = error.message;
`${error.message} does not match ${message}`);
}
} }
// Check all error properties. // Check all error properties.
const keys = Object.keys(settings); const keys = Object.keys(settings);
for (const key of keys) { for (const key of keys) {
if (key === 'message' || key === 'type' || key === 'info') if (!util.isDeepStrictEqual(error[key], innerSettings[key])) {
continue; // Create placeholder objects to create a nice output.
const actual = error[key]; const a = new Comparison(error, keys);
const expected = settings[key]; const b = new Comparison(innerSettings, keys);
assert.strictEqual(actual, expected,
`${key}: expected ${expected}, not ${actual}`); const tmpLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
const err = new assert.AssertionError({
actual: a,
expected: b,
operator: 'strictEqual',
stackStartFn: assert.throws
});
Error.stackTraceLimit = tmpLimit;
throw new assert.AssertionError({
actual: error,
expected: settings,
operator: 'common.expectsError',
message: err.message
});
}
} }
return true; return true;
} }

View File

@ -6,7 +6,7 @@
const tmp = global.console; const tmp = global.console;
global.console = 42; global.console = 42;
const common = require('../common'); require('../common');
const assert = require('assert'); const assert = require('assert');
// Originally the console had a getter. Test twice to verify it had no side // Originally the console had a getter. Test twice to verify it had no side
@ -14,11 +14,9 @@ const assert = require('assert');
assert.strictEqual(global.console, 42); assert.strictEqual(global.console, 42);
assert.strictEqual(global.console, 42); assert.strictEqual(global.console, 42);
common.expectsError( assert.throws(
() => console.log('foo'), () => console.log('foo'),
{ { name: 'TypeError' }
type: TypeError
}
); );
global.console = 1; global.console = 1;

View File

@ -77,7 +77,7 @@ common.expectsError(() => {
}, { code: 'TEST_ERROR_1', type: RangeError }); }, { code: 'TEST_ERROR_1', type: RangeError });
}, { }, {
code: 'ERR_ASSERTION', code: 'ERR_ASSERTION',
message: /^.+ is not instance of \S/ message: /- type: \[Function: TypeError]\n\+ type: \[Function: RangeError]/
}); });
common.expectsError(() => { common.expectsError(() => {
@ -89,7 +89,7 @@ common.expectsError(() => {
}, { }, {
code: 'ERR_ASSERTION', code: 'ERR_ASSERTION',
type: assert.AssertionError, type: assert.AssertionError,
message: /.+ does not match \S/ message: /- message: 'Error for testing purposes: a'\n\+ message: \/\^Error/
}); });
// Test ERR_INVALID_FD_TYPE // Test ERR_INVALID_FD_TYPE