From df52e03144a6d91f74b9f47758906409323896ec Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 12 Dec 2019 10:42:52 -0500 Subject: [PATCH 1/2] test: delay loading 'os' in test/common module There is a test that doesn't load the common module initially because it needs to monkey-patch the 'os' module. I think it would be a good idea to minimize the side-effects of loading common anyway, so let's defer loading 'os' unless/until it's actually needed. --- test/common/index.js | 67 ++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index 1be78720810b34..d774bffe0fadb5 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -23,17 +23,20 @@ /* eslint-disable node-core/crypto-check */ 'use strict'; const process = global.process; // Some tests tamper with the process global. -const path = require('path'); -const fs = require('fs'); + const assert = require('assert'); -const os = require('os'); const { exec, execSync, spawnSync } = require('child_process'); +const fs = require('fs'); +// Do not require 'os' until needed so that test-os-checked-fucnction can +// monkey patch it. If 'os' is required here, that test will fail. +const path = require('path'); const util = require('util'); +const { isMainThread } = require('worker_threads'); + const tmpdir = require('./tmpdir'); 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 // different umask will set it themselves. @@ -102,23 +105,12 @@ if (process.argv.length === 2 && const isWindows = process.platform === 'win32'; const isAIX = process.platform === 'aix'; -// On IBMi, process.platform and os.platform() both return 'aix', -// It is not enough to differentiate between IBMi and real AIX system. -const isIBMi = os.type() === 'OS400'; -const isLinuxPPCBE = (process.platform === 'linux') && - (process.arch === 'ppc64') && - (os.endianness() === 'BE'); const isSunOS = process.platform === 'sunos'; const isFreeBSD = process.platform === 'freebsd'; const isOpenBSD = process.platform === 'openbsd'; const isLinux = process.platform === 'linux'; const isOSX = process.platform === 'darwin'; -const enoughTestMem = os.totalmem() > 0x70000000; /* 1.75 Gb */ -const cpus = os.cpus(); -const enoughTestCpu = Array.isArray(cpus) && - (cpus.length > 1 || cpus[0].speed > 999); - const rootDir = isWindows ? 'c:\\' : '/'; const buildType = process.config.target_defaults ? @@ -198,15 +190,6 @@ const PIPE = (() => { return path.join(pipePrefix, pipeName); })(); -const hasIPv6 = (() => { - const iFaces = os.networkInterfaces(); - const re = isWindows ? /Loopback Pseudo-Interface/ : /lo/; - return Object.keys(iFaces).some((name) => { - return re.test(name) && - iFaces[name].some(({ family }) => family === 'IPv6'); - }); -})(); - /* * Check that when running a test with * `$node --abort-on-uncaught-exception $file child` @@ -742,8 +725,6 @@ module.exports = { childShouldThrowAndAbort, createZeroFilledFile, disableCrashOnUnhandledRejection, - enoughTestCpu, - enoughTestMem, expectsError, expectsInternalAssertion, expectWarning, @@ -753,14 +734,11 @@ module.exports = { getTTYfd, hasIntl, hasCrypto, - hasIPv6, hasMultiLocalhost, isAIX, isAlive, isFreeBSD, - isIBMi, isLinux, - isLinuxPPCBE, isMainThread, isOpenBSD, isOSX, @@ -784,12 +762,28 @@ module.exports = { skipIfReportDisabled, skipIfWorker, - get localhostIPv6() { return '::1'; }, + get enoughTestCPU() { + const cpus = require('os').cpus(); + return Array.isArray(cpus) && (cpus.length > 1 || cpus[0].speed > 999); + }, + + get enoughTestMeme() { + return require('os').totalmem() > 0x70000000; /* 1.75 Gb */ + }, get hasFipsCrypto() { return hasCrypto && require('crypto').getFips(); }, + get hasIPv6() { + const iFaces = require('os').networkInterfaces(); + const re = isWindows ? /Loopback Pseudo-Interface/ : /lo/; + return Object.keys(iFaces).some((name) => { + return re.test(name) && + iFaces[name].some(({ family }) => family === 'IPv6'); + }); + }, + get inFreeBSDJail() { if (inFreeBSDJail !== null) return inFreeBSDJail; @@ -802,6 +796,17 @@ module.exports = { return inFreeBSDJail; }, + // On IBMi, process.platform and os.platform() both return 'aix', + // It is not enough to differentiate between IBMi and real AIX system. + get isIBMi() { + return require('os').type() === 'OS400'; + }, + + get isLinuxPPCBE() { + return (process.platform === 'linux') && (process.arch === 'ppc64') && + (require('os').endianness() === 'BE'); + }, + get localhostIPv4() { if (localhostIPv4 !== null) return localhostIPv4; @@ -823,6 +828,8 @@ module.exports = { return localhostIPv4; }, + get localhostIPv6() { return '::1'; }, + // opensslCli defined lazily to reduce overhead of spawnSync get opensslCli() { if (opensslCli !== null) return opensslCli; From 352853edaaa94d9cf249d99d2c7179f83e2ba39b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 12 Dec 2019 10:44:28 -0500 Subject: [PATCH 2/2] test: make test-os-checked-function work without test harness Most tests in `test/parallel` work when invoked with `node` rather than `tools/test.py` but not test-os-checked-function because it doesn't load the `common` module initially, which means it won't get re-spawned with the necessary flags (in the Flags: comment, in this case --expose_internals). Now that common delays loading 'os' until it needs to load it, this test can load the common module and it will work from the command line without the test harness. Additionally, we now can remove a comment disabling a lint rule. --- test/parallel/test-os-checked-function.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-os-checked-function.js b/test/parallel/test-os-checked-function.js index 44337adb4063ae..4a6c1cd0cafb09 100644 --- a/test/parallel/test-os-checked-function.js +++ b/test/parallel/test-os-checked-function.js @@ -1,7 +1,7 @@ -/* eslint-disable node-core/require-common-first */ 'use strict'; // Flags: --expose_internals +const common = require('../common'); const { internalBinding } = require('internal/test/binding'); // Monkey patch the os binding before requiring any other modules, including @@ -12,7 +12,6 @@ internalBinding('os').getHomeDirectory = function(ctx) { ctx.message = 'baz'; }; -const common = require('../common'); const os = require('os'); common.expectsError(os.homedir, {