Skip to content

test: improve logged errors #31425

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.

Sign up for GitHub

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

Closed
BridgeAR wants to merge 2 commits into from
Closed

test: improve logged errors #31425

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62c91f7
test: improve logged errors
BridgeAR Jan 20, 2020
f4f440c
fixup: address comment
BridgeAR Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fixup: address comment
  • Loading branch information
@BridgeAR
BridgeAR committed Jan 24, 2020
commit f4f440c6cfbfc2f6f34e44b75751e344e701e763
25 changes: 14 additions & 11 deletions test/common/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const fs = require('fs');
// Do not require 'os' until needed so that test-os-checked-fucnction can
// monkey patch it. If 'os' is required here, that test will fail.
const path = require('path');
const { inspect } = require('util');
const util = require('util');
const { isMainThread } = require('worker_threads');

const tmpdir = require('./tmpdir');
Expand Down Expand Up @@ -88,7 +88,7 @@ if (process.argv.length === 2 &&
(process.features.inspector || !flag.startsWith('--inspect'))) {
console.log(
'NOTE: The test started as a child_process using these flags:',
inspect(flags),
util.inspect(flags),
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.'
);
const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
Expand Down Expand Up @@ -129,7 +129,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process.on('exit', () => {
// Iterate through handles to make sure nothing crashes
for (const k in initHandles)
inspect(initHandles[k]);
util.inspect(initHandles[k]);
});

const _queueDestroyAsyncId = async_wrap.queueDestroyAsyncId;
Expand All @@ -139,7 +139,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process._rawDebug();
throw new Error(`same id added to destroy list twice (${id})`);
}
destroyListList[id] = inspect(new Error());
destroyListList[id] = util.inspect(new Error());
Copy link
Copy Markdown
Member

@Trott Trott Jan 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's an example of a situation where this will result in a better stack?

Copy link
Copy Markdown
Member Author

@BridgeAR BridgeAR Feb 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node.js core frames are visualized in grey. It makes it easier to distinguish the test code from core code.

_queueDestroyAsyncId(id);
};

Expand All @@ -151,7 +151,10 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process._rawDebug(`Previous stack:\n${initHandles[id].stack}\n`);
throw new Error(`init called twice for same id (${id})`);
}
initHandles[id] = { resource, stack: inspect(new Error()).substr(6) };
initHandles[id] = {
resource,
stack: util.inspect(new Error()).substr(6)
};
},
before() { },
after() { },
Expand All @@ -161,7 +164,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process._rawDebug();
throw new Error(`destroy called for same id (${id})`);
}
destroydIdsList[id] = inspect(new Error());
destroydIdsList[id] = util.inspect(new Error());
},
}).enable();
}
Expand Down Expand Up @@ -345,7 +348,7 @@ function _mustCallInner(fn, criteria = 1, field) {
const context = {
[field]: criteria,
actual: 0,
stack: inspect(new Error()),
stack: util.inspect(new Error()),
name: fn.name || '<anonymous>'
};

Expand Down Expand Up @@ -511,7 +514,7 @@ function expectWarning(nameOrMap, expected, code) {
if (!catchWarning[warning.name]) {
throw new TypeError(
`"${warning.name}" was triggered without being expected.\n` +
inspect(warning)
util.inspect(warning)
);
}
catchWarning[warning.name](warning);
Expand All @@ -532,7 +535,7 @@ function expectsError(validator, exact) {
if (args.length !== 1) {
// Do not use `assert.strictEqual()` to prevent `inspect` from
// always being called.
assert.fail(`Expected one argument, got ${inspect(args)}`);
assert.fail(`Expected one argument, got ${util.inspect(args)}`);
}
const error = args.pop();
const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
Expand Down Expand Up @@ -661,9 +664,9 @@ function invalidArgTypeHelper(input) {
if (input.constructor && input.constructor.name) {
return ` Received an instance of ${input.constructor.name}`;
}
return ` Received ${inspect(input, { depth: -1 })}`;
return ` Received ${util.inspect(input, { depth: -1 })}`;
}
let inspected = inspect(input, { colors: false });
let inspected = util.inspect(input, { colors: false });
if (inspected.length > 25)
inspected = `${inspected.slice(0, 25)}...`;
return ` Received type ${typeof input} (${inspected})`;
Expand Down