Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: refactor coverage setup during bootstrap #25398

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,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 is used in a build without inspector');
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
}
}

function internalBindingWhitelistHas(name) {
Expand Down
42 changes: 35 additions & 7 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -297,6 +290,41 @@ 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.
if (global.__coverage__) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly, we should soon be in a position to drop this code. CC: @mhdawson

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until that actually happens, I guess a TODO would be enough?

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.
const coverageDireoctry = resolve(cwd, process.env.NODE_V8_COVERAGE);
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
process.env.NODE_V8_COVERAGE = coverageDireoctry;
const {
writeCoverage,
setCoverageDirectory
} = NativeModule.require('internal/coverage-gen/with_profiler');
setCoverageDirectory(coverageDireoctry);
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 @@ -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',
Expand Down Expand Up @@ -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',
Expand Down