Skip to content

Commit

Permalink
process: refactor coverage setup during bootstrap
Browse files Browse the repository at this point in the history
- 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>
  • Loading branch information
joyeecheung authored and BridgeAR committed Jan 17, 2019
1 parent 3bd8e4b commit b524a7b
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 54 deletions.
9 changes: 8 additions & 1 deletion lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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) {
Expand Down
46 changes: 39 additions & 7 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -311,6 +304,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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
};
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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) {
Expand All @@ -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
};
2 changes: 1 addition & 1 deletion lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
4 changes: 2 additions & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down

0 comments on commit b524a7b

Please sign in to comment.