test: compute list of expected globals from ESLint config file

We don't want to rely on mutable globals for core modules. Instead of maintaining a separate list of known globals in our test files, parse the ESLint config to ensure all globals are restricted in the `lib/` directory.
nodejs · Feb 21, 2022 · d6e4f2d · d6e4f2d
1 parent df0e665
commit d6e4f2d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 67 deletions.
diff --git a/test/common/index.js b/test/common/index.js
@@ -37,10 +37,7 @@ 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');
+const parseEslintConfigForGlobals = require('./parseEslintConfigForGlobals');
 
 // Some tests assume a umask of 0o022 so set that up front. Tests that need a
 // different umask will set it themselves.
@@ -257,67 +254,10 @@ 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(
-    global.fetch,
-    global.FormData,
-    global.Request,
-    global.Response,
-    global.Headers,
-  );
-}
-if (hasCrypto && global.crypto) {
-  knownGlobals.push(global.crypto);
-  knownGlobals.push(global.Crypto);
-  knownGlobals.push(global.CryptoKey);
-  knownGlobals.push(global.SubtleCrypto);
-}
+const knownGlobals = parseEslintConfigForGlobals();
 
 function allowGlobals(...allowlist) {
-  knownGlobals = knownGlobals.concat(allowlist);
+  knownGlobals.push(...allowlist);
 }
 
 if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
@@ -329,9 +269,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 (!knownGlobals.includes(global[val]) && globals[val].configurable) {
+        leaked.push(val.toString());
       }
     }
 
@@ -341,7 +282,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/parseEslintConfigForGlobals.js b/test/common/parseEslintConfigForGlobals.js
@@ -0,0 +1,39 @@
+'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]/;
+
+module.exports = function parseEslintConfigForGlobals() {
+  const eslintConfig = readline.createInterface({
+    input: fs.createReadStream(path.join(__dirname, '..', '..', 'lib', '.eslintrc.yaml')),
+    crlfDelay: Infinity,
+  });
+
+  const globals = [process];
+
+  let isReadingGlobals = false;
+  eslintConfig.on('line', (line) => {
+    if (isReadingGlobals) {
+      const match = restrictedGlobalLine.exec(line);
+      if (match && match[1] in global) {
+        globals.push(global[match[1]]);
+      } else if (closingLine.test(line)) {
+        isReadingGlobals = false;
+      }
+    } else if (line === searchLines[0]) {
+      searchLines.shift();
+      isReadingGlobals = true;
+    }
+  });
+
+  return globals;
+};