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]>
nodejs · Sep 19, 2018 · 57f2c12 · 57f2c12
1 parent 6975e08
commit 57f2c12
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 12 deletions.
diff --git a/test/common/heap.js b/test/common/heap.js
@@ -33,10 +33,17 @@ class State {
             (node) => [expectedChild.name, 'Node / ' + expectedChild.name]
               .includes(node.name);
 
-          assert(snapshot.some((node) => {
+          const hasChild = snapshot.some((node) => {
             return node.outgoingEdges.map((edge) => edge.toNode).some(check);
-          }), `expected to find child ${util.inspect(expectedChild)} ` +
-              `in ${util.inspect(snapshot)}`);
+          });
+          // Don't use assert with a custom message here. Otherwise the
+          // inspection in the message is done eagerly and wastes a lot of CPU
+          // time.
+          if (!hasChild) {
+            throw new Error(
+              'expected to find child ' +
+              `${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
+          }
         }
       }
     }
@@ -57,9 +64,15 @@ class State {
                  node.value.constructor.name === expectedChild.name);
             };
 
-          assert(graph.some((node) => node.edges.some(check)),
-                 `expected to find child ${util.inspect(expectedChild)} ` +
-                 `in ${util.inspect(snapshot)}`);
+          // Don't use assert with a custom message here. Otherwise the
+          // inspection in the message is done eagerly and wastes a lot of CPU
+          // time.
+          const hasChild = graph.some((node) => node.edges.some(check));
+          if (!hasChild) {
+            throw new Error(
+              'expected to find child ' +
+              `${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
+          }
         }
       }
     }

diff --git a/test/internet/test-trace-events-dns.js b/test/internet/test-trace-events-dns.js
@@ -49,7 +49,12 @@ for (const tr in tests) {
                             { encoding: 'utf8' });
 
   // Make sure the operation is successful.
-  assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
+  // Don't use assert with a custom message here. Otherwise the
+  // inspection in the message is done eagerly and wastes a lot of CPU
+  // time.
+  if (proc.status !== 0) {
+    throw new Error(`${tr}:\n${util.inspect(proc)}`);
+  }
 
   const file = path.join(tmpdir.path, traceFile);
 

diff --git a/test/parallel/test-trace-events-fs-sync.js b/test/parallel/test-trace-events-fs-sync.js
@@ -136,7 +136,12 @@ for (const tr in tests) {
   }
 
   // Make sure the operation is successful.
-  assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
+  // Don't use assert with a custom message here. Otherwise the
+  // inspection in the message is done eagerly and wastes a lot of CPU
+  // time.
+  if (proc.status !== 0) {
+    throw new Error(`${tr}:\n${util.inspect(proc)}`);
+  }
 
   // Confirm that trace log file is created.
   assert(fs.existsSync(traceFile));

diff --git a/test/parallel/test-worker-message-port-transfer-self.js b/test/parallel/test-worker-message-port-transfer-self.js
@@ -27,14 +27,18 @@ assert.throws(common.mustCall(() => {
 port2.onmessage = common.mustCall((message) => {
   assert.strictEqual(message, 2);
 
-  assert(util.inspect(port1).includes('active: true'), util.inspect(port1));
-  assert(util.inspect(port2).includes('active: true'), util.inspect(port2));
+  const inspectedPort1 = util.inspect(port1);
+  const inspectedPort2 = util.inspect(port2);
+  assert(inspectedPort1.includes('active: true'), inspectedPort1);
+  assert(inspectedPort2.includes('active: true'), inspectedPort2);
 
   port1.close();
 
   tick(10, () => {
-    assert(util.inspect(port1).includes('active: false'), util.inspect(port1));
-    assert(util.inspect(port2).includes('active: false'), util.inspect(port2));
+    const inspectedPort1 = util.inspect(port1);
+    const inspectedPort2 = util.inspect(port2);
+    assert(inspectedPort1.includes('active: false'), inspectedPort1);
+    assert(inspectedPort2.includes('active: false'), inspectedPort2);
   });
 });
 port1.postMessage(2);