Skip to content

Commit

Permalink
bootstrap: add exception handling for profiler bootstrap
Browse files Browse the repository at this point in the history
Add exception handling for the case when profile is
not bootstrapped when coverage is enabled.

Fixes: #29542

PR-URL: #29552
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
Shobhit Chittora authored and BridgeAR committed Sep 25, 2019
1 parent 3878e1e commit 6b89512
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
12 changes: 10 additions & 2 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,16 @@ function setupCoverageHooks(dir) {
const { resolve } = require('path');
const coverageDirectory = resolve(cwd, dir);
const { sourceMapCacheToObject } = require('internal/source_map');
internalBinding('profiler').setCoverageDirectory(coverageDirectory);
internalBinding('profiler').setSourceMapCacheGetter(sourceMapCacheToObject);

if (process.features.inspector) {
internalBinding('profiler').setCoverageDirectory(coverageDirectory);
internalBinding('profiler').setSourceMapCacheGetter(sourceMapCacheToObject);
} else {
process.emitWarning('The inspector is disabled, ' +
'coverage could not be collected',
'Warning');
return '';
}
return coverageDirectory;
}

Expand Down
7 changes: 7 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,12 @@ function skipIfInspectorDisabled() {
}
}

function skipIfInspectorEnabled() {
if (process.features.inspector) {
skip('V8 inspector is enabled');
}
}

function skipIfReportDisabled() {
if (!process.config.variables.node_report) {
skip('Diagnostic reporting is disabled');
Expand Down Expand Up @@ -783,6 +789,7 @@ module.exports = {
skipIf32Bits,
skipIfEslintMissing,
skipIfInspectorDisabled,
skipIfInspectorEnabled,
skipIfReportDisabled,
skipIfWorker,

Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-coverage-with-inspector-disabled.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const common = require('../common');
common.skipIfInspectorEnabled();

const fixtures = require('../common/fixtures');
const assert = require('assert');
const { spawnSync } = require('child_process');
const env = { ...process.env, NODE_V8_COVERAGE: '/foo/bar' };
const childPath = fixtures.path('v8-coverage/subprocess');
const { status, stderr } = spawnSync(
process.execPath,
[childPath],
{ env }
);

const warningMessage = 'The inspector is disabled, ' +
'coverage could not be collected';

assert.strictEqual(status, 0);
assert.strictEqual(
stderr.toString().includes(`Warning: ${warningMessage}`),
true
);

0 comments on commit 6b89512

Please sign in to comment.