Skip to content

Commit

Permalink
test: compute list of expected globals from ESLint config file
Browse files Browse the repository at this point in the history
  • Loading branch information
aduh95 committed Feb 26, 2022
1 parent 5f21a2b commit fbb89f2
Show file tree
Hide file tree
Showing 18 changed files with 89 additions and 66 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ _UpgradeReport_Files/
tools/*/*.i
tools/*/*.i.tmp

test/common/knownGlobals.json

# === Rules for release artifacts ===
/*.tar.*
/*.pkg
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) \
Expand Down
73 changes: 12 additions & 61 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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') {
Expand All @@ -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());
}
}

Expand All @@ -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().`);
}
});
}
Expand Down
42 changes: 42 additions & 0 deletions test/common/parseEslintConfigForKnownGlobals.js
Original file line number Diff line number Diff line change
@@ -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(']');
});
2 changes: 2 additions & 0 deletions test/parallel/test-repl-autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-autolibs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-repl-envvars.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-history-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-history-perm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-repl-let-process.js
Original file line number Diff line number Diff line change
@@ -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 });
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-persistent-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-repl-reset-event.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-reverse-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-repl-underscore.js
Original file line number Diff line number Diff line change
@@ -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();
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-use-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/sequential/test-module-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ assert.throws(

assert.deepStrictEqual(children, {
'common/index.js': {
'common/knownGlobals.json': {},
'common/tmpdir.js': {}
},
'fixtures/not-main-module.js': {},
Expand Down

0 comments on commit fbb89f2

Please sign in to comment.