test: remove common.globalCheck

This flag is partially used in tests where it was not necessary and
it is always possible to replace this flag with
`common.allowGlobals`. This makes sure all globals are truly tested
for.

PR-URL: https://github.com/nodejs/node/pull/20717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

test/common/README.md

@@ -150,11 +150,6 @@ Attempts to get a valid TTY file descriptor. Returns `-1` if it fails.
 
 The TTY file descriptor is assumed to be capable of being writable.
 
-### globalCheck
-* [<boolean>]
-
-Set to `false` if the test should not check for global leaks.
-
 ### hasCrypto
 * [<boolean>]
 

test/common/index.js

@@ -366,21 +366,15 @@ function leakedGlobals() {
 }
 exports.leakedGlobals = leakedGlobals;
 
-// Turn this off if the test should not check for global leaks.
-exports.globalCheck = true;
-
 process.on('exit', function() {
-  if (!exports.globalCheck) return;
   const leaked = leakedGlobals();
   if (leaked.length > 0) {
     assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
   }
 });
 
-
 const mustCallChecks = [];
 
-
 function runCallChecks(exitCode) {
   if (exitCode !== 0) return;
 

test/common/index.mjs

@@ -63,11 +63,7 @@ export function leakedGlobals() {
   }
 }
 
-// Turn this off if the test should not check for global leaks.
-export let globalCheck = true;  // eslint-disable-line
-
 process.on('exit', function() {
-  if (!globalCheck) return;
   const leaked = leakedGlobals();
   if (leaked.length > 0) {
     assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);

test/parallel/test-domain-crypto.js

@@ -29,7 +29,7 @@ if (!common.hasCrypto)
 const crypto = require('crypto');
 
 // Pollution of global is intentional as part of test.
-common.globalCheck = false;
+common.allowGlobals(require('domain'));
 // See https://github.com/nodejs/node/commit/d1eff9ab
 global.domain = require('domain');
 

test/parallel/test-global.js

@@ -28,7 +28,7 @@ const fixtures = require('../common/fixtures');
 
 const assert = require('assert');
 
-common.globalCheck = false;
+common.allowGlobals('bar', 'foo');
 
 baseFoo = 'foo'; // eslint-disable-line no-undef
 global.baseBar = 'bar';

test/parallel/test-repl-autolibs.js

@@ -25,9 +25,6 @@ const assert = require('assert');
 const util = require('util');
 const repl = require('repl');
 
-// This test adds global variables
-common.globalCheck = false;
-
 const putIn = new common.ArrayStream();
 repl.start('', putIn, null, true);
 
@@ -65,6 +62,7 @@ function test2() {
   };
   const val = {};
   global.url = val;
+  common.allowGlobals(val);
   assert(!gotWrite);
   putIn.run(['url']);
   assert(gotWrite);

test/parallel/test-repl-envvars.js

@@ -2,7 +2,7 @@
 
 // Flags: --expose-internals
 
-const common = require('../common');
+require('../common');
 const stream = require('stream');
 const REPL = require('internal/repl');
 const assert = require('assert');
@@ -47,9 +47,6 @@ function run(test) {
   REPL.createInternalRepl(env, opts, function(err, repl) {
     assert.ifError(err);
 
-    // The REPL registers 'module' and 'require' globals
-    common.allowGlobals(repl.context.module, repl.context.require);
-
     assert.strictEqual(expected.terminal, repl.terminal,
                        `Expected ${inspect(expected)} with ${inspect(env)}`);
     assert.strictEqual(expected.useColors, repl.useColors,

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

@@ -38,9 +38,6 @@ const replHistoryPath = path.join(tmpdir.path, '.node_repl_history');
 const checkResults = common.mustCall(function(err, r) {
   assert.ifError(err);
 
-  // The REPL registers 'module' and 'require' globals
-  common.allowGlobals(r.context.module, r.context.require);
-
   r.input.end();
   const stat = fs.statSync(replHistoryPath);
   const fileMode = stat.mode & 0o777;

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

@@ -216,9 +216,6 @@ function runTest(assertCleaned) {
       throw err;
     }
 
-    // The REPL registers 'module' and 'require' globals
-    common.allowGlobals(repl.context.module, repl.context.require);
-
     repl.once('close', () => {
       if (repl._flushing) {
         repl.once('flushHistory', onClose);

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

@@ -26,6 +26,8 @@ const assert = require('assert');
 const repl = require('repl');
 const util = require('util');
 
+common.allowGlobals(42);
+
 // Create a dummy stream that does nothing
 const dummy = new common.ArrayStream();
 

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

@@ -18,9 +18,6 @@ const globalTest = (useGlobal, cb, output) => (err, repl) => {
   if (err)
     return cb(err);
 
-  // The REPL registers 'module' and 'require' globals
-  common.allowGlobals(repl.context.module, repl.context.require);
-
   let str = '';
   output.on('data', (data) => (str += data));
   global.lunch = 'tacos';

test/parallel/test-repl.js

@@ -26,7 +26,6 @@ const assert = require('assert');
 const net = require('net');
 const repl = require('repl');
 
-common.globalCheck = false;
 common.crashOnUnhandledRejection();
 
 const message = 'Read, Eval, Print Loop';
@@ -41,7 +40,6 @@ global.invoke_me = function(arg) {
   return `invoked ${arg}`;
 };
 
-
 // Helpers for describing the expected output:
 const kArrow = /^ *\^+ *$/;  // Arrow of ^ pointing to syntax error location
 const kSource = Symbol('kSource');  // Placeholder standing for input readback
@@ -779,6 +777,7 @@ const tcpTests = [
 
     socket.end();
   }
+  common.allowGlobals(...Object.values(global));
 })();
 
 function startTCPRepl() {

test/parallel/test-require-deps-deprecation.js

@@ -2,8 +2,6 @@
 
 const common = require('../common');
 const assert = require('assert');
-// The v8 modules when imported leak globals. Disable global check.
-common.globalCheck = false;
 
 const deprecatedModules = [
   'node-inspect/lib/_inspect',
@@ -53,3 +51,6 @@ for (const m of deps) {
   }
   assert.notStrictEqual(path, m);
 }
+
+// The V8 modules add the WebInspector to the globals.
+common.allowGlobals(global.WebInspector);

test/parallel/test-timer-immediate.js

@@ -1,5 +1,5 @@
 'use strict';
 const common = require('../common');
-common.globalCheck = false;
 global.process = {};  // Boom!
+common.allowGlobals(global.process);
 setImmediate(common.mustCall());

test/parallel/test-vm-new-script-this-context.js

@@ -24,8 +24,6 @@ const common = require('../common');
 const assert = require('assert');
 const Script = require('vm').Script;
 
-common.globalCheck = false;
-
 // Run a string
 let script = new Script('\'passed\';');
 const result = script.runInThisContext(script);
@@ -60,3 +58,11 @@ global.f = function() { global.foo = 100; };
 script = new Script('f()');
 script.runInThisContext(script);
 assert.strictEqual(100, global.foo);
+
+common.allowGlobals(
+  global.hello,
+  global.code,
+  global.foo,
+  global.obj,
+  global.f
+);

test/parallel/test-vm-run-in-new-context.js

@@ -29,8 +29,6 @@ const vm = require('vm');
 assert.strictEqual(typeof global.gc, 'function',
                    'Run this test with --expose-gc');
 
-common.globalCheck = false;
-
 // Run a string
 const result = vm.runInNewContext('\'passed\';');
 assert.strictEqual(result, 'passed');
@@ -93,3 +91,10 @@ fn();
     return true;
   });
 }
+
+common.allowGlobals(
+  global.hello,
+  global.code,
+  global.foo,
+  global.obj
+);

test/parallel/test-vm-static-this.js

@@ -24,8 +24,6 @@ const common = require('../common');
 const assert = require('assert');
 const vm = require('vm');
 
-common.globalCheck = false;
-
 // Run a string
 const result = vm.runInThisContext('\'passed\';');
 assert.strictEqual('passed', result);
@@ -58,3 +56,10 @@ assert.strictEqual(1, global.foo);
 global.f = function() { global.foo = 100; };
 vm.runInThisContext('f()');
 assert.strictEqual(100, global.foo);
+
+common.allowGlobals(
+  global.hello,
+  global.foo,
+  global.obj,
+  global.f
+);