Skip to content

Commit 590f6c6

Browse files
committed
test_runner: set the properties inside of the error method
This improves the error recreation performance due to limiting the stack trace that is generated and it makes sure NodeErrors only set properties inside of the actual error method. Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 675cbcb commit 590f6c6

3 files changed

Lines changed: 13 additions & 8 deletions

File tree

lib/internal/errors.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1529,7 +1529,7 @@ E('ERR_TAP_VALIDATION_ERROR', function(errorMsg) {
15291529
hideInternalStackFrames(this);
15301530
return errorMsg;
15311531
}, Error);
1532-
E('ERR_TEST_FAILURE', function(error, failureType) {
1532+
E('ERR_TEST_FAILURE', function(error, failureType, extraProperties = undefined) {
15331533
hideInternalStackFrames(this);
15341534
assert(typeof failureType === 'string',
15351535
"The 'failureType' argument must be of type string.");
@@ -1542,6 +1542,9 @@ E('ERR_TEST_FAILURE', function(error, failureType) {
15421542

15431543
this.failureType = failureType;
15441544
this.cause = error;
1545+
if (extraProperties !== undefined) {
1546+
ObjectAssign(this, extraProperties);
1547+
}
15451548
return msg;
15461549
}, Error);
15471550
E('ERR_TLS_CERT_ALTNAME_FORMAT', 'Invalid subject alternative name string',

lib/internal/test_runner/runner.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const {
1010
ArrayPrototypeSort,
1111
FunctionPrototypeCall,
1212
Number,
13-
ObjectAssign,
1413
ObjectKeys,
1514
PromisePrototypeThen,
1615
SafePromiseAllReturnVoid,
@@ -32,6 +31,7 @@ const {
3231
codes: {
3332
ERR_TEST_FAILURE,
3433
},
34+
setStackTraceLimit,
3535
} = require('internal/errors');
3636
const { validateArray, validateBoolean } = require('internal/validators');
3737
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
@@ -306,16 +306,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
306306
if (code !== 0 || signal !== null) {
307307
if (!err) {
308308
const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
309-
// TODO(BridgeAR): The stack should not be created in the first place
310-
// and the properties should be set inside of the function.
311-
err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
309+
const { stackTraceLimit } = Error;
310+
setStackTraceLimit(0);
311+
err = new ERR_TEST_FAILURE('test failed', failureType, {
312312
__proto__: null,
313313
exitCode: code,
314314
signal: signal,
315-
// The stack will not be useful since the failures came from tests
316-
// in a child process.
317-
stack: undefined,
318315
});
316+
setStackTraceLimit(stackTraceLimit);
319317
}
320318

321319
throw err;

lib/internal/test_runner/yaml_to_js.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const {
33
codes: {
44
ERR_TEST_FAILURE,
55
},
6+
setStackTraceLimit,
67
} = require('internal/errors');
78
const AssertionError = require('internal/assert/assertion_error');
89
const {
@@ -32,6 +33,8 @@ function reConstructError(parsedYaml) {
3233
const stack = parsedYaml.stack ? kStackDelimiter + ArrayPrototypeJoin(parsedYaml.stack, `\n${kStackDelimiter}`) : '';
3334
let error, cause;
3435

36+
const { stackTraceLimit } = Error;
37+
setStackTraceLimit(0);
3538
if (isAssertionError) {
3639
cause = new AssertionError({
3740
message: parsedYaml.error,
@@ -50,6 +53,7 @@ function reConstructError(parsedYaml) {
5053
error = new ERR_TEST_FAILURE(cause, parsedYaml.failureType);
5154
error.stack = stack;
5255
}
56+
setStackTraceLimit(stackTraceLimit);
5357

5458
parsedYaml.error = error ?? cause;
5559
delete parsedYaml.stack;

0 commit comments

Comments
 (0)