From fbb89f292989784b83b2264515b810dd94b80bcb Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 22 Feb 2022 12:39:46 +0100 Subject: [PATCH 01/15] test: compute list of expected globals from ESLint config file --- .gitignore | 2 + Makefile | 6 +- test/common/index.js | 73 +++---------------- .../parseEslintConfigForKnownGlobals.js | 42 +++++++++++ test/parallel/test-repl-autocomplete.js | 2 + test/parallel/test-repl-autolibs.js | 2 + test/parallel/test-repl-envvars.js | 4 +- test/parallel/test-repl-history-navigation.js | 2 + test/parallel/test-repl-history-perm.js | 2 + test/parallel/test-repl-let-process.js | 4 +- test/parallel/test-repl-options.js | 2 + test/parallel/test-repl-persistent-history.js | 2 + test/parallel/test-repl-reset-event.js | 2 +- test/parallel/test-repl-reverse-search.js | 2 + test/parallel/test-repl-underscore.js | 4 +- test/parallel/test-repl-use-global.js | 2 + test/parallel/test-repl.js | 1 + test/sequential/test-module-loading.js | 1 + 18 files changed, 89 insertions(+), 66 deletions(-) create mode 100755 test/common/parseEslintConfigForKnownGlobals.js 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 4aace77c7c8c63..f56b4b4fb15956 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 29cb7816f0feb0..37d4d7415eb635 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': {}, From 9a5ec5f2aefe6cde71f4da01087dde1c61270c56 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 12:44:01 +0100 Subject: [PATCH 02/15] write json file from python --- test/common/parseEslintConfigForKnownGlobals.js | 9 ++++++--- tools/test.py | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/common/parseEslintConfigForKnownGlobals.js b/test/common/parseEslintConfigForKnownGlobals.js index 4aa255e0d2634e..074cdb7bba6275 100755 --- a/test/common/parseEslintConfigForKnownGlobals.js +++ b/test/common/parseEslintConfigForKnownGlobals.js @@ -13,7 +13,9 @@ const searchLines = [ const restrictedGlobalLine = /^\s{4}- name:\s?([^#\s]+)/; const closingLine = /^\s{0,3}[^#\s]/; -process.stdout.write('["process"'); +const jsonFile = fs.openSync(path.join(__dirname, 'knownGlobals.json'), 'w') + +fs.writeSync(jsonFile,'["process"'); const eslintConfig = readline.createInterface({ input: fs.createReadStream( @@ -27,7 +29,7 @@ eslintConfig.on('line', (line) => { if (isReadingGlobals) { const match = restrictedGlobalLine.exec(line); if (match != null) { - process.stdout.write(',' + JSON.stringify(match[1])); + fs.writeSync(jsonFile, ',' + JSON.stringify(match[1])); } else if (closingLine.test(line)) { isReadingGlobals = false; } @@ -38,5 +40,6 @@ eslintConfig.on('line', (line) => { }); eslintConfig.once('close', () => { - process.stdout.write(']'); + fs.writeSync(jsonFile, ']'); + fs.closeSync(jsonFile) }); diff --git a/tools/test.py b/tools/test.py index f204004c7a63f1..2b8a63a9d83fc0 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1640,6 +1640,7 @@ def Main(): print("Can't determine the arch of: '%s'" % vm) print(archEngineContext.stderr.rstrip()) continue + Execute([vm, 'test/common/parseEslintConfigForKnownGlobals.js'], context) env = { 'mode': mode, 'system': utils.GuessOS(), From 48e859d53d94fe13ab9376232c081c1beaf95352 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 17:02:16 +0100 Subject: [PATCH 03/15] fixup! write json file from python --- tools/test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/test.py b/tools/test.py index 2b8a63a9d83fc0..14b21997186a59 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1640,7 +1640,6 @@ def Main(): print("Can't determine the arch of: '%s'" % vm) print(archEngineContext.stderr.rstrip()) continue - Execute([vm, 'test/common/parseEslintConfigForKnownGlobals.js'], context) env = { 'mode': mode, 'system': utils.GuessOS(), @@ -1668,6 +1667,8 @@ def Main(): '-p', 'process.versions.openssl'], context) if has_crypto.stdout.rstrip() == 'undefined': context.node_has_crypto = False + + Execute([vm, join(workspace, "tools", "common", "parseEslintConfigForKnownGlobals.js")], context) if options.cat: visited = set() From afdec40ef0d1f0f20fe4c63d8b190efdd9da1002 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 17:17:19 +0100 Subject: [PATCH 04/15] fixup! fixup! write json file from python --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f56b4b4fb15956..9984b917d9d066 100644 --- a/Makefile +++ b/Makefile @@ -610,7 +610,7 @@ test-doc: doc-only lint-md ## Builds, lints, and verifies the docs. fi .PHONY: test-doc-ci -test-doc-ci: doc-only +test-doc-ci: doc-only test/common/knownGlobals.json $(PYTHON) tools/test.py --shell $(NODE) $(TEST_CI_ARGS) $(PARALLEL_ARGS) doctool .PHONY: test-known-issues From 5f9d376969b689e5adf500e8dbfbae5c6c71e072 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 17:22:15 +0100 Subject: [PATCH 05/15] lint fix --- test/common/parseEslintConfigForKnownGlobals.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/parseEslintConfigForKnownGlobals.js b/test/common/parseEslintConfigForKnownGlobals.js index 074cdb7bba6275..745427559bd926 100755 --- a/test/common/parseEslintConfigForKnownGlobals.js +++ b/test/common/parseEslintConfigForKnownGlobals.js @@ -13,9 +13,9 @@ const searchLines = [ const restrictedGlobalLine = /^\s{4}- name:\s?([^#\s]+)/; const closingLine = /^\s{0,3}[^#\s]/; -const jsonFile = fs.openSync(path.join(__dirname, 'knownGlobals.json'), 'w') +const jsonFile = fs.openSync(path.join(__dirname, 'knownGlobals.json'), 'w'); -fs.writeSync(jsonFile,'["process"'); +fs.writeSync(jsonFile, '["process"'); const eslintConfig = readline.createInterface({ input: fs.createReadStream( @@ -41,5 +41,5 @@ eslintConfig.on('line', (line) => { eslintConfig.once('close', () => { fs.writeSync(jsonFile, ']'); - fs.closeSync(jsonFile) + fs.closeSync(jsonFile); }); From 76b549187fd3cc0c78ea381cab1ac97258f0dbf4 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 17:23:21 +0100 Subject: [PATCH 06/15] fixup! lint fix --- tools/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/test.py b/tools/test.py index 14b21997186a59..7c3353829acc2c 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1667,7 +1667,7 @@ def Main(): '-p', 'process.versions.openssl'], context) if has_crypto.stdout.rstrip() == 'undefined': context.node_has_crypto = False - + Execute([vm, join(workspace, "tools", "common", "parseEslintConfigForKnownGlobals.js")], context) if options.cat: From f1cb9bf209f1e958ff41872cc1531028ce5a6801 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 17:40:58 +0100 Subject: [PATCH 07/15] fix for tarball --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9984b917d9d066..188b4b6a3ef47e 100644 --- a/Makefile +++ b/Makefile @@ -1140,7 +1140,7 @@ pkg-upload: pkg scp -p $(TARNAME).pkg $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).pkg ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).pkg.done" -$(TARBALL): release-only doc-only +$(TARBALL): release-only doc-only test/common/knownGlobals.json git checkout-index -a -f --prefix=$(TARNAME)/ mkdir -p $(TARNAME)/doc/api cp doc/node.1 $(TARNAME)/doc/node.1 @@ -1159,6 +1159,7 @@ $(TARBALL): release-only doc-only $(RM) -r $(TARNAME)/deps/v8/tools/run-tests.py $(RM) -r $(TARNAME)/doc/images # too big $(RM) -r $(TARNAME)/test*.tap + $(RM) -r $(TARNAME)/test/common/parseEslintConfigForKnownGlobals.js $(RM) -r $(TARNAME)/tools/cpplint.py $(RM) -r $(TARNAME)/tools/eslint-rules $(RM) -r $(TARNAME)/tools/license-builder.sh From ef7dd44a9c69b18619dc526d7e7a38ff502db6f1 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 27 Feb 2022 13:38:17 +0100 Subject: [PATCH 08/15] move to python --- Makefile | 11 ++--- .../parseEslintConfigForKnownGlobals.js | 45 ------------------ tools/test.py | 47 ++++++++++++++++++- 3 files changed, 51 insertions(+), 52 deletions(-) delete mode 100755 test/common/parseEslintConfigForKnownGlobals.js diff --git a/Makefile b/Makefile index 188b4b6a3ef47e..e6f08ce66619b2 100644 --- a/Makefile +++ b/Makefile @@ -281,11 +281,8 @@ 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 test/common/knownGlobals.json ## Runs addon tests and JS tests +jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \ $(TEST_CI_ARGS) \ --skip-tests=$(CI_SKIP_TESTS) \ @@ -610,7 +607,7 @@ test-doc: doc-only lint-md ## Builds, lints, and verifies the docs. fi .PHONY: test-doc-ci -test-doc-ci: doc-only test/common/knownGlobals.json +test-doc-ci: doc-only $(PYTHON) tools/test.py --shell $(NODE) $(TEST_CI_ARGS) $(PARALLEL_ARGS) doctool .PHONY: test-known-issues @@ -1140,6 +1137,9 @@ pkg-upload: pkg scp -p $(TARNAME).pkg $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).pkg ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).pkg.done" +test/common/knownGlobals.json: lib/.eslintrc.yaml + $(PYTHON) tools/test.py --create-knownGlobals-json + $(TARBALL): release-only doc-only test/common/knownGlobals.json git checkout-index -a -f --prefix=$(TARNAME)/ mkdir -p $(TARNAME)/doc/api @@ -1159,7 +1159,6 @@ $(TARBALL): release-only doc-only test/common/knownGlobals.json $(RM) -r $(TARNAME)/deps/v8/tools/run-tests.py $(RM) -r $(TARNAME)/doc/images # too big $(RM) -r $(TARNAME)/test*.tap - $(RM) -r $(TARNAME)/test/common/parseEslintConfigForKnownGlobals.js $(RM) -r $(TARNAME)/tools/cpplint.py $(RM) -r $(TARNAME)/tools/eslint-rules $(RM) -r $(TARNAME)/tools/license-builder.sh diff --git a/test/common/parseEslintConfigForKnownGlobals.js b/test/common/parseEslintConfigForKnownGlobals.js deleted file mode 100755 index 745427559bd926..00000000000000 --- a/test/common/parseEslintConfigForKnownGlobals.js +++ /dev/null @@ -1,45 +0,0 @@ -#!/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]/; - -const jsonFile = fs.openSync(path.join(__dirname, 'knownGlobals.json'), 'w'); - -fs.writeSync(jsonFile, '["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) { - fs.writeSync(jsonFile, ',' + JSON.stringify(match[1])); - } else if (closingLine.test(line)) { - isReadingGlobals = false; - } - } else if (line === searchLines[0]) { - searchLines.shift(); - isReadingGlobals = true; - } -}); - -eslintConfig.once('close', () => { - fs.writeSync(jsonFile, ']'); - fs.closeSync(jsonFile); -}); diff --git a/tools/test.py b/tools/test.py index 7c3353829acc2c..9f8cccd24801e2 100755 --- a/tools/test.py +++ b/tools/test.py @@ -43,6 +43,7 @@ import multiprocessing import errno import copy +import json if sys.version_info >= (3, 5): @@ -89,6 +90,43 @@ def get_module(name, path): os.umask(0o022) os.environ['NODE_OPTIONS'] = '' + +def createKnowGlobalsJSON(): + __dirname__ = dirname(__file__) + eslintConfigFile = join(__dirname__, '..', 'lib', '.eslintrc.yaml') + outputFile = join(__dirname__, '..', 'test', 'common', 'knownGlobals.json') + searchLines = [ + ' no-restricted-globals:\n', + ' node-core/prefer-primordials:\n', + ] + isReadingGlobals = False + try: + FileNotFoundError + except NameError: + # py2.X + FileNotFoundError = IOError + restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") + closingSectionLine = re.compile("^\s{0,3}[^#\s]") + try: + with open(eslintConfigFile, 'r') as eslintConfig, open(outputFile, 'w') as output: + output.write(u'["process"') + for line in eslintConfig.readlines(): + if isReadingGlobals: + match = restrictedGlobalDeclaration.match(line) + if match is not None: + output.write(u',{}'.format(json.dumps(match.group(1)))) + elif closingSectionLine.match(line) is not None: + isReadingGlobals = False + elif searchLines and line == searchLines[0]: + searchLines = searchLines[1:] + isReadingGlobals = True + output.write(u']') + except FileNotFoundError: + # If the .eslintrc.yaml file doesn't exist, we cannot create the JSON file. + # Let's ignore this exception, if the JSON file already exists we'll use it, + # otherwise, JavaScript will raise an error. + pass + # --------------------------------------------- # --- P r o g r e s s I n d i c a t o r s --- # --------------------------------------------- @@ -1395,6 +1433,9 @@ def BuildOptions(): result.add_option("--type", help="Type of build (simple, fips, coverage)", default=None) + result.add_option('--create-knownGlobals-json', + help='Generates the knownGlobal.json file. No tests will be run when using this flag.', + default=False, action='store_true', dest='create_knownGlobal_json') return result @@ -1565,6 +1606,10 @@ def Main(): if not ProcessOptions(options): parser.print_help() return 1 + + if options.create_knownGlobal_json: + createKnowGlobalsJSON() + return 0 ch = logging.StreamHandler(sys.stdout) logger.addHandler(ch) @@ -1668,7 +1713,7 @@ def Main(): if has_crypto.stdout.rstrip() == 'undefined': context.node_has_crypto = False - Execute([vm, join(workspace, "tools", "common", "parseEslintConfigForKnownGlobals.js")], context) + createKnowGlobalsJSON() if options.cat: visited = set() From 8d54abbb41ff2bb2e0c33d04305e1a0c2de80c76 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 27 Feb 2022 13:40:14 +0100 Subject: [PATCH 09/15] copy generated file in the tarball --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e6f08ce66619b2..bc11ee630f30e4 100644 --- a/Makefile +++ b/Makefile @@ -1140,11 +1140,12 @@ pkg-upload: pkg test/common/knownGlobals.json: lib/.eslintrc.yaml $(PYTHON) tools/test.py --create-knownGlobals-json -$(TARBALL): release-only doc-only test/common/knownGlobals.json +$(TARBALL): test/common/knownGlobals.json release-only doc-only git checkout-index -a -f --prefix=$(TARNAME)/ mkdir -p $(TARNAME)/doc/api cp doc/node.1 $(TARNAME)/doc/node.1 cp -r out/doc/api/* $(TARNAME)/doc/api/ + cp $< $(TARNAME)/$< $(RM) -r $(TARNAME)/.editorconfig $(RM) -r $(TARNAME)/.git* $(RM) -r $(TARNAME)/.mailmap From d85d9b83a2ad51ba3f53cee6a887a38c78dd084b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 27 Feb 2022 13:44:56 +0100 Subject: [PATCH 10/15] fix lint --- tools/test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/test.py b/tools/test.py index 9f8cccd24801e2..5a13d1e1fbf399 100755 --- a/tools/test.py +++ b/tools/test.py @@ -101,9 +101,10 @@ def createKnowGlobalsJSON(): ] isReadingGlobals = False try: - FileNotFoundError + # Python 3 + FileNotFoundError # noqa: F823 except NameError: - # py2.X + # Python 2 FileNotFoundError = IOError restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") closingSectionLine = re.compile("^\s{0,3}[^#\s]") @@ -1606,7 +1607,7 @@ def Main(): if not ProcessOptions(options): parser.print_help() return 1 - + if options.create_knownGlobal_json: createKnowGlobalsJSON() return 0 From 044600696f09ccbcf89761583b80a3a120292bc9 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 27 Feb 2022 16:01:58 +0100 Subject: [PATCH 11/15] don't swallow errors when we don't mean to --- tools/test.py | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/tools/test.py b/tools/test.py index 5a13d1e1fbf399..a7c0a2309bacf4 100755 --- a/tools/test.py +++ b/tools/test.py @@ -100,34 +100,40 @@ def createKnowGlobalsJSON(): ' node-core/prefer-primordials:\n', ] isReadingGlobals = False + restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") + closingSectionLine = re.compile("^\s{0,3}[^#\s]") + with open(eslintConfigFile, 'r') as eslintConfig, open(outputFile, 'w') as output: + output.write(u'["process"') + for line in eslintConfig.readlines(): + if isReadingGlobals: + match = restrictedGlobalDeclaration.match(line) + if match is not None: + output.write(u',{}'.format(json.dumps(match.group(1)))) + elif closingSectionLine.match(line) is not None: + isReadingGlobals = False + elif searchLines and line == searchLines[0]: + searchLines = searchLines[1:] + isReadingGlobals = True + output.write(u']') + +def createKnowGlobalsJSONIfPossible(): try: # Python 3 FileNotFoundError # noqa: F823 except NameError: # Python 2 FileNotFoundError = IOError - restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") - closingSectionLine = re.compile("^\s{0,3}[^#\s]") try: - with open(eslintConfigFile, 'r') as eslintConfig, open(outputFile, 'w') as output: - output.write(u'["process"') - for line in eslintConfig.readlines(): - if isReadingGlobals: - match = restrictedGlobalDeclaration.match(line) - if match is not None: - output.write(u',{}'.format(json.dumps(match.group(1)))) - elif closingSectionLine.match(line) is not None: - isReadingGlobals = False - elif searchLines and line == searchLines[0]: - searchLines = searchLines[1:] - isReadingGlobals = True - output.write(u']') + createKnowGlobalsJSON() except FileNotFoundError: - # If the .eslintrc.yaml file doesn't exist, we cannot create the JSON file. - # Let's ignore this exception, if the JSON file already exists we'll use it, - # otherwise, JavaScript will raise an error. + # In the tarball, the .eslintrc.yaml file doesn't exist, and we cannot + # create the JSON file. However, in the tarball the JSON file has already + # been generated, so we can ignore this error. + # If the JSON file is not present, JavaScript will raise an error, so we can + # also ignore. pass + # --------------------------------------------- # --- P r o g r e s s I n d i c a t o r s --- # --------------------------------------------- @@ -1714,7 +1720,7 @@ def Main(): if has_crypto.stdout.rstrip() == 'undefined': context.node_has_crypto = False - createKnowGlobalsJSON() + createKnowGlobalsJSONIfPossible() if options.cat: visited = set() From c11b45df22a1412c162f2fbd21dc14fa3e879f32 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 27 Feb 2022 16:03:58 +0100 Subject: [PATCH 12/15] fix for `\r` support --- tools/test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/test.py b/tools/test.py index a7c0a2309bacf4..19e7d9a7b5cc65 100755 --- a/tools/test.py +++ b/tools/test.py @@ -96,8 +96,8 @@ def createKnowGlobalsJSON(): eslintConfigFile = join(__dirname__, '..', 'lib', '.eslintrc.yaml') outputFile = join(__dirname__, '..', 'test', 'common', 'knownGlobals.json') searchLines = [ - ' no-restricted-globals:\n', - ' node-core/prefer-primordials:\n', + ' no-restricted-globals:', + ' node-core/prefer-primordials:', ] isReadingGlobals = False restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") @@ -111,7 +111,7 @@ def createKnowGlobalsJSON(): output.write(u',{}'.format(json.dumps(match.group(1)))) elif closingSectionLine.match(line) is not None: isReadingGlobals = False - elif searchLines and line == searchLines[0]: + elif searchLines and line.rstrip() == searchLines[0]: searchLines = searchLines[1:] isReadingGlobals = True output.write(u']') From 642a1ad7a7dcead9fbe141c4f63d7d0fc3d32b77 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 1 Mar 2022 01:02:12 +0100 Subject: [PATCH 13/15] fix for tarball --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index bc11ee630f30e4..4e45fa369dad6b 100644 --- a/Makefile +++ b/Makefile @@ -189,7 +189,6 @@ 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 @@ -262,7 +261,8 @@ coverage-report-js: .PHONY: cctest # Runs the C++ tests using the built `cctest` executable. -cctest: all +# knownGlobals.json is listed as order-only prerequisit to make it work from the tarball. +cctest: all | test/common/knownGlobals.json @out/$(BUILDTYPE)/$@ --gtest_filter=$(GTEST_FILTER) @out/$(BUILDTYPE)/embedtest "require('./test/embedding/test-embedding.js')" @@ -326,8 +326,8 @@ test-cov: all $(MAKE) build-addons $(MAKE) build-js-native-api-tests $(MAKE) build-node-api-tests - $(MAKE) cctest CI_SKIP_TESTS=$(COV_SKIP_TESTS) $(MAKE) jstest + $(MAKE) cctest .PHONY: test-parallel test-parallel: all From 945c83538ff2ddb7eaaa59b5c1b7fb747228c5fd Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 1 Mar 2022 01:08:04 +0100 Subject: [PATCH 14/15] create JSON file before testing addongs on Windows --- vcbuild.bat | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vcbuild.bat b/vcbuild.bat index d1a9e592551593..f2a943ee063095 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -596,6 +596,7 @@ if %errorlevel% neq 0 exit /b %errorlevel% :: building addons setlocal set npm_config_nodedir=%~dp0 +python "%~dp0tools\test.py" --create-knownGlobals-json "%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\addons" if errorlevel 1 exit /b 1 endlocal @@ -614,6 +615,7 @@ for /d %%F in (test\js-native-api\??_*) do ( :: building js-native-api setlocal set npm_config_nodedir=%~dp0 +python "%~dp0tools\test.py" --create-knownGlobals-json "%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\js-native-api" if errorlevel 1 exit /b 1 endlocal @@ -633,6 +635,7 @@ for /d %%F in (test\node-api\??_*) do ( :: building node-api setlocal set npm_config_nodedir=%~dp0 +python "%~dp0tools\test.py" --create-knownGlobals-json "%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\node-api" if errorlevel 1 exit /b 1 endlocal From 9f4470d9584cbb7c0bb1ee8c9190bf826ae663f3 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 2 Mar 2022 11:35:27 +0100 Subject: [PATCH 15/15] respect test-root option --- tools/test.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tools/test.py b/tools/test.py index 19e7d9a7b5cc65..cb86c5a30a7a37 100755 --- a/tools/test.py +++ b/tools/test.py @@ -91,17 +91,16 @@ def get_module(name, path): os.environ['NODE_OPTIONS'] = '' -def createKnowGlobalsJSON(): - __dirname__ = dirname(__file__) - eslintConfigFile = join(__dirname__, '..', 'lib', '.eslintrc.yaml') - outputFile = join(__dirname__, '..', 'test', 'common', 'knownGlobals.json') +def createKnowGlobalsJSON(workspace, test_root): + eslintConfigFile = join(workspace, 'lib', '.eslintrc.yaml') + outputFile = join(test_root, 'common', 'knownGlobals.json') searchLines = [ ' no-restricted-globals:', ' node-core/prefer-primordials:', ] isReadingGlobals = False - restrictedGlobalDeclaration = re.compile("^\s{4}- name:\s?([^#\s]+)") - closingSectionLine = re.compile("^\s{0,3}[^#\s]") + restrictedGlobalDeclaration = re.compile(r"^\s{4}- name:\s?([^#\s]+)") + closingSectionLine = re.compile(r"^\s{0,3}[^#\s]") with open(eslintConfigFile, 'r') as eslintConfig, open(outputFile, 'w') as output: output.write(u'["process"') for line in eslintConfig.readlines(): @@ -116,7 +115,7 @@ def createKnowGlobalsJSON(): isReadingGlobals = True output.write(u']') -def createKnowGlobalsJSONIfPossible(): +def createKnowGlobalsJSONIfPossible(workspace, test_root): try: # Python 3 FileNotFoundError # noqa: F823 @@ -124,7 +123,7 @@ def createKnowGlobalsJSONIfPossible(): # Python 2 FileNotFoundError = IOError try: - createKnowGlobalsJSON() + createKnowGlobalsJSON(workspace, test_root) except FileNotFoundError: # In the tarball, the .eslintrc.yaml file doesn't exist, and we cannot # create the JSON file. However, in the tarball the JSON file has already @@ -1614,10 +1613,6 @@ def Main(): parser.print_help() return 1 - if options.create_knownGlobal_json: - createKnowGlobalsJSON() - return 0 - ch = logging.StreamHandler(sys.stdout) logger.addHandler(ch) logger.setLevel(logging.INFO) @@ -1633,6 +1628,10 @@ def Main(): repositories = [TestRepository(join(test_root, name)) for name in suites] repositories += [TestRepository(a) for a in options.suite] + if options.create_knownGlobal_json: + createKnowGlobalsJSON(workspace, test_root) + return 0 + root = LiteralTestSuite(repositories, test_root) paths = ArgsToTestPaths(test_root, args, suites) @@ -1720,7 +1719,7 @@ def Main(): if has_crypto.stdout.rstrip() == 'undefined': context.node_has_crypto = False - createKnowGlobalsJSONIfPossible() + createKnowGlobalsJSONIfPossible(workspace, test_root) if options.cat: visited = set()