Skip to content

Commit e580a44

Browse files
BridgeARtargos
authored andcommitted
test: don't inspect values if not necessary
The inspection triggered on each assert call eagerly even tough the assertion was never triggered. That caused significant CPU overhead. PR-URL: #22903 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 624e516 commit e580a44

File tree

4 files changed

+39
-12
lines changed

4 files changed

+39
-12
lines changed

test/common/heap.js

+19-6
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,17 @@ class State {
3333
(node) => [expectedChild.name, 'Node / ' + expectedChild.name]
3434
.includes(node.name);
3535

36-
assert(snapshot.some((node) => {
36+
const hasChild = snapshot.some((node) => {
3737
return node.outgoingEdges.map((edge) => edge.toNode).some(check);
38-
}), `expected to find child ${util.inspect(expectedChild)} ` +
39-
`in ${util.inspect(snapshot)}`);
38+
});
39+
// Don't use assert with a custom message here. Otherwise the
40+
// inspection in the message is done eagerly and wastes a lot of CPU
41+
// time.
42+
if (!hasChild) {
43+
throw new Error(
44+
'expected to find child ' +
45+
`${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
46+
}
4047
}
4148
}
4249
}
@@ -57,9 +64,15 @@ class State {
5764
node.value.constructor.name === expectedChild.name);
5865
};
5966

60-
assert(graph.some((node) => node.edges.some(check)),
61-
`expected to find child ${util.inspect(expectedChild)} ` +
62-
`in ${util.inspect(snapshot)}`);
67+
// Don't use assert with a custom message here. Otherwise the
68+
// inspection in the message is done eagerly and wastes a lot of CPU
69+
// time.
70+
const hasChild = graph.some((node) => node.edges.some(check));
71+
if (!hasChild) {
72+
throw new Error(
73+
'expected to find child ' +
74+
`${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
75+
}
6376
}
6477
}
6578
}

test/internet/test-trace-events-dns.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ for (const tr in tests) {
4949
{ encoding: 'utf8' });
5050

5151
// Make sure the operation is successful.
52-
assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
52+
// Don't use assert with a custom message here. Otherwise the
53+
// inspection in the message is done eagerly and wastes a lot of CPU
54+
// time.
55+
if (proc.status !== 0) {
56+
throw new Error(`${tr}:\n${util.inspect(proc)}`);
57+
}
5358

5459
const file = path.join(tmpdir.path, traceFile);
5560

test/parallel/test-trace-events-fs-sync.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,12 @@ for (const tr in tests) {
136136
}
137137

138138
// Make sure the operation is successful.
139-
assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
139+
// Don't use assert with a custom message here. Otherwise the
140+
// inspection in the message is done eagerly and wastes a lot of CPU
141+
// time.
142+
if (proc.status !== 0) {
143+
throw new Error(`${tr}:\n${util.inspect(proc)}`);
144+
}
140145

141146
// Confirm that trace log file is created.
142147
assert(fs.existsSync(traceFile));

test/parallel/test-worker-message-port-transfer-self.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,18 @@ assert.throws(common.mustCall(() => {
2727
port2.onmessage = common.mustCall((message) => {
2828
assert.strictEqual(message, 2);
2929

30-
assert(util.inspect(port1).includes('active: true'), util.inspect(port1));
31-
assert(util.inspect(port2).includes('active: true'), util.inspect(port2));
30+
const inspectedPort1 = util.inspect(port1);
31+
const inspectedPort2 = util.inspect(port2);
32+
assert(inspectedPort1.includes('active: true'), inspectedPort1);
33+
assert(inspectedPort2.includes('active: true'), inspectedPort2);
3234

3335
port1.close();
3436

3537
tick(10, () => {
36-
assert(util.inspect(port1).includes('active: false'), util.inspect(port1));
37-
assert(util.inspect(port2).includes('active: false'), util.inspect(port2));
38+
const inspectedPort1 = util.inspect(port1);
39+
const inspectedPort2 = util.inspect(port2);
40+
assert(inspectedPort1.includes('active: false'), inspectedPort1);
41+
assert(inspectedPort2.includes('active: false'), inspectedPort2);
3842
});
3943
});
4044
port1.postMessage(2);

0 commit comments

Comments
 (0)