lib: make coverage work for Node.js

PR-URL: #23941 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yang Guo <[email protected]>
nodejs · Nov 5, 2018 · 7ee0cea · 7ee0cea
1 parent fdba226
commit 7ee0cea
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 48 deletions.
diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -362,6 +362,12 @@
     NativeModule._cache[this.id] = this;
   };
 
+  // coverage must be turned on early, so that we can collect
+  // it for Node.js' own internal libraries.
+  if (process.env.NODE_V8_COVERAGE) {
+    NativeModule.require('internal/process/coverage').setup();
+  }
+
   // This will be passed to the bootstrapNodeJSCore function in
   // bootstrap/node.js.
   return loaderExports;

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -103,9 +103,7 @@
       NativeModule.require('internal/process/write-coverage').setup();
 
     if (process.env.NODE_V8_COVERAGE) {
-      const { resolve } = NativeModule.require('path');
-      process.env.NODE_V8_COVERAGE = resolve(process.env.NODE_V8_COVERAGE);
-      NativeModule.require('internal/process/coverage').setup();
+      NativeModule.require('internal/process/coverage').setupExitHooks();
     }
 
     if (process.config.variables.v8_enable_inspector) {

diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js
@@ -1,65 +1,93 @@
 'use strict';
-const path = require('path');
-const { mkdirSync, writeFileSync } = require('fs');
-const hasInspector = process.config.variables.v8_enable_inspector === 1;
-let inspector = null;
-if (hasInspector) inspector = require('inspector');
-
-let session;
+let coverageConnection = null;
+let coverageDirectory;
 
 function writeCoverage() {
-  if (!session) {
+  if (!coverageConnection && coverageDirectory) {
     return;
   }
 
+  const { join } = require('path');
+  const { mkdirSync, writeFileSync } = require('fs');
   const { threadId } = require('internal/worker');
 
   const filename = `coverage-${process.pid}-${Date.now()}-${threadId}.json`;
   try {
-    // TODO(bcoe): switch to mkdirp once #22302 is addressed.
-    mkdirSync(process.env.NODE_V8_COVERAGE);
+    mkdirSync(coverageDirectory, { recursive: true });
   } catch (err) {
     if (err.code !== 'EEXIST') {
       console.error(err);
       return;
     }
   }
 
-  const target = path.join(process.env.NODE_V8_COVERAGE, filename);
-
+  const target = join(coverageDirectory, filename);
   try {
-    session.post('Profiler.takePreciseCoverage', (err, coverageInfo) => {
-      if (err) return console.error(err);
-      try {
-        writeFileSync(target, JSON.stringify(coverageInfo));
-      } catch (err) {
-        console.error(err);
-      }
-    });
+    disableAllAsyncHooks();
+    let msg;
+    coverageConnection._coverageCallback = function(_msg) {
+      msg = _msg;
+    };
+    coverageConnection.dispatch(JSON.stringify({
+      id: 3,
+      method: 'Profiler.takePreciseCoverage'
+    }));
+    const coverageInfo = JSON.parse(msg).result;
+    writeFileSync(target, JSON.stringify(coverageInfo));
   } catch (err) {
     console.error(err);
   } finally {
-    session.disconnect();
-    session = null;
+    coverageConnection.disconnect();
+    coverageConnection = null;
   }
 }
 
+function disableAllAsyncHooks() {
+  const { getHookArrays } = require('internal/async_hooks');
+  const [hooks_array] = getHookArrays();
+  hooks_array.forEach((hook) => { hook.disable(); });
+}
+
 exports.writeCoverage = writeCoverage;
 
 function setup() {
-  if (!hasInspector) {
-    console.warn('coverage currently only supported in main thread');
+  const { Connection } = process.binding('inspector');
+  if (!Connection) {
+    console.warn('inspector not enabled');
     return;
   }
 
-  session = new inspector.Session();
-  session.connect();
-  session.post('Profiler.enable');
-  session.post('Profiler.startPreciseCoverage', { callCount: true,
-                                                  detailed: true });
+  coverageConnection = new Connection((res) => {
+    if (coverageConnection._coverageCallback) {
+      coverageConnection._coverageCallback(res);
+    }
+  });
+  coverageConnection.dispatch(JSON.stringify({
+    id: 1,
+    method: 'Profiler.enable'
+  }));
+  coverageConnection.dispatch(JSON.stringify({
+    id: 2,
+    method: 'Profiler.startPreciseCoverage',
+    params: {
+      callCount: true,
+      detailed: true
+    }
+  }));
 
-  const reallyReallyExit = process.reallyExit;
+  try {
+    const { resolve } = require('path');
+    coverageDirectory = process.env.NODE_V8_COVERAGE =
+      resolve(process.env.NODE_V8_COVERAGE);
+  } catch (err) {
+    console.error(err);
+  }
+}
 
+exports.setup = setup;
+
+function setupExitHooks() {
+  const reallyReallyExit = process.reallyExit;
   process.reallyExit = function(code) {
     writeCoverage();
     reallyReallyExit(code);
@@ -68,4 +96,4 @@ function setup() {
   process.on('exit', writeCoverage);
 }
 
-exports.setup = setup;
+exports.setupExitHooks = setupExitHooks;
diff --git a/test/fixtures/v8-coverage/async-hooks.js b/test/fixtures/v8-coverage/async-hooks.js
@@ -0,0 +1,11 @@
+const async_hooks = require('async_hooks');
+const common = require('../../common');
+
+const hook = async_hooks.createHook({
+  init: common.mustNotCall(),
+  before: common.mustNotCall(),
+  after: common.mustNotCall(),
+  destroy: common.mustNotCall()
+});
+
+hook.enable();
diff --git a/test/parallel/test-heapdump-inspector.js b/test/parallel/test-heapdump-inspector.js
@@ -7,17 +7,38 @@ common.skipIfInspectorDisabled();
 const { validateSnapshotNodes } = require('../common/heap');
 const inspector = require('inspector');
 
-const session = new inspector.Session();
-validateSnapshotNodes('Node / JSBindingsConnection', []);
-session.connect();
-validateSnapshotNodes('Node / JSBindingsConnection', [
-  {
-    children: [
-      { node_name: 'Node / InspectorSession', edge_name: 'session' },
-      { node_name: 'Connection', edge_name: 'wrapped' },
-      (edge) => edge.name === 'callback' &&
-        (edge.to.type === undefined || // embedded graph
-         edge.to.type === 'closure') // snapshot
-    ]
+const snapshotNode = {
+  children: [
+    { node_name: 'Node / InspectorSession', edge_name: 'session' }
+  ]
+};
+
+// starts with no JSBindingsConnection (or 1 if coverage enabled).
+{
+  const expected = [];
+  if (process.env.NODE_V8_COVERAGE) {
+    expected.push(snapshotNode);
+  }
+  validateSnapshotNodes('Node / JSBindingsConnection', expected);
+}
+
+// JSBindingsConnection should be added.
+{
+  const session = new inspector.Session();
+  session.connect();
+  const expected = [
+    {
+      children: [
+        { node_name: 'Node / InspectorSession', edge_name: 'session' },
+        { node_name: 'Connection', edge_name: 'wrapped' },
+        (edge) => edge.name === 'callback' &&
+          (edge.to.type === undefined || // embedded graph
+           edge.to.type === 'closure') // snapshot
+      ]
+    }
+  ];
+  if (process.env.NODE_V8_COVERAGE) {
+    expected.push(snapshotNode);
   }
-]);
+  validateSnapshotNodes('Node / JSBindingsConnection', expected);
+}
diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js
@@ -108,6 +108,20 @@ function nextdir() {
   assert.strictEqual(fixtureCoverage, undefined);
 }
 
+// disables async hooks before writing coverage.
+{
+  const coverageDirectory = path.join(tmpdir.path, nextdir());
+  const output = spawnSync(process.execPath, [
+    require.resolve('../fixtures/v8-coverage/async-hooks')
+  ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
+  assert.strictEqual(output.status, 0);
+  const fixtureCoverage = getFixtureCoverage('async-hooks.js',
+                                             coverageDirectory);
+  assert.ok(fixtureCoverage);
+  // first branch executed.
+  assert.strictEqual(fixtureCoverage.functions[1].ranges[0].count, 1);
+}
+
 // extracts the coverage object for a given fixture name.
 function getFixtureCoverage(fixtureFile, coverageDirectory) {
   const coverageFiles = fs.readdirSync(coverageDirectory);

diff --git a/test/sequential/test-inspector-enabled.js b/test/sequential/test-inspector-enabled.js
@@ -20,7 +20,10 @@ assert(
 `;
 
 const args = ['--inspect', '-e', script];
-const child = spawn(process.execPath, args, { stdio: 'inherit' });
+const child = spawn(process.execPath, args, {
+  stdio: 'inherit',
+  env: { ...process.env, NODE_V8_COVERAGE: '' }
+});
 child.on('exit', (code, signal) => {
   process.exit(code || signal);
 });