Skip to content

process: refactor coverage setup during bootstrap #25398

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.

Sign up for GitHub

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

Closed
joyeecheung wants to merge 2 commits into from
Closed

process: refactor coverage setup during bootstrap #25398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60ccdf5
process: refactor coverage setup during bootstrap
joyeecheung Dec 29, 2018
3ead96b
fixup! process: refactor coverage setup during bootstrap
joyeecheung Jan 9, 2019
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
View file
Open in desktop
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');
Comment thread
joyeecheung marked this conversation as resolved.
Outdated
}
}

function internalBindingWhitelistHas(name) {
Expand Down
42 changes: 35 additions & 7 deletions lib/internal/bootstrap/node.js
View file
Open in desktop
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
Copy Markdown
Contributor

@bcoe bcoe Jan 8, 2019

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
Copy Markdown
Member Author

@joyeecheung joyeecheung Jan 9, 2019

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);
Comment thread
joyeecheung marked this conversation as resolved.
Outdated
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
23 changes: 9 additions & 14 deletions lib/internal/process/write-coverage.js → ...rnal/coverage-gen/with_instrumentation.js
View file
Open in desktop
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
};
41 changes: 12 additions & 29 deletions lib/internal/process/coverage.js → lib/internal/coverage-gen/with_profiler.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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