diff --git a/.gitignore b/.gitignore index bc986b3c4c0659..85168ea6cefcd9 100644 --- a/.gitignore +++ b/.gitignore @@ -102,6 +102,8 @@ _UpgradeReport_Files/ tools/*/*.i tools/*/*.i.tmp +test/common/knownGlobals.json + # === Rules for release artifacts === /*.tar.* /*.pkg diff --git a/Makefile b/Makefile index 8b1b1f16014cd9..dc19a7d4d142b8 100644 --- a/Makefile +++ b/Makefile @@ -189,6 +189,7 @@ testclean: # Next one is legacy remove this at some point $(RM) -r test/tmp* $(RM) -r test/.tmp* + $(RM) -d test/common/knownGlobals.json .PHONY: distclean .NOTPARALLEL: distclean @@ -280,8 +281,11 @@ v8: export PATH="$(NO_BIN_OVERRIDE_PATH)" && \ tools/make-v8.sh $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS) +test/common/knownGlobals.json: test/common/parseEslintConfigForKnownGlobals.js lib/.eslintrc.yaml + $(NODE) $< > $@ + .PHONY: jstest -jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests +jstest: build-addons build-js-native-api-tests build-node-api-tests test/common/knownGlobals.json ## Runs addon tests and JS tests $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \ $(TEST_CI_ARGS) \ --skip-tests=$(CI_SKIP_TESTS) \ diff --git a/test/common/index.js b/test/common/index.js index b69d726e8ef323..c5aaa43a55dc1d 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -37,11 +37,6 @@ const bits = ['arm64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x', 'x64'] .includes(process.arch) ? 64 : 32; const hasIntl = !!process.config.variables.v8_enable_i18n_support; -const { - atob, - btoa -} = require('buffer'); - // Some tests assume a umask of 0o022 so set that up front. Tests that need a // different umask will set it themselves. // @@ -257,61 +252,16 @@ function platformTimeout(ms) { return ms; // ARMv8+ } -let knownGlobals = [ - atob, - btoa, - clearImmediate, - clearInterval, - clearTimeout, - global, - setImmediate, - setInterval, - setTimeout, - queueMicrotask, -]; - -// TODO(@jasnell): This check can be temporary. AbortController is -// not currently supported in either Node.js 12 or 10, making it -// difficult to run tests comparatively on those versions. Once -// all supported versions have AbortController as a global, this -// check can be removed and AbortController can be added to the -// knownGlobals list above. -if (global.AbortController) - knownGlobals.push(global.AbortController); - -if (global.gc) { - knownGlobals.push(global.gc); -} - -if (global.performance) { - knownGlobals.push(global.performance); -} -if (global.PerformanceMark) { - knownGlobals.push(global.PerformanceMark); -} -if (global.PerformanceMeasure) { - knownGlobals.push(global.PerformanceMeasure); -} - -// TODO(@ethan-arrowood): Similar to previous checks, this can be temporary -// until v16.x is EOL. Once all supported versions have structuredClone we -// can add this to the list above instead. -if (global.structuredClone) { - knownGlobals.push(global.structuredClone); -} - -if (global.fetch) { - knownGlobals.push(fetch); -} -if (hasCrypto && global.crypto) { - knownGlobals.push(global.crypto); - knownGlobals.push(global.Crypto); - knownGlobals.push(global.CryptoKey); - knownGlobals.push(global.SubtleCrypto); +let knownGlobals; +try { + knownGlobals = require('./knownGlobals.json'); +} catch (err) { + console.info('You may need to run `make test/common/knownGlobals.json`.'); + throw err; } function allowGlobals(...allowlist) { - knownGlobals = knownGlobals.concat(allowlist); + knownGlobals.push(...allowlist); } if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { @@ -323,9 +273,10 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { function leakedGlobals() { const leaked = []; - for (const val in global) { - if (!knownGlobals.includes(global[val])) { - leaked.push(val); + const globals = Object.getOwnPropertyDescriptors(global); + for (const val in globals) { + if (globals[val].configurable && !knownGlobals.includes(val) && !knownGlobals.includes(global[val])) { + leaked.push(val.toString()); } } @@ -335,7 +286,7 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { process.on('exit', function() { const leaked = leakedGlobals(); if (leaked.length > 0) { - assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`); + assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}. Add it to lib/.eslint.yaml or call common.allowGlobals().`); } }); } diff --git a/test/common/parseEslintConfigForKnownGlobals.js b/test/common/parseEslintConfigForKnownGlobals.js new file mode 100755 index 00000000000000..4aa255e0d2634e --- /dev/null +++ b/test/common/parseEslintConfigForKnownGlobals.js @@ -0,0 +1,42 @@ +#!/usr/bin/env node +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const readline = require('readline'); + +const searchLines = [ + ' no-restricted-globals:', + ' node-core/prefer-primordials:', +]; + +const restrictedGlobalLine = /^\s{4}- name:\s?([^#\s]+)/; +const closingLine = /^\s{0,3}[^#\s]/; + +process.stdout.write('["process"'); + +const eslintConfig = readline.createInterface({ + input: fs.createReadStream( + path.join(__dirname, '..', '..', 'lib', '.eslintrc.yaml') + ), + crlfDelay: Infinity, +}); + +let isReadingGlobals = false; +eslintConfig.on('line', (line) => { + if (isReadingGlobals) { + const match = restrictedGlobalLine.exec(line); + if (match != null) { + process.stdout.write(',' + JSON.stringify(match[1])); + } else if (closingLine.test(line)) { + isReadingGlobals = false; + } + } else if (line === searchLines[0]) { + searchLines.shift(); + isReadingGlobals = true; + } +}); + +eslintConfig.once('close', () => { + process.stdout.write(']'); +}); diff --git a/test/parallel/test-repl-autocomplete.js b/test/parallel/test-repl-autocomplete.js index b107053183080a..25a744b1543995 100644 --- a/test/parallel/test-repl-autocomplete.js +++ b/test/parallel/test-repl-autocomplete.js @@ -15,6 +15,8 @@ common.skipIfDumbTerminal(); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + process.throwDeprecation = true; const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history'); diff --git a/test/parallel/test-repl-autolibs.js b/test/parallel/test-repl-autolibs.js index 5cf3b1497221d0..ff118e4da511cc 100644 --- a/test/parallel/test-repl-autolibs.js +++ b/test/parallel/test-repl-autolibs.js @@ -26,6 +26,8 @@ const assert = require('assert'); const util = require('util'); const repl = require('repl'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + const putIn = new ArrayStream(); repl.start('', putIn, null, true); diff --git a/test/parallel/test-repl-envvars.js b/test/parallel/test-repl-envvars.js index b9216bc4aa0303..5c69828e830d33 100644 --- a/test/parallel/test-repl-envvars.js +++ b/test/parallel/test-repl-envvars.js @@ -2,13 +2,15 @@ // Flags: --expose-internals -require('../common'); +const common = require('../common'); const stream = require('stream'); const REPL = require('internal/repl'); const assert = require('assert'); const inspect = require('util').inspect; const { REPL_MODE_SLOPPY, REPL_MODE_STRICT } = require('repl'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + const tests = [ { env: {}, diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 527cf235bddd21..6540f448ba38d7 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -18,6 +18,8 @@ tmpdir.refresh(); process.throwDeprecation = true; process.on('warning', common.mustNotCall()); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history'); // Create an input stream specialized for testing an array of actions diff --git a/test/parallel/test-repl-history-perm.js b/test/parallel/test-repl-history-perm.js index aeca832d430978..a105ddbebd7c22 100644 --- a/test/parallel/test-repl-history-perm.js +++ b/test/parallel/test-repl-history-perm.js @@ -20,6 +20,8 @@ const Duplex = require('stream').Duplex; // Invoking the REPL should create a repl history file at the specified path // and mode 600. +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + const stream = new Duplex(); stream.pause = stream.resume = () => {}; // ends immediately diff --git a/test/parallel/test-repl-let-process.js b/test/parallel/test-repl-let-process.js index d0524953d74650..a16c11482458b9 100644 --- a/test/parallel/test-repl-let-process.js +++ b/test/parallel/test-repl-let-process.js @@ -1,8 +1,10 @@ 'use strict'; -require('../common'); +const common = require('../common'); const ArrayStream = require('../common/arraystream'); const repl = require('repl'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + // Regression test for https://github.com/nodejs/node/issues/6802 const input = new ArrayStream(); repl.start({ input, output: process.stdout, useGlobal: true }); diff --git a/test/parallel/test-repl-options.js b/test/parallel/test-repl-options.js index 953255319cf9eb..d2dfa4010e3b2d 100644 --- a/test/parallel/test-repl-options.js +++ b/test/parallel/test-repl-options.js @@ -28,6 +28,8 @@ const assert = require('assert'); const repl = require('repl'); const cp = require('child_process'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + assert.strictEqual(repl.repl, undefined); repl._builtinLibs; // eslint-disable-line no-unused-expressions diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js index b0cddf0a2bd020..2a24fa8d5c2b1a 100644 --- a/test/parallel/test-repl-persistent-history.js +++ b/test/parallel/test-repl-persistent-history.js @@ -17,6 +17,8 @@ common.skipIfDumbTerminal(); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + // Mock os.homedir() os.homedir = function() { return tmpdir.path; diff --git a/test/parallel/test-repl-reset-event.js b/test/parallel/test-repl-reset-event.js index 1f1347547e95f8..69844b14d7452e 100644 --- a/test/parallel/test-repl-reset-event.js +++ b/test/parallel/test-repl-reset-event.js @@ -26,7 +26,7 @@ const assert = require('assert'); const repl = require('repl'); const util = require('util'); -common.allowGlobals(42); +common.allowGlobals(42, 'require', '_', '_error', ...require('module').builtinModules); // Create a dummy stream that does nothing const dummy = new ArrayStream(); diff --git a/test/parallel/test-repl-reverse-search.js b/test/parallel/test-repl-reverse-search.js index 5165dc2820d2d6..68747e5d5511db 100644 --- a/test/parallel/test-repl-reverse-search.js +++ b/test/parallel/test-repl-reverse-search.js @@ -16,6 +16,8 @@ common.allowGlobals('aaaa'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history'); // Create an input stream specialized for testing an array of actions diff --git a/test/parallel/test-repl-underscore.js b/test/parallel/test-repl-underscore.js index 3abd01ba9d0cbc..130dbba9ad241e 100644 --- a/test/parallel/test-repl-underscore.js +++ b/test/parallel/test-repl-underscore.js @@ -1,10 +1,12 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const repl = require('repl'); const stream = require('stream'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + testSloppyMode(); testStrictMode(); testResetContext(); diff --git a/test/parallel/test-repl-use-global.js b/test/parallel/test-repl-use-global.js index 3457d0c5ba7210..af2373b1aa06e9 100644 --- a/test/parallel/test-repl-use-global.js +++ b/test/parallel/test-repl-use-global.js @@ -7,6 +7,8 @@ const stream = require('stream'); const repl = require('internal/repl'); const assert = require('assert'); +common.allowGlobals('require', '_', '_error', ...require('module').builtinModules); + // Array of [useGlobal, expectedResult] pairs const globalTestCases = [ [false, 'undefined'], diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index b70fce93ba7c49..7c74b7a39ebe4c 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -39,6 +39,7 @@ const moduleFilename = fixtures.path('a'); global.invoke_me = function(arg) { return `invoked ${arg}`; }; +common.allowGlobals('invoke_me', 'require', 'a', '_', '_error', 'message', ...require('module').builtinModules); // Helpers for describing the expected output: const kArrow = /^ *\^+ *$/; // Arrow of ^ pointing to syntax error location diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index ebbbcbb6c937ef..4e6dc01d80deba 100644 --- a/test/sequential/test-module-loading.js +++ b/test/sequential/test-module-loading.js @@ -278,6 +278,7 @@ assert.throws( assert.deepStrictEqual(children, { 'common/index.js': { + 'common/knownGlobals.json': {}, 'common/tmpdir.js': {} }, 'fixtures/not-main-module.js': {},