process: refactor coverage setup during bootstrap

- Renamed `internal/process/write-coverage.js` to
  `internal/coverage-gen/with_instrumentation.js`,
  `internal/process/coverage.js` to
  `internal/coverage-gen/with_profiler.js` to distinguish
  the two better and added comments.
- Separate the coverage directory setup and the connection
  setup, moves the directory setup into `node.js` and
  closer to the exit hooks because that's where it's used.
- Moves the `process.reallyExit` overwrite and
  `process.on('exit')` hooks setup into bootstrap/node.js
  for clarity, and move them to a later stage of
  bootstrap since they do not have to happen that early.

PR-URL: #25398
Reviewed-By: Ben Coe <[email protected]>

Author: joyeecheung

lib/internal/bootstrap/loaders.js

@@ -329,7 +329,14 @@ NativeModule.prototype.compile = function() {
 // 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();
+  if (internalBinding('config').hasInspector) {
+    const coverage =
+      NativeModule.require('internal/coverage-gen/with_profiler');
+    // Inform the profiler to start collecting coverage
+    coverage.startCoverageCollection();
+  } else {
+    process._rawDebug('NODE_V8_COVERAGE cannot be used without inspector');
+  }
 }
 
 function internalBindingWhitelistHas(name) {

lib/internal/bootstrap/node.js

@@ -158,13 +158,6 @@ function startup() {
     setupProcessStdio(getStdout, getStdin, getStderr);
   }
 
-  if (global.__coverage__)
-    NativeModule.require('internal/process/write-coverage').setup();
-
-  if (process.env.NODE_V8_COVERAGE) {
-    NativeModule.require('internal/process/coverage').setupExitHooks();
-  }
-
   if (config.hasInspector) {
     NativeModule.require('internal/inspector_async_hook').setup();
   }
@@ -295,6 +288,45 @@ function startup() {
     }
   });
 
+  // Set up coverage exit hooks.
+  let originalReallyExit = process.reallyExit;
+  // Core coverage generation using nyc instrumented lib/ files.
+  // See `make coverage-build`. This does not affect user land.
+  // TODO(joyeecheung): this and `with_instrumentation.js` can be
+  // removed in favor of NODE_V8_COVERAGE once we switch to that
+  // in https://coverage.nodejs.org/
+  if (global.__coverage__) {
+    const {
+      writeCoverage
+    } = NativeModule.require('internal/coverage-gen/with_instrumentation');
+    process.on('exit', writeCoverage);
+    originalReallyExit = process.reallyExit = (code) => {
+      writeCoverage();
+      originalReallyExit(code);
+    };
+  }
+  // User-facing NODE_V8_COVERAGE environment variable that writes
+  // ScriptCoverage to a specified file.
+  if (process.env.NODE_V8_COVERAGE) {
+    const cwd = NativeModule.require('internal/process/execution').tryGetCwd();
+    const { resolve } = NativeModule.require('path');
+    // Resolve the coverage directory to an absolute path, and
+    // overwrite process.env so that the original path gets passed
+    // to child processes even when they switch cwd.
+    const coverageDirectory = resolve(cwd, process.env.NODE_V8_COVERAGE);
+    process.env.NODE_V8_COVERAGE = coverageDirectory;
+    const {
+      writeCoverage,
+      setCoverageDirectory
+    } = NativeModule.require('internal/coverage-gen/with_profiler');
+    setCoverageDirectory(coverageDirectory);
+    process.on('exit', writeCoverage);
+    process.reallyExit = (code) => {
+      writeCoverage();
+      originalReallyExit(code);
+    };
+  }
+
   const perf = internalBinding('performance');
   const {
     NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,

lib/internal/process/write-coverage.js renamed to lib/internal/coverage-gen/with_instrumentation.js

@@ -1,12 +1,16 @@
 'use strict';
-const path = require('path');
-const { mkdirSync, writeFileSync } = require('fs');
 
+// This file contains hooks for nyc instrumented lib/ files to collect
+// JS coverage for core.
+// See `make coverage-build`.
 function writeCoverage() {
   if (!global.__coverage__) {
     return;
   }
 
+  const path = require('path');
+  const { mkdirSync, writeFileSync } = require('fs');
+
   const dirname = path.join(path.dirname(process.execPath), '.coverage');
   const filename = `coverage-${process.pid}-${Date.now()}.json`;
   try {
@@ -27,15 +31,6 @@ function writeCoverage() {
   }
 }
 
-function setup() {
-  const reallyReallyExit = process.reallyExit;
-
-  process.reallyExit = function(code) {
-    writeCoverage();
-    reallyReallyExit(code);
-  };
-
-  process.on('exit', writeCoverage);
-}
-
-exports.setup = setup;
+module.exports = {
+  writeCoverage
+};

lib/internal/process/coverage.js renamed to lib/internal/coverage-gen/with_profiler.js

@@ -1,4 +1,8 @@
 'use strict';
+
+// Implements coverage collection exposed by the `NODE_V8_COVERAGE`
+// environment variable which can also be used in the user land.
+
 let coverageConnection = null;
 let coverageDirectory;
 
@@ -48,15 +52,7 @@ function disableAllAsyncHooks() {
   hooks_array.forEach((hook) => { hook.disable(); });
 }
 
-exports.writeCoverage = writeCoverage;
-
-function setup() {
-  const { hasInspector } = internalBinding('config');
-  if (!hasInspector) {
-    process._rawDebug('inspector not enabled');
-    return;
-  }
-
+function startCoverageCollection() {
   const { Connection } = internalBinding('inspector');
   coverageConnection = new Connection((res) => {
     if (coverageConnection._coverageCallback) {
@@ -75,27 +71,14 @@ function setup() {
       detailed: true
     }
   }));
-
-  try {
-    const { cwd } = internalBinding('process_methods');
-    const { resolve } = require('path');
-    coverageDirectory = process.env.NODE_V8_COVERAGE =
-      resolve(cwd(), process.env.NODE_V8_COVERAGE);
-  } catch (err) {
-    process._rawDebug(err.toString());
-  }
 }
 
-exports.setup = setup;
-
-function setupExitHooks() {
-  const reallyReallyExit = process.reallyExit;
-  process.reallyExit = function(code) {
-    writeCoverage();
-    reallyReallyExit(code);
-  };
-
-  process.on('exit', writeCoverage);
+function setCoverageDirectory(dir) {
+  coverageDirectory = dir;
 }
 
-exports.setupExitHooks = setupExitHooks;
+module.exports = {
+  startCoverageCollection,
+  writeCoverage,
+  setCoverageDirectory
+};

lib/internal/process/per_thread.js

@@ -155,7 +155,7 @@ function wrapProcessMethods(binding) {
   function kill(pid, sig) {
     var err;
     if (process.env.NODE_V8_COVERAGE) {
-      const { writeCoverage } = require('internal/process/coverage');
+      const { writeCoverage } = require('internal/coverage-gen/with_profiler');
       writeCoverage();
     }
 

node.gyp

@@ -98,6 +98,8 @@
       'lib/internal/console/constructor.js',
       'lib/internal/console/global.js',
       'lib/internal/console/inspector.js',
+      'lib/internal/coverage-gen/with_profiler.js',
+      'lib/internal/coverage-gen/with_instrumentation.js',
       'lib/internal/crypto/certificate.js',
       'lib/internal/crypto/cipher.js',
       'lib/internal/crypto/diffiehellman.js',
@@ -152,8 +154,6 @@
       'lib/internal/process/warning.js',
       'lib/internal/process/worker_thread_only.js',
       'lib/internal/querystring.js',
-      'lib/internal/process/write-coverage.js',
-      'lib/internal/process/coverage.js',
       'lib/internal/queue_microtask.js',
       'lib/internal/readline.js',
       'lib/internal/repl.js',