Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 02f1e64

Browse filesBrowse files
joyeecheungBridgeAR
joyeecheung
authored and
BridgeAR
committed
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>
1 parent f4fa04e commit 02f1e64
Copy full SHA for 02f1e64

File tree

Expand file treeCollapse file tree

6 files changed

+71
-54
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+71
-54
lines changed
Open diff view settings
Collapse file

‎lib/internal/bootstrap/loaders.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/loaders.js
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,14 @@ NativeModule.prototype.cache = function() {
357357
// Coverage must be turned on early, so that we can collect
358358
// it for Node.js' own internal libraries.
359359
if (process.env.NODE_V8_COVERAGE) {
360-
NativeModule.require('internal/process/coverage').setup();
360+
if (internalBinding('config').hasInspector) {
361+
const coverage =
362+
NativeModule.require('internal/coverage-gen/with_profiler');
363+
// Inform the profiler to start collecting coverage
364+
coverage.startCoverageCollection();
365+
} else {
366+
process._rawDebug('NODE_V8_COVERAGE cannot be used without inspector');
367+
}
361368
}
362369

363370
function internalBindingWhitelistHas(name) {
Collapse file

‎lib/internal/bootstrap/node.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/node.js
+39-7Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,6 @@ function startup() {
150150
setupProcessStdio(getStdout, getStdin, getStderr);
151151
}
152152

153-
if (global.__coverage__)
154-
NativeModule.require('internal/process/write-coverage').setup();
155-
156-
if (process.env.NODE_V8_COVERAGE) {
157-
NativeModule.require('internal/process/coverage').setupExitHooks();
158-
}
159-
160153
if (config.hasInspector) {
161154
NativeModule.require('internal/inspector_async_hook').setup();
162155
}
@@ -311,6 +304,45 @@ function startup() {
311304
}
312305
});
313306

307+
// Set up coverage exit hooks.
308+
let originalReallyExit = process.reallyExit;
309+
// Core coverage generation using nyc instrumented lib/ files.
310+
// See `make coverage-build`. This does not affect user land.
311+
// TODO(joyeecheung): this and `with_instrumentation.js` can be
312+
// removed in favor of NODE_V8_COVERAGE once we switch to that
313+
// in https://coverage.nodejs.org/
314+
if (global.__coverage__) {
315+
const {
316+
writeCoverage
317+
} = NativeModule.require('internal/coverage-gen/with_instrumentation');
318+
process.on('exit', writeCoverage);
319+
originalReallyExit = process.reallyExit = (code) => {
320+
writeCoverage();
321+
originalReallyExit(code);
322+
};
323+
}
324+
// User-facing NODE_V8_COVERAGE environment variable that writes
325+
// ScriptCoverage to a specified file.
326+
if (process.env.NODE_V8_COVERAGE) {
327+
const cwd = NativeModule.require('internal/process/execution').tryGetCwd();
328+
const { resolve } = NativeModule.require('path');
329+
// Resolve the coverage directory to an absolute path, and
330+
// overwrite process.env so that the original path gets passed
331+
// to child processes even when they switch cwd.
332+
const coverageDirectory = resolve(cwd, process.env.NODE_V8_COVERAGE);
333+
process.env.NODE_V8_COVERAGE = coverageDirectory;
334+
const {
335+
writeCoverage,
336+
setCoverageDirectory
337+
} = NativeModule.require('internal/coverage-gen/with_profiler');
338+
setCoverageDirectory(coverageDirectory);
339+
process.on('exit', writeCoverage);
340+
process.reallyExit = (code) => {
341+
writeCoverage();
342+
originalReallyExit(code);
343+
};
344+
}
345+
314346
const perf = internalBinding('performance');
315347
const {
316348
NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,
Collapse file

‎lib/internal/process/write-coverage.js‎ ‎…nal/coverage-gen/with_instrumentation.js‎lib/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

Copy file name to clipboard
+9-14Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
'use strict';
2-
const path = require('path');
3-
const { mkdirSync, writeFileSync } = require('fs');
42

3+
// This file contains hooks for nyc instrumented lib/ files to collect
4+
// JS coverage for core.
5+
// See `make coverage-build`.
56
function writeCoverage() {
67
if (!global.__coverage__) {
78
return;
89
}
910

11+
const path = require('path');
12+
const { mkdirSync, writeFileSync } = require('fs');
13+
1014
const dirname = path.join(path.dirname(process.execPath), '.coverage');
1115
const filename = `coverage-${process.pid}-${Date.now()}.json`;
1216
try {
@@ -27,15 +31,6 @@ function writeCoverage() {
2731
}
2832
}
2933

30-
function setup() {
31-
const reallyReallyExit = process.reallyExit;
32-
33-
process.reallyExit = function(code) {
34-
writeCoverage();
35-
reallyReallyExit(code);
36-
};
37-
38-
process.on('exit', writeCoverage);
39-
}
40-
41-
exports.setup = setup;
34+
module.exports = {
35+
writeCoverage
36+
};
Collapse file

‎lib/internal/process/coverage.js‎ ‎…b/internal/coverage-gen/with_profiler.js‎lib/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

Copy file name to clipboardExpand all lines: lib/internal/coverage-gen/with_profiler.js
+12-29Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
'use strict';
2+
3+
// Implements coverage collection exposed by the `NODE_V8_COVERAGE`
4+
// environment variable which can also be used in the user land.
5+
26
let coverageConnection = null;
37
let coverageDirectory;
48

@@ -48,15 +52,7 @@ function disableAllAsyncHooks() {
4852
hooks_array.forEach((hook) => { hook.disable(); });
4953
}
5054

51-
exports.writeCoverage = writeCoverage;
52-
53-
function setup() {
54-
const { hasInspector } = internalBinding('config');
55-
if (!hasInspector) {
56-
process._rawDebug('inspector not enabled');
57-
return;
58-
}
59-
55+
function startCoverageCollection() {
6056
const { Connection } = internalBinding('inspector');
6157
coverageConnection = new Connection((res) => {
6258
if (coverageConnection._coverageCallback) {
@@ -75,27 +71,14 @@ function setup() {
7571
detailed: true
7672
}
7773
}));
78-
79-
try {
80-
const { cwd } = internalBinding('process_methods');
81-
const { resolve } = require('path');
82-
coverageDirectory = process.env.NODE_V8_COVERAGE =
83-
resolve(cwd(), process.env.NODE_V8_COVERAGE);
84-
} catch (err) {
85-
process._rawDebug(err.toString());
86-
}
8774
}
8875

89-
exports.setup = setup;
90-
91-
function setupExitHooks() {
92-
const reallyReallyExit = process.reallyExit;
93-
process.reallyExit = function(code) {
94-
writeCoverage();
95-
reallyReallyExit(code);
96-
};
97-
98-
process.on('exit', writeCoverage);
76+
function setCoverageDirectory(dir) {
77+
coverageDirectory = dir;
9978
}
10079

101-
exports.setupExitHooks = setupExitHooks;
80+
module.exports = {
81+
startCoverageCollection,
82+
writeCoverage,
83+
setCoverageDirectory
84+
};
Collapse file

‎lib/internal/process/per_thread.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/per_thread.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ function wrapProcessMethods(binding) {
155155
function kill(pid, sig) {
156156
var err;
157157
if (process.env.NODE_V8_COVERAGE) {
158-
const { writeCoverage } = require('internal/process/coverage');
158+
const { writeCoverage } = require('internal/coverage-gen/with_profiler');
159159
writeCoverage();
160160
}
161161

Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@
9999
'lib/internal/console/constructor.js',
100100
'lib/internal/console/global.js',
101101
'lib/internal/console/inspector.js',
102+
'lib/internal/coverage-gen/with_profiler.js',
103+
'lib/internal/coverage-gen/with_instrumentation.js',
102104
'lib/internal/crypto/certificate.js',
103105
'lib/internal/crypto/cipher.js',
104106
'lib/internal/crypto/diffiehellman.js',
@@ -153,8 +155,6 @@
153155
'lib/internal/process/warning.js',
154156
'lib/internal/process/worker_thread_only.js',
155157
'lib/internal/querystring.js',
156-
'lib/internal/process/write-coverage.js',
157-
'lib/internal/process/coverage.js',
158158
'lib/internal/queue_microtask.js',
159159
'lib/internal/readline.js',
160160
'lib/internal/repl.js',

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.