repl,test: fix global check

lib/repl.js

@@ -783,7 +783,7 @@ REPLServer.prototype.createContext = function() {
     const _console = new Console(this.outputStream);
     Object.defineProperty(context, 'console', {
       configurable: true,
-      enumerable: true,
+      writable: true,
       value: _console
     });
 
@@ -803,9 +803,16 @@ REPLServer.prototype.createContext = function() {
   module.paths =
     CJSModule._resolveLookupPaths('<repl>', parentModule, true) || [];
 
-  var require = makeRequireFunction(module);
-  context.module = module;
-  context.require = require;
+  Object.defineProperty(context, 'module', {
+    configurable: true,
+    writable: true,
+    value: module
+  });
+  Object.defineProperty(context, 'require', {
+    configurable: true,
+    writable: true,
+    value: makeRequireFunction(module)
+  });
 
   addBuiltinLibsToObject(context);
 

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

@@ -310,8 +310,6 @@ let knownGlobals = [
   clearImmediate,
   clearInterval,
   clearTimeout,
-  console,
-  constructor, // Enumerable in V8 3.21.
   global,
   process,
   setImmediate,
@@ -341,29 +339,6 @@ if (global.COUNTER_NET_SERVER_CONNECTION) {
   knownGlobals.push(COUNTER_HTTP_CLIENT_RESPONSE);
 }
 
-if (global.ArrayBuffer) {
-  knownGlobals.push(ArrayBuffer);
-  knownGlobals.push(Int8Array);
-  knownGlobals.push(Uint8Array);
-  knownGlobals.push(Uint8ClampedArray);
-  knownGlobals.push(Int16Array);
-  knownGlobals.push(Uint16Array);
-  knownGlobals.push(Int32Array);
-  knownGlobals.push(Uint32Array);
-  knownGlobals.push(Float32Array);
-  knownGlobals.push(Float64Array);
-  knownGlobals.push(DataView);
-}
-
-// Harmony features.
-if (global.Proxy) {
-  knownGlobals.push(Proxy);
-}
-
-if (global.Symbol) {
-  knownGlobals.push(Symbol);
-}
-
 if (process.env.NODE_TEST_KNOWN_GLOBALS) {
   const knownFromEnv = process.env.NODE_TEST_KNOWN_GLOBALS.split(',');
   allowGlobals(...knownFromEnv);
@@ -391,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

@@ -8,8 +8,6 @@ let knownGlobals = [
   clearImmediate,
   clearInterval,
   clearTimeout,
-  console,
-  constructor, // Enumerable in V8 3.21.
   global,
   process,
   setImmediate,
@@ -50,29 +48,6 @@ export function leakedGlobals() {
     knownGlobals.push(COUNTER_HTTP_CLIENT_RESPONSE);
   }
 
-  if (global.ArrayBuffer) {
-    knownGlobals.push(ArrayBuffer);
-    knownGlobals.push(Int8Array);
-    knownGlobals.push(Uint8Array);
-    knownGlobals.push(Uint8ClampedArray);
-    knownGlobals.push(Int16Array);
-    knownGlobals.push(Uint16Array);
-    knownGlobals.push(Int32Array);
-    knownGlobals.push(Uint32Array);
-    knownGlobals.push(Float32Array);
-    knownGlobals.push(Float64Array);
-    knownGlobals.push(DataView);
-  }
-
-  // Harmony features.
-  if (global.Proxy) {
-    knownGlobals.push(Proxy);
-  }
-
-  if (global.Symbol) {
-    knownGlobals.push(Symbol);
-  }
-
   const leaked = [];
 
   for (const val in global) {
@@ -88,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/message/console_low_stack_space.js

@@ -1,8 +1,11 @@
 'use strict';
-// copy console accessor because requiring ../common touches it
+// Copy console accessor because requiring ../common touches it
 const consoleDescriptor = Object.getOwnPropertyDescriptor(global, 'console');
-delete global.console;
-global.console = {};
+Object.defineProperty(global, 'console', {
+  configurable: true,
+  writable: true,
+  value: {}
+});
 
 require('../common');
 

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-context.js

@@ -4,32 +4,39 @@ const assert = require('assert');
 const repl = require('repl');
 const vm = require('vm');
 
-// Create a dummy stream that does nothing
+// Create a dummy stream that does nothing.
 const stream = new common.ArrayStream();
 
-// Test when useGlobal is false
-testContext(repl.start({
-  input: stream,
-  output: stream,
-  useGlobal: false
-}));
+// Test context when useGlobal is false.
+{
+  const r = repl.start({
+    input: stream,
+    output: stream,
+    useGlobal: false
+  });
 
-function testContext(repl) {
-  const context = repl.createContext();
-  // ensure that the repl context gets its own "console" instance
+  // Ensure that the repl context gets its own "console" instance.
+  assert(r.context.console);
+
+  // Ensure that the repl console instance is not the global one.
+  assert.notStrictEqual(r.context.console, console);
+
+  const context = r.createContext();
+  // Ensure that the repl context gets its own "console" instance.
   assert(context.console instanceof require('console').Console);
 
-  // ensure that the repl's global property is the context
+  // Ensure that the repl's global property is the context.
   assert.strictEqual(context.global, context);
 
-  // ensure that the repl console instance does not have a setter
-  assert.throws(() => context.console = 'foo', TypeError);
-  repl.close();
+  // Ensure that the repl console instance is writable.
+  context.console = 'foo';
+  r.close();
 }
 
-testContextSideEffects(repl.start({ input: stream, output: stream }));
+// Test for context side effects.
+{
+  const server = repl.start({ input: stream, output: stream });
 
-function testContextSideEffects(server) {
   assert.ok(!server.underscoreAssigned);
   assert.strictEqual(server.lines.length, 0);
 

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-function-definition-edge-case.js

@@ -1,12 +1,10 @@
 // Reference: https://github.com/nodejs/node/pull/7624
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const repl = require('repl');
 const stream = require('stream');
 
-common.globalCheck = false;
-
 const r = initRepl();
 
 r.input.emit('data', 'function a() { return 42; } (1)\n');

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-let-process.js

@@ -2,8 +2,6 @@
 const common = require('../common');
 const repl = require('repl');
 
-common.globalCheck = false;
-
 // Regression test for https://github.com/nodejs/node/issues/6802
 const input = new common.ArrayStream();
 repl.start({ input, output: process.stdout, useGlobal: true });

test/parallel/test-repl-mode.js

@@ -1,11 +1,9 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const Stream = require('stream');
 const repl = require('repl');
 
-common.globalCheck = false;
-
 const tests = [
   testSloppyMode,
   testStrictMode,

test/parallel/test-repl-options.js

@@ -24,8 +24,6 @@ const common = require('../common');
 const assert = require('assert');
 const repl = require('repl');
 
-common.globalCheck = false;
-
 // Create a dummy stream that does nothing
 const stream = new common.ArrayStream();
 

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);