Commit e580a44
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 <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>1 parent 624e516 commit e580a44
4 files changed
Lines changed: 39 additions & 12 deletions
File tree
- test
- common
- internet
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -33,10 +33,17 @@ class State { | |||
| 33 | 33 | (node) => [expectedChild.name, 'Node / ' + expectedChild.name] | |
| 34 | 34 | .includes(node.name); | |
| 35 | 35 | ||
| 36 | - assert(snapshot.some((node) => { | ||
| 36 | + const hasChild = snapshot.some((node) => { | ||
| 37 | 37 | 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 | + } | ||
| 40 | 47 | } | |
| 41 | 48 | } | |
| 42 | 49 | } | |
@@ -57,9 +64,15 @@ class State { | |||
| 57 | 64 | node.value.constructor.name === expectedChild.name); | |
| 58 | 65 | }; | |
| 59 | 66 | ||
| 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 | + } | ||
| 63 | 76 | } | |
| 64 | 77 | } | |
| 65 | 78 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -49,7 +49,12 @@ for (const tr in tests) { | |||
| 49 | 49 | { encoding: 'utf8' }); | |
| 50 | 50 | ||
| 51 | 51 | // 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 | + } | ||
| 53 | 58 | ||
| 54 | 59 | const file = path.join(tmpdir.path, traceFile); | |
| 55 | 60 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -136,7 +136,12 @@ for (const tr in tests) { | |||
| 136 | 136 | } | |
| 137 | 137 | ||
| 138 | 138 | // 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 | + } | ||
| 140 | 145 | ||
| 141 | 146 | // Confirm that trace log file is created. | |
| 142 | 147 | assert(fs.existsSync(traceFile)); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -27,14 +27,18 @@ assert.throws(common.mustCall(() => { | |||
| 27 | 27 | port2.onmessage = common.mustCall((message) => { | |
| 28 | 28 | assert.strictEqual(message, 2); | |
| 29 | 29 | ||
| 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); | ||
| 32 | 34 | ||
| 33 | 35 | port1.close(); | |
| 34 | 36 | ||
| 35 | 37 | 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); | ||
| 38 | 42 | }); | |
| 39 | 43 | }); | |
| 40 | 44 | port1.postMessage(2); | |
0 commit comments