Skip to content

Commit

Permalink
process: expose process.features.inspector
Browse files Browse the repository at this point in the history
Instead of using process.config.variables.v8_enable_inspector
to detect whether inspector is enabled in the build.
  • Loading branch information
joyeecheung committed Jan 30, 2019
1 parent 5e1d446 commit 731f799
Show file tree
Hide file tree
Showing 14 changed files with 23 additions and 21 deletions.
3 changes: 2 additions & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,13 @@ process.assert = deprecate(

// TODO(joyeecheung): this property has not been well-maintained, should we
// deprecate it in favor of a better API?
const { isDebugBuild, hasOpenSSL } = config;
const { isDebugBuild, hasOpenSSL, hasInspector } = config;
Object.defineProperty(process, 'features', {
enumerable: true,
writable: false,
configurable: false,
value: {
inspector: hasInspector,
debug: isDebugBuild,
uv: true,
ipv6: true, // TODO(bnoordhuis) ping libuv
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
let session;
function sendInspectorCommand(cb, onError) {
const { hasInspector } = internalBinding('config');
const inspector = hasInspector ? require('inspector') : undefined;
if (!hasInspector) return onError();
const inspector = require('inspector');
if (session === undefined) session = new inspector.Session();
try {
session.connect();
Expand Down
12 changes: 9 additions & 3 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ if (process.argv.length === 2 &&
hasCrypto &&
// If the binary is build without `intl` the inspect option is
// invalid. The test itself should handle this case.
(process.config.variables.v8_enable_inspector !== 0 ||
!flag.startsWith('--inspect'))) {
(process.features.inspector || !flag.startsWith('--inspect'))) {
throw new Error(`Test has to be started with the flag: '${flag}'`);
}
}
Expand Down Expand Up @@ -618,11 +617,17 @@ function expectsError(fn, settings, exact) {
}

function skipIfInspectorDisabled() {
if (process.config.variables.v8_enable_inspector === 0) {
if (!process.features.inspector) {
skip('V8 inspector is disabled');
}
}

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

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

Expand Down
2 changes: 1 addition & 1 deletion test/known_issues/test-inspector-cluster-port-clash.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const assert = require('assert');

// This following check should be replaced by common.skipIfInspectorDisabled()
// if moved out of the known_issues directory.
if (process.config.variables.v8_enable_inspector === 0) {
if (!process.features.inspector) {
// When the V8 inspector is disabled, using either --without-inspector or
// --without-ssl, this test will not fail which it is expected to do.
// The following fail will allow this test to be skipped by failing it.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cli-bad-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require('../common');
const assert = require('assert');
const spawn = require('child_process').spawnSync;

if (process.config.variables.v8_enable_inspector === 1) {
if (process.features.inspector) {
requiresArgument('--inspect-port');
requiresArgument('--inspect-port=');
requiresArgument('--debug-port');
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-cli-node-print-help.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ function startPrintHelpTest() {
}

function validateNodePrintHelp() {
const config = process.config;
const HAVE_OPENSSL = common.hasCrypto;
const NODE_HAVE_I18N_SUPPORT = common.hasIntl;
const HAVE_INSPECTOR = config.variables.v8_enable_inspector === 1;
const HAVE_INSPECTOR = process.features.inspector;

const cliHelpOptions = [
{ compileConstant: HAVE_OPENSSL,
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-module-cjs-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,5 @@ require('../common');
const assert = require('assert');
const { builtinLibs } = require('internal/modules/cjs/helpers');

const hasInspector = process.config.variables.v8_enable_inspector === 1;

const expectedLibs = hasInspector ? 34 : 33;
const expectedLibs = process.features.inspector ? 34 : 33;
assert.strictEqual(builtinLibs.length, expectedLibs);
2 changes: 1 addition & 1 deletion test/parallel/test-process-env-allowed-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ require('../common');
'r',
'--stack-trace-limit=100',
'--stack-trace-limit=-=xX_nodejs_Xx=-',
].concat(process.config.variables.v8_enable_inspector ? [
].concat(process.features.inspector ? [
'--inspect-brk',
'inspect-brk',
'--inspect_brk',
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-process-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const assert = require('assert');
const keys = new Set(Object.keys(process.features));

assert.deepStrictEqual(keys, new Set([
'inspector',
'debug',
'uv',
'ipv6',
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const {
} = require('../common/hijackstdio');
const assert = require('assert');
const fixtures = require('../common/fixtures');
const hasInspector = process.config.variables.v8_enable_inspector === 1;
const hasInspector = process.features.inspector;

if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-v8-coverage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

if (!process.config.variables.v8_enable_inspector) return;
if (!process.features.inspector) return;

const common = require('../common');
const assert = require('assert');
Expand Down
3 changes: 1 addition & 2 deletions test/sequential/test-async-wrap-getasyncid.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
testInitialized(req, 'SendWrap');
}

if (process.config.variables.v8_enable_inspector !== 0 &&
common.isMainThread) {
if (process.features.inspector && common.isMainThread) {
const binding = internalBinding('inspector');
const handle = new binding.Connection(() => {});
testInitialized(handle, 'Connection');
Expand Down
4 changes: 1 addition & 3 deletions test/sequential/test-inspector-has-inspector-false.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@

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

// This is to ensure that the sendInspectorCommand function calls the error
// function if its called with the v8_enable_inspector is disabled
common.skipIfInspectorEnabled();

process.config.variables.v8_enable_inspector = 0;
const inspector = require('internal/util/inspector');

inspector.sendInspectorCommand(
Expand Down
2 changes: 1 addition & 1 deletion tools/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,7 @@ def Main():

# We want to skip the inspector tests if node was built without the inspector.
has_inspector = Execute([vm,
'-p', 'process.config.variables.v8_enable_inspector'], context)
'-p', 'process.features.inspector'], context)
if has_inspector.stdout.rstrip() == '0':
context.v8_enable_inspector = False

Expand Down

0 comments on commit 731f799

Please sign in to comment.