From 8c087d2f54ffb243e25a3face8558f1ded9c0b02 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 25 Feb 2019 22:27:55 +0100 Subject: [PATCH] test: remove usage of `process.binding()` Prefer `internalBinding` or other equivalents over `process.binding()` (except in tests checking `process.binding()` itself). PR-URL: https://github.com/nodejs/node/pull/26304 Reviewed-By: Rich Trott Reviewed-By: James M Snell Reviewed-By: Minwoo Jung Reviewed-By: Ruben Bridgewater Reviewed-By: Gus Caplan Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Colin Ihrig --- test/abort/test-zlib-invalid-internals-usage.js | 6 ++++-- test/async-hooks/test-zlib.zlib-binding.deflate.js | 2 +- test/common/index.js | 7 +++---- test/common/inspector-helper.js | 5 +++-- .../es-module-loaders/builtin-named-exports-loader.mjs | 7 +------ test/fixtures/es-module-loaders/example-loader.mjs | 7 ++----- test/fixtures/es-module-loaders/js-loader.mjs | 8 ++------ test/parallel/test-trace-events-api.js | 6 +++++- test/parallel/test-trace-events-async-hooks-dynamic.js | 5 ++++- test/parallel/test-trace-events-async-hooks-worker.js | 5 ++++- test/sequential/test-inspector-async-call-stack-abort.js | 6 ++++-- .../test-inspector-async-hook-setup-at-signal.js | 7 ++++--- 12 files changed, 37 insertions(+), 34 deletions(-) diff --git a/test/abort/test-zlib-invalid-internals-usage.js b/test/abort/test-zlib-invalid-internals-usage.js index c218a5ee956f19..717a88e83472f2 100644 --- a/test/abort/test-zlib-invalid-internals-usage.js +++ b/test/abort/test-zlib-invalid-internals-usage.js @@ -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( diff --git a/test/async-hooks/test-zlib.zlib-binding.deflate.js b/test/async-hooks/test-zlib.zlib-binding.deflate.js index 21fd0af7da5e5a..16d9b3f3ebd7d5 100644 --- a/test/async-hooks/test-zlib.zlib-binding.deflate.js +++ b/test/async-hooks/test-zlib.zlib-binding.deflate.js @@ -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); diff --git a/test/common/index.js b/test/common/index.js index efb13c3f675e1a..732d43f07aaeca 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -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 diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index 75b87e9cf91372..4531ce5fccd380 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -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(); @@ -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') {} diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs index 28ccd6ecf2076f..a944c4fd5ebc67 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -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)]; @@ -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' diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index acb4486edc1288..a7cf276d4ad4c4 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -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' diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs index 9fa6b9eed4d029..2ac959a4643032 100644 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -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' diff --git a/test/parallel/test-trace-events-api.js b/test/parallel/test-trace-events-api.js index 1953e2add96c49..40fc661879b6fc 100644 --- a/test/parallel/test-trace-events-api.js +++ b/test/parallel/test-trace-events-api.js @@ -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'); diff --git a/test/parallel/test-trace-events-async-hooks-dynamic.js b/test/parallel/test-trace-events-async-hooks-dynamic.js index 884909b2a380fb..4ad1c10bcf8859 100644 --- a/test/parallel/test-trace-events-async-hooks-dynamic.js +++ b/test/parallel/test-trace-events-async-hooks-dynamic.js @@ -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'); diff --git a/test/parallel/test-trace-events-async-hooks-worker.js b/test/parallel/test-trace-events-async-hooks-worker.js index 6d7bf91c11e6c4..94183e311a76bc 100644 --- a/test/parallel/test-trace-events-async-hooks-worker.js +++ b/test/parallel/test-trace-events-async-hooks-worker.js @@ -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'); diff --git a/test/sequential/test-inspector-async-call-stack-abort.js b/test/sequential/test-inspector-async-call-stack-abort.js index 946d6dc1920d6b..ce2b43cdf02e82 100644 --- a/test/sequential/test-inspector-async-call-stack-abort.js +++ b/test/sequential/test-inspector-async-call-stack-abort.js @@ -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, () => {}); @@ -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); diff --git a/test/sequential/test-inspector-async-hook-setup-at-signal.js b/test/sequential/test-inspector-async-hook-setup-at-signal.js index 139678a1e5a493..a52627e5012fee 100644 --- a/test/sequential/test-inspector-async-hook-setup-at-signal.js +++ b/test/sequential/test-inspector-async-hook-setup-at-signal.js @@ -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 @@ -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) { @@ -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');