Skip to content

Commit

Permalink
src: refactor coverage connection
Browse files Browse the repository at this point in the history
- Refactor the C++ class to be resuable for other types of profiles
- Move the try-catch block around coverage collection callback
  to be inside the callback to silence potential JSON or write
  errors.
- Use Function::Call instead of MakeCallback to call the coverage
  message callback since it does not actually need async hook
  handling. This way we no longer needs to disable the async
  hooks when writing the coverage results.
- Renames `lib/internal/coverage-gen/with_profiler.js` to
  `lib/internal/profiler.js` because it is now the only way
  to generate coverage.

PR-URL: #26513
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
  • Loading branch information
joyeecheung authored and targos committed Mar 30, 2019
1 parent 63e7cc7 commit ab70c96
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 194 deletions.
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function setupCoverageHooks(dir) {
const {
writeCoverage,
setCoverageDirectory
} = require('internal/coverage-gen/with_profiler');
} = require('internal/profiler');
setCoverageDirectory(coverageDirectory);
process.on('exit', writeCoverage);
process.reallyExit = (code) => {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ function wrapProcessMethods(binding) {
function kill(pid, sig) {
var err;
if (process.env.NODE_V8_COVERAGE) {
const { writeCoverage } = require('internal/coverage-gen/with_profiler');
const { writeCoverage } = require('internal/profiler');
writeCoverage();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,16 @@ function writeCoverage() {
}

const target = join(coverageDirectory, filename);
try {
disableAllAsyncHooks();
internalBinding('coverage').end((msg) => {
internalBinding('profiler').endCoverage((msg) => {
try {
const coverageInfo = JSON.parse(msg).result;
if (coverageInfo) {
writeFileSync(target, JSON.stringify(coverageInfo));
}
});
} catch (err) {
console.error(err);
}
}

function disableAllAsyncHooks() {
const { getHookArrays } = require('internal/async_hooks');
const [hooks_array] = getHookArrays();
hooks_array.forEach((hook) => { hook.disable(); });
} catch (err) {
console.error(err);
}
});
}

function setCoverageDirectory(dir) {
Expand Down
2 changes: 1 addition & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
'lib/internal/cluster/worker.js',
'lib/internal/console/constructor.js',
'lib/internal/console/global.js',
'lib/internal/coverage-gen/with_profiler.js',
'lib/internal/crypto/certificate.js',
'lib/internal/crypto/cipher.js',
'lib/internal/crypto/diffiehellman.js',
Expand Down Expand Up @@ -170,6 +169,7 @@
'lib/internal/process/worker_thread_only.js',
'lib/internal/process/report.js',
'lib/internal/process/task_queues.js',
'lib/internal/profiler.js',
'lib/internal/querystring.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
Expand Down
2 changes: 1 addition & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ struct CompileFnEntry {
#define DEBUG_CATEGORY_NAMES(V) \
NODE_ASYNC_PROVIDER_TYPES(V) \
V(INSPECTOR_SERVER) \
V(COVERAGE)
V(INSPECTOR_PROFILER)

enum class DebugCategory {
#define V(name) name,
Expand Down
2 changes: 1 addition & 1 deletion src/inspector/node_inspector.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
'../../src/inspector_io.cc',
'../../src/inspector_agent.h',
'../../src/inspector_io.h',
'../../src/inspector_coverage.cc',
'../../src/inspector_profiler.cc',
'../../src/inspector_js_api.cc',
'../../src/inspector_socket.cc',
'../../src/inspector_socket.h',
Expand Down
168 changes: 0 additions & 168 deletions src/inspector_coverage.cc

This file was deleted.

Loading

0 comments on commit ab70c96

Please sign in to comment.