Skip to content

Commit 96115e9

Browse files
committed
test: compute list of expected globals from ESLint config file
1 parent 0ab4a1c commit 96115e9

18 files changed

+89
-66
lines changed

.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ _UpgradeReport_Files/
102102
tools/*/*.i
103103
tools/*/*.i.tmp
104104

105+
test/common/knownGlobals.json
106+
105107
# === Rules for release artifacts ===
106108
/*.tar.*
107109
/*.pkg

Makefile

+5-1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ testclean:
189189
# Next one is legacy remove this at some point
190190
$(RM) -r test/tmp*
191191
$(RM) -r test/.tmp*
192+
$(RM) -d test/common/knownGlobals.json
192193

193194
.PHONY: distclean
194195
.NOTPARALLEL: distclean
@@ -280,8 +281,11 @@ v8:
280281
export PATH="$(NO_BIN_OVERRIDE_PATH)" && \
281282
tools/make-v8.sh $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS)
282283

284+
test/common/knownGlobals.json: test/common/parseEslintConfigForKnownGlobals.js lib/.eslintrc.yaml
285+
$(NODE) $< > $@
286+
283287
.PHONY: jstest
284-
jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests
288+
jstest: build-addons build-js-native-api-tests build-node-api-tests test/common/knownGlobals.json ## Runs addon tests and JS tests
285289
$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \
286290
$(TEST_CI_ARGS) \
287291
--skip-tests=$(CI_SKIP_TESTS) \

test/common/index.js

+12-61
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ const bits = ['arm64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x', 'x64']
3737
.includes(process.arch) ? 64 : 32;
3838
const hasIntl = !!process.config.variables.v8_enable_i18n_support;
3939

40-
const {
41-
atob,
42-
btoa
43-
} = require('buffer');
44-
4540
// Some tests assume a umask of 0o022 so set that up front. Tests that need a
4641
// different umask will set it themselves.
4742
//
@@ -257,61 +252,16 @@ function platformTimeout(ms) {
257252
return ms; // ARMv8+
258253
}
259254

260-
let knownGlobals = [
261-
atob,
262-
btoa,
263-
clearImmediate,
264-
clearInterval,
265-
clearTimeout,
266-
global,
267-
setImmediate,
268-
setInterval,
269-
setTimeout,
270-
queueMicrotask,
271-
];
272-
273-
// TODO(@jasnell): This check can be temporary. AbortController is
274-
// not currently supported in either Node.js 12 or 10, making it
275-
// difficult to run tests comparatively on those versions. Once
276-
// all supported versions have AbortController as a global, this
277-
// check can be removed and AbortController can be added to the
278-
// knownGlobals list above.
279-
if (global.AbortController)
280-
knownGlobals.push(global.AbortController);
281-
282-
if (global.gc) {
283-
knownGlobals.push(global.gc);
284-
}
285-
286-
if (global.performance) {
287-
knownGlobals.push(global.performance);
288-
}
289-
if (global.PerformanceMark) {
290-
knownGlobals.push(global.PerformanceMark);
291-
}
292-
if (global.PerformanceMeasure) {
293-
knownGlobals.push(global.PerformanceMeasure);
294-
}
295-
296-
// TODO(@ethan-arrowood): Similar to previous checks, this can be temporary
297-
// until v16.x is EOL. Once all supported versions have structuredClone we
298-
// can add this to the list above instead.
299-
if (global.structuredClone) {
300-
knownGlobals.push(global.structuredClone);
301-
}
302-
303-
if (global.fetch) {
304-
knownGlobals.push(fetch);
305-
}
306-
if (hasCrypto && global.crypto) {
307-
knownGlobals.push(global.crypto);
308-
knownGlobals.push(global.Crypto);
309-
knownGlobals.push(global.CryptoKey);
310-
knownGlobals.push(global.SubtleCrypto);
255+
let knownGlobals;
256+
try {
257+
knownGlobals = require('./knownGlobals.json');
258+
} catch (err) {
259+
console.info('You may need to run `make test/common/knownGlobals.json`.');
260+
throw err;
311261
}
312262

313263
function allowGlobals(...allowlist) {
314-
knownGlobals = knownGlobals.concat(allowlist);
264+
knownGlobals.push(...allowlist);
315265
}
316266

317267
if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
@@ -323,9 +273,10 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
323273
function leakedGlobals() {
324274
const leaked = [];
325275

326-
for (const val in global) {
327-
if (!knownGlobals.includes(global[val])) {
328-
leaked.push(val);
276+
const globals = Object.getOwnPropertyDescriptors(global);
277+
for (const val in globals) {
278+
if (globals[val].configurable && !knownGlobals.includes(val) && !knownGlobals.includes(global[val])) {
279+
leaked.push(val.toString());
329280
}
330281
}
331282

@@ -335,7 +286,7 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
335286
process.on('exit', function() {
336287
const leaked = leakedGlobals();
337288
if (leaked.length > 0) {
338-
assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
289+
assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}. Add it to lib/.eslint.yaml or call common.allowGlobals().`);
339290
}
340291
});
341292
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#!/usr/bin/env node
2+
'use strict';
3+
4+
const fs = require('fs');
5+
const path = require('path');
6+
const readline = require('readline');
7+
8+
const searchLines = [
9+
' no-restricted-globals:',
10+
' node-core/prefer-primordials:',
11+
];
12+
13+
const restrictedGlobalLine = /^\s{4}- name:\s?([^#\s]+)/;
14+
const closingLine = /^\s{0,3}[^#\s]/;
15+
16+
process.stdout.write('["process"');
17+
18+
const eslintConfig = readline.createInterface({
19+
input: fs.createReadStream(
20+
path.join(__dirname, '..', '..', 'lib', '.eslintrc.yaml')
21+
),
22+
crlfDelay: Infinity,
23+
});
24+
25+
let isReadingGlobals = false;
26+
eslintConfig.on('line', (line) => {
27+
if (isReadingGlobals) {
28+
const match = restrictedGlobalLine.exec(line);
29+
if (match != null) {
30+
process.stdout.write(',' + JSON.stringify(match[1]));
31+
} else if (closingLine.test(line)) {
32+
isReadingGlobals = false;
33+
}
34+
} else if (line === searchLines[0]) {
35+
searchLines.shift();
36+
isReadingGlobals = true;
37+
}
38+
});
39+
40+
eslintConfig.once('close', () => {
41+
process.stdout.write(']');
42+
});

test/parallel/test-repl-autocomplete.js

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ common.skipIfDumbTerminal();
1515
const tmpdir = require('../common/tmpdir');
1616
tmpdir.refresh();
1717

18+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
19+
1820
process.throwDeprecation = true;
1921

2022
const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history');

test/parallel/test-repl-autolibs.js

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const assert = require('assert');
2626
const util = require('util');
2727
const repl = require('repl');
2828

29+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
30+
2931
const putIn = new ArrayStream();
3032
repl.start('', putIn, null, true);
3133

test/parallel/test-repl-envvars.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22

33
// Flags: --expose-internals
44

5-
require('../common');
5+
const common = require('../common');
66
const stream = require('stream');
77
const REPL = require('internal/repl');
88
const assert = require('assert');
99
const inspect = require('util').inspect;
1010
const { REPL_MODE_SLOPPY, REPL_MODE_STRICT } = require('repl');
1111

12+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
13+
1214
const tests = [
1315
{
1416
env: {},

test/parallel/test-repl-history-navigation.js

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ tmpdir.refresh();
1818
process.throwDeprecation = true;
1919
process.on('warning', common.mustNotCall());
2020

21+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
22+
2123
const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history');
2224

2325
// Create an input stream specialized for testing an array of actions

test/parallel/test-repl-history-perm.js

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ const Duplex = require('stream').Duplex;
2020
// Invoking the REPL should create a repl history file at the specified path
2121
// and mode 600.
2222

23+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
24+
2325
const stream = new Duplex();
2426
stream.pause = stream.resume = () => {};
2527
// ends immediately

test/parallel/test-repl-let-process.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const ArrayStream = require('../common/arraystream');
44
const repl = require('repl');
55

6+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
7+
68
// Regression test for https://github.com/nodejs/node/issues/6802
79
const input = new ArrayStream();
810
repl.start({ input, output: process.stdout, useGlobal: true });

test/parallel/test-repl-options.js

+2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const assert = require('assert');
2828
const repl = require('repl');
2929
const cp = require('child_process');
3030

31+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
32+
3133
assert.strictEqual(repl.repl, undefined);
3234
repl._builtinLibs; // eslint-disable-line no-unused-expressions
3335

test/parallel/test-repl-persistent-history.js

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ common.skipIfDumbTerminal();
1717
const tmpdir = require('../common/tmpdir');
1818
tmpdir.refresh();
1919

20+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
21+
2022
// Mock os.homedir()
2123
os.homedir = function() {
2224
return tmpdir.path;

test/parallel/test-repl-reset-event.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const assert = require('assert');
2626
const repl = require('repl');
2727
const util = require('util');
2828

29-
common.allowGlobals(42);
29+
common.allowGlobals(42, 'require', '_', '_error', ...require('module').builtinModules);
3030

3131
// Create a dummy stream that does nothing
3232
const dummy = new ArrayStream();

test/parallel/test-repl-reverse-search.js

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ common.allowGlobals('aaaa');
1616
const tmpdir = require('../common/tmpdir');
1717
tmpdir.refresh();
1818

19+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
20+
1921
const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history');
2022

2123
// Create an input stream specialized for testing an array of actions

test/parallel/test-repl-underscore.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
'use strict';
22

3-
require('../common');
3+
const common = require('../common');
44
const assert = require('assert');
55
const repl = require('repl');
66
const stream = require('stream');
77

8+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
9+
810
testSloppyMode();
911
testStrictMode();
1012
testResetContext();

test/parallel/test-repl-use-global.js

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const stream = require('stream');
77
const repl = require('internal/repl');
88
const assert = require('assert');
99

10+
common.allowGlobals('require', '_', '_error', ...require('module').builtinModules);
11+
1012
// Array of [useGlobal, expectedResult] pairs
1113
const globalTestCases = [
1214
[false, 'undefined'],

test/parallel/test-repl.js

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const moduleFilename = fixtures.path('a');
3939
global.invoke_me = function(arg) {
4040
return `invoked ${arg}`;
4141
};
42+
common.allowGlobals('invoke_me', 'require', 'a', '_', '_error', 'message', ...require('module').builtinModules);
4243

4344
// Helpers for describing the expected output:
4445
const kArrow = /^ *\^+ *$/; // Arrow of ^ pointing to syntax error location

test/sequential/test-module-loading.js

+1
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ assert.throws(
278278

279279
assert.deepStrictEqual(children, {
280280
'common/index.js': {
281+
'common/knownGlobals.json': {},
281282
'common/tmpdir.js': {}
282283
},
283284
'fixtures/not-main-module.js': {},

0 commit comments

Comments
 (0)