Skip to content

Commit a4ee930

Browse files
BridgeARtargos
authored andcommitted
test: improve logged errors
To indicate which lines are test lines and which from Node.js core, it's good to rely on `util.inspect()` while inspecting errors. The stack was accessed directly instead in multiple cases and logging that does not provide as much information as using `util.inspect()`. PR-URL: #31425 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yihong Wang <[email protected]>
1 parent 9eeee0d commit a4ee930

14 files changed

+35
-30
lines changed

test/common/index.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,19 +139,22 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
139139
process._rawDebug();
140140
throw new Error(`same id added to destroy list twice (${id})`);
141141
}
142-
destroyListList[id] = new Error().stack;
142+
destroyListList[id] = util.inspect(new Error());
143143
_queueDestroyAsyncId(id);
144144
};
145145

146146
require('async_hooks').createHook({
147-
init(id, ty, tr, r) {
147+
init(id, ty, tr, resource) {
148148
if (initHandles[id]) {
149149
process._rawDebug(
150-
`Is same resource: ${r === initHandles[id].resource}`);
150+
`Is same resource: ${resource === initHandles[id].resource}`);
151151
process._rawDebug(`Previous stack:\n${initHandles[id].stack}\n`);
152152
throw new Error(`init called twice for same id (${id})`);
153153
}
154-
initHandles[id] = { resource: r, stack: new Error().stack.substr(6) };
154+
initHandles[id] = {
155+
resource,
156+
stack: util.inspect(new Error()).substr(6)
157+
};
155158
},
156159
before() { },
157160
after() { },
@@ -161,7 +164,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
161164
process._rawDebug();
162165
throw new Error(`destroy called for same id (${id})`);
163166
}
164-
destroydIdsList[id] = new Error().stack;
167+
destroydIdsList[id] = util.inspect(new Error());
165168
},
166169
}).enable();
167170
}
@@ -345,7 +348,7 @@ function _mustCallInner(fn, criteria = 1, field) {
345348
const context = {
346349
[field]: criteria,
347350
actual: 0,
348-
stack: (new Error()).stack,
351+
stack: util.inspect(new Error()),
349352
name: fn.name || '<anonymous>'
350353
};
351354

@@ -530,7 +533,7 @@ function expectWarning(nameOrMap, expected, code) {
530533
function expectsError(validator, exact) {
531534
return mustCall((...args) => {
532535
if (args.length !== 1) {
533-
// Do not use `assert.strictEqual()` to prevent `util.inspect` from
536+
// Do not use `assert.strictEqual()` to prevent `inspect` from
534537
// always being called.
535538
assert.fail(`Expected one argument, got ${util.inspect(args)}`);
536539
}

test/common/wpt.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const fs = require('fs');
77
const fsPromises = fs.promises;
88
const path = require('path');
99
const vm = require('vm');
10+
const { inspect } = require('util');
1011

1112
// https://github.com/w3c/testharness.js/blob/master/testharness.js
1213
// TODO: get rid of this half-baked harness in favor of the one
@@ -354,7 +355,7 @@ class WPTRunner {
354355
this.fail(filename, {
355356
name: '',
356357
message: err.message,
357-
stack: err.stack
358+
stack: inspect(err)
358359
}, 'UNCAUGHT');
359360
this.inProgress.delete(filename);
360361
}

test/fixtures/uncaught-exceptions/domain.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const domain = require('domain');
22

33
var d = domain.create();
44
d.on('error', function(err) {
5-
console.log('[ignored]', err.stack);
5+
console.log('[ignored]', err);
66
});
77

88
d.run(function() {

test/message/vm_display_runtime_error.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ console.error('beginning');
2828
try {
2929
vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm' });
3030
} catch (err) {
31-
console.error(err.stack);
31+
console.error(err);
3232
}
3333

3434
vm.runInThisContext('throw new Error("spooky!")', { filename: 'test.vm' });

test/message/vm_display_syntax_error.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ console.error('beginning');
2828
try {
2929
vm.runInThisContext('var 4;', { filename: 'foo.vm', displayErrors: true });
3030
} catch (err) {
31-
console.error(err.stack);
31+
console.error(err);
3232
}
3333

3434
vm.runInThisContext('var 5;', { filename: 'test.vm' });

test/parallel/test-assert-if-error.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ assert.ifError(undefined);
8383
} catch (e) {
8484
threw = true;
8585
assert.strictEqual(e.message, 'Missing expected exception.');
86-
assert(!e.stack.includes('throws'), e.stack);
86+
assert(!e.stack.includes('throws'), e);
8787
}
8888
assert(threw);
8989
}

test/parallel/test-assert.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ assert.throws(
390390
threw = true;
391391
assert.ok(e.message.includes(rangeError.message));
392392
assert.ok(e instanceof assert.AssertionError);
393-
assert.ok(!e.stack.includes('doesNotThrow'), e.stack);
393+
assert.ok(!e.stack.includes('doesNotThrow'), e);
394394
}
395395
assert.ok(threw);
396396
}

test/parallel/test-http-request-agent.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ server.listen(0, common.mustCall(function() {
3434
res.resume();
3535
server.close();
3636
})).on('error', function(e) {
37-
console.error(e.stack);
37+
console.error(e);
3838
process.exit(1);
3939
});
4040
}));

test/parallel/test-http-unix-socket.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ server.listen(common.PIPE, common.mustCall(function() {
6969
}));
7070

7171
req.on('error', function(e) {
72-
assert.fail(e.stack);
72+
assert.fail(e);
7373
});
7474

7575
req.end();

test/parallel/test-promises-unhandled-rejections.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const common = require('../common');
33
const assert = require('assert');
44
const domain = require('domain');
5+
const { inspect } = require('util');
56

67
common.disableCrashOnUnhandledRejection();
78

@@ -14,8 +15,8 @@ const asyncTest = (function() {
1415

1516
function fail(error) {
1617
const stack = currentTest ?
17-
`${error.stack}\nFrom previous event:\n${currentTest.stack}` :
18-
error.stack;
18+
`${inspect(error)}\nFrom previous event:\n${currentTest.stack}` :
19+
inspect(error);
1920

2021
if (currentTest)
2122
process.stderr.write(`'${currentTest.description}' failed\n\n`);
@@ -44,11 +45,11 @@ const asyncTest = (function() {
4445
}
4546

4647
return function asyncTest(description, fn) {
47-
const stack = new Error().stack.split('\n').slice(1).join('\n');
48+
const stack = inspect(new Error()).split('\n').slice(1).join('\n');
4849
asyncTestQueue.push({
4950
action: fn,
50-
stack: stack,
51-
description: description
51+
stack,
52+
description
5253
});
5354
if (!asyncTestsEnabled) {
5455
asyncTestsEnabled = true;

0 commit comments

Comments
 (0)