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 <bencoe@gmail.com>

Author: joyeecheung

lib/internal/bootstrap/loaders.js

@@ -357,7 +357,14 @@ NativeModule.prototype.cache = 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){
+constcoverage=
+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');
+}
 }
 
 functioninternalBindingWhitelistHas(name){

lib/internal/bootstrap/node.js

@@ -150,13 +150,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();
 }
@@ -311,6 +304,45 @@ function startup() {
 }
 });
 
+// Set up coverage exit hooks.
+letoriginalReallyExit=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){
+constcwd=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.
+constcoverageDirectory=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);
+};
+}
+
 constperf=internalBinding('performance');
 const{
 NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,

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

@@ -1,12 +1,16 @@
 'use strict';
-constpath=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`.
 functionwriteCoverage(){
 if(!global.__coverage__){
 return;
 }
 
+constpath=require('path');
+const{ mkdirSync, writeFileSync }=require('fs');
+
 constdirname=path.join(path.dirname(process.execPath),'.coverage');
 constfilename=`coverage-${process.pid}-${Date.now()}.json`;
 try{
@@ -27,15 +31,6 @@ function writeCoverage() {
 }
 }
 
-functionsetup(){
-constreallyReallyExit=process.reallyExit;
-
-process.reallyExit=function(code){
-writeCoverage();
-reallyReallyExit(code);
-};
-
-process.on('exit',writeCoverage);
-}
-
-exports.setup=setup;
+module.exports={
+ writeCoverage
+};

lib/internal/process/coverage.js …b/internal/coverage-gen/with_profiler.jslib/internal/process/coverage.js renamed to lib/internal/coverage-gen/with_profiler.js 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.
+
 letcoverageConnection=null;
 letcoverageDirectory;
 
@@ -48,15 +52,7 @@ function disableAllAsyncHooks() {
 hooks_array.forEach((hook)=>{hook.disable();});
 }
 
-exports.writeCoverage=writeCoverage;
-
-functionsetup(){
-const{ hasInspector }=internalBinding('config');
-if(!hasInspector){
-process._rawDebug('inspector not enabled');
-return;
-}
-
+functionstartCoverageCollection(){
 const{ Connection }=internalBinding('inspector');
 coverageConnection=newConnection((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;
-
-functionsetupExitHooks(){
-constreallyReallyExit=process.reallyExit;
-process.reallyExit=function(code){
-writeCoverage();
-reallyReallyExit(code);
-};
-
-process.on('exit',writeCoverage);
+functionsetCoverageDirectory(dir){
+coverageDirectory=dir;
 }
 
-exports.setupExitHooks=setupExitHooks;
+module.exports={
+ startCoverageCollection,
+ writeCoverage,
+ setCoverageDirectory
+};

lib/internal/process/per_thread.js

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

node.gyp

@@ -99,6 +99,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',
@@ -153,8 +155,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',