Skip to content

Commit

Permalink
test: remove usage of process.binding()
Browse files Browse the repository at this point in the history
Prefer `internalBinding` or other equivalents over `process.binding()`
(except in tests checking `process.binding()` itself).

PR-URL: nodejs#26304
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax authored and BridgeAR committed Mar 12, 2019
1 parent 8e94cb2 commit 8c087d2
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 34 deletions.
6 changes: 4 additions & 2 deletions test/abort/test-zlib-invalid-internals-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ const os = require('os');
const cp = require('child_process');

if (process.argv[2] === 'child') {
const { internalBinding } = require('internal/test/binding');
// This is the heart of the test.
new (process.binding('zlib').Zlib)(0).init(1, 2, 3, 4, 5);
new (internalBinding('zlib').Zlib)(0).init(1, 2, 3, 4, 5);
} else {
const child = cp.spawnSync(`${process.execPath}`, [`${__filename}`, 'child']);
const child = cp.spawnSync(
`${process.execPath}`, ['--expose-internals', `${__filename}`, 'child']);

assert.strictEqual(child.stdout.toString(), '');
assert.ok(child.stderr.includes(
Expand Down
2 changes: 1 addition & 1 deletion test/async-hooks/test-zlib.zlib-binding.deflate.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const hooks = initHooks();
hooks.enable();
const { internalBinding } = require('internal/test/binding');
const { Zlib } = internalBinding('zlib');
const constants = internalBinding('constants').zlib;
const constants = require('zlib').constants;

const handle = new Zlib(constants.DEFLATE);

Expand Down
7 changes: 3 additions & 4 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ const os = require('os');
const { exec, execSync, spawnSync } = require('child_process');
const util = require('util');
const tmpdir = require('./tmpdir');
const {
bits,
hasIntl
} = process.binding('config');
const bits = ['arm64', 'mips', 'mipsel', 'ppc64', 's390x', 'x64']
.includes(process.arch) ? 64 : 32;
const hasIntl = !!process.config.variables.v8_enable_i18n_support;
const { isMainThread } = require('worker_threads');

// Some tests assume a umask of 0o022 so set that up front. Tests that need a
Expand Down
5 changes: 3 additions & 2 deletions test/common/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ class InspectorSession {
}

class NodeInstance extends EventEmitter {
constructor(inspectorFlags = ['--inspect-brk=0'],
constructor(inspectorFlags = ['--inspect-brk=0', '--expose-internals'],
scriptContents = '',
scriptFile = _MAINSCRIPT) {
super();
Expand Down Expand Up @@ -348,7 +348,8 @@ class NodeInstance extends EventEmitter {

static async startViaSignal(scriptContents) {
const instance = new NodeInstance(
[], `${scriptContents}\nprocess._rawDebug('started');`, undefined);
['--expose-internals'],
`${scriptContents}\nprocess._rawDebug('started');`, undefined);
const msg = 'Timed out waiting for process to start';
while (await fires(instance.nextStderrString(), msg, TIMEOUT) !==
'started') {}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import module from 'module';

const builtins = new Set(
Object.keys(process.binding('natives')).filter(str =>
/^(?!(?:internal|node|v8)\/)/.test(str))
);

export function dynamicInstantiate(url) {
const builtinInstance = module._load(url.substr(5));
const builtinExports = ['default', ...Object.keys(builtinInstance)];
Expand All @@ -19,7 +14,7 @@ export function dynamicInstantiate(url) {
}

export function resolve(specifier, base, defaultResolver) {
if (builtins.has(specifier)) {
if (module.builtinModules.includes(specifier)) {
return {
url: `node:${specifier}`,
format: 'dynamic'
Expand Down
7 changes: 2 additions & 5 deletions test/fixtures/es-module-loaders/example-loader.mjs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import url from 'url';
import path from 'path';
import process from 'process';
import { builtinModules } from 'module';

const builtins = new Set(
Object.keys(process.binding('natives')).filter((str) =>
/^(?!(?:internal|node|v8)\/)/.test(str))
);
const JS_EXTENSIONS = new Set(['.js', '.mjs']);

const baseURL = new url.URL('file://');
baseURL.pathname = process.cwd() + '/';

export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) {
if (builtins.has(specifier)) {
if (builtinModules.includes(specifier)) {
return {
url: specifier,
format: 'builtin'
Expand Down
8 changes: 2 additions & 6 deletions test/fixtures/es-module-loaders/js-loader.mjs
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { URL } from 'url';

const builtins = new Set(
Object.keys(process.binding('natives')).filter(str =>
/^(?!(?:internal|node|v8)\/)/.test(str))
)
import { builtinModules } from 'module';

const baseURL = new URL('file://');
baseURL.pathname = process.cwd() + '/';

export function resolve (specifier, base = baseURL) {
if (builtins.has(specifier)) {
if (builtinModules.includes(specifier)) {
return {
url: specifier,
format: 'builtin'
Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-trace-events-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@

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

if (!process.binding('config').hasTracing)
try {
require('trace_events');
} catch {
common.skip('missing trace events');
}

common.skipIfWorker(); // https://github.com/nodejs/node/issues/22767

const assert = require('assert');
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-trace-events-async-hooks-dynamic.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
'use strict';

const common = require('../common');
if (!process.binding('config').hasTracing)
try {
require('trace_events');
} catch {
common.skip('missing trace events');
}

const assert = require('assert');
const cp = require('child_process');
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-trace-events-async-hooks-worker.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
'use strict';

const common = require('../common');
if (!process.binding('config').hasTracing)
try {
require('trace_events');
} catch {
common.skip('missing trace events');
}

const assert = require('assert');
const cp = require('child_process');
Expand Down
6 changes: 4 additions & 2 deletions test/sequential/test-inspector-async-call-stack-abort.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ if (process.argv[2] === 'child') {
common.disableCrashOnUnhandledRejection();
const { Session } = require('inspector');
const { promisify } = require('util');
const { registerAsyncHook } = process.binding('inspector');
const { internalBinding } = require('internal/test/binding');
const { registerAsyncHook } = internalBinding('inspector');
(async () => {
let enabled = 0;
registerAsyncHook(() => ++enabled, () => {});
Expand All @@ -28,7 +29,8 @@ if (process.argv[2] === 'child') {
} else {
const { spawnSync } = require('child_process');
const options = { encoding: 'utf8' };
const proc = spawnSync(process.execPath, [__filename, 'child'], options);
const proc = spawnSync(
process.execPath, ['--expose-internals', __filename, 'child'], options);
strictEqual(proc.status, 0);
strictEqual(proc.signal, null);
strictEqual(proc.stderr.includes(eyecatcher), true);
Expand Down
7 changes: 4 additions & 3 deletions test/sequential/test-inspector-async-hook-setup-at-signal.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ process._rawDebug('Waiting until a signal enables the inspector...');
let waiting = setInterval(waitUntilDebugged, 50);
function waitUntilDebugged() {
if (!process.binding('inspector').isEnabled()) return;
const { internalBinding } = require('internal/test/binding');
if (!internalBinding('inspector').isEnabled()) return;
clearInterval(waiting);
// At this point, even though the Inspector is enabled, the default async
// call stack depth is 0. We need a chance to call
Expand All @@ -36,7 +37,7 @@ function setupTimeoutWithBreak() {

async function waitForInitialSetup(session) {
console.error('[test]', 'Waiting for initial setup');
await session.waitForBreakOnLine(15, '[eval]');
await session.waitForBreakOnLine(16, '[eval]');
}

async function setupTimeoutForStackTrace(session) {
Expand All @@ -50,7 +51,7 @@ async function setupTimeoutForStackTrace(session) {

async function checkAsyncStackTrace(session) {
console.error('[test]', 'Verify basic properties of asyncStackTrace');
const paused = await session.waitForBreakOnLine(22, '[eval]');
const paused = await session.waitForBreakOnLine(23, '[eval]');
assert(paused.params.asyncStackTrace,
`${Object.keys(paused.params)} contains "asyncStackTrace" property`);
assert(paused.params.asyncStackTrace.description, 'Timeout');
Expand Down

0 comments on commit 8c087d2

Please sign in to comment.