From f18b1c91b8be9c668d6f8a4b71d9f3d81628f429 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 21 Jul 2016 12:44:01 -0400 Subject: [PATCH 1/2] test: allow globals to be whitelisted This commit adds a function to test/common.js that allows additional global variables to be whitelisted in a test. PR-URL: https://github.com/nodejs/node/pull/7826 Reviewed-By: James M Snell --- test/common.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/common.js b/test/common.js index e25088a6688b..5d77dc8954a8 100644 --- a/test/common.js +++ b/test/common.js @@ -345,6 +345,11 @@ if (global.Symbol) { knownGlobals.push(Symbol); } +function allowGlobals(...whitelist) { + knownGlobals = knownGlobals.concat(whitelist); +} +exports.allowGlobals = allowGlobals; + function leakedGlobals() { var leaked = []; From 2d4a521d5868457d5d5675a824b3c3a5cfaf0985 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 21 Jul 2016 13:17:07 -0400 Subject: [PATCH 2/2] repl: don't override all internal repl defaults The createInternalRepl() module accepts an options object as an argument. However, if one is provided, it overrides all of the default options. This commit applies the options object to the defaults, only changing the values that are explicitly set. PR-URL: https://github.com/nodejs/node/pull/7826 Reviewed-By: James M Snell --- lib/internal/repl.js | 7 ++++--- test/parallel/test-repl-envvars.js | 6 +++++- test/parallel/test-repl-history-perm.js | 4 ++++ test/parallel/test-repl-persistent-history.js | 3 +++ test/parallel/test-repl-use-global.js | 5 +++-- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/internal/repl.js b/lib/internal/repl.js index dd14f42fa527..7782d6b84bb8 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -5,7 +5,8 @@ const REPL = require('repl'); const path = require('path'); const fs = require('fs'); const os = require('os'); -const debug = require('util').debuglog('repl'); +const util = require('util'); +const debug = util.debuglog('repl'); module.exports = Object.create(REPL); module.exports.createInternalRepl = createRepl; @@ -19,12 +20,12 @@ function createRepl(env, opts, cb) { cb = opts; opts = null; } - opts = opts || { + opts = util._extend({ ignoreUndefined: false, terminal: process.stdout.isTTY, useGlobal: true, breakEvalOnSigint: true - }; + }, opts); if (parseInt(env.NODE_NO_READLINE)) { opts.terminal = false; diff --git a/test/parallel/test-repl-envvars.js b/test/parallel/test-repl-envvars.js index 759b4e15a12f..b08f6cbaf621 100644 --- a/test/parallel/test-repl-envvars.js +++ b/test/parallel/test-repl-envvars.js @@ -2,7 +2,7 @@ // Flags: --expose-internals -require('../common'); +const common = require('../common'); const stream = require('stream'); const REPL = require('internal/repl'); const assert = require('assert'); @@ -46,6 +46,10 @@ function run(test) { REPL.createInternalRepl(env, opts, function(err, repl) { if (err) throw err; + + // The REPL registers 'module' and 'require' globals + common.allowGlobals(repl.context.module, repl.context.require); + assert.equal(expected.terminal, repl.terminal, 'Expected ' + inspect(expected) + ' with ' + inspect(env)); assert.equal(expected.useColors, repl.useColors, diff --git a/test/parallel/test-repl-history-perm.js b/test/parallel/test-repl-history-perm.js index c7d2852539a0..4a374cb0ab12 100644 --- a/test/parallel/test-repl-history-perm.js +++ b/test/parallel/test-repl-history-perm.js @@ -35,6 +35,10 @@ const replHistoryPath = path.join(common.tmpDir, '.node_repl_history'); const checkResults = common.mustCall(function(err, r) { if (err) throw err; + + // The REPL registers 'module' and 'require' globals + common.allowGlobals(r.context.module, r.context.require); + r.input.end(); const stat = fs.statSync(replHistoryPath); assert.strictEqual( diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js index 70d6a51c81bd..0a38cfc480c1 100644 --- a/test/parallel/test-repl-persistent-history.js +++ b/test/parallel/test-repl-persistent-history.js @@ -262,6 +262,9 @@ 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); diff --git a/test/parallel/test-repl-use-global.js b/test/parallel/test-repl-use-global.js index 86c646cc7673..79e13cd819ae 100644 --- a/test/parallel/test-repl-use-global.js +++ b/test/parallel/test-repl-use-global.js @@ -7,8 +7,6 @@ const stream = require('stream'); const repl = require('internal/repl'); const assert = require('assert'); -common.globalCheck = false; - // Array of [useGlobal, expectedResult] pairs const globalTestCases = [ [false, 'undefined'], @@ -20,6 +18,9 @@ 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';