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 <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
Shobhit Chittora authored and targos committed Sep 23, 2019
1 parent 8b55ccb commit 84ebbd7
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 84ebbd7

Please sign in to comment.