From 7a7b8f7e67de79aa796ae761fbbcd10647348dfb Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Mon, 14 Mar 2016 16:08:21 -0400 Subject: [PATCH] repl: Default `useGlobal` to false in CLI REPL. Documentation for REPL states that the default value of `useGlobal` is `false`. It makes no distinction between a REPL that is created programmatically, and the one a user is dropped into on the command line by executing `node` with no arguments. This change ensures that the CLI REPL uses a default value of `false`. Fixes: https://github.com/nodejs/node/issues/5659 Ref: https://github.com/nodejs/node/issues/6802 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen PR-URL: https://github.com/nodejs/node/pull/5703 --- lib/internal/repl.js | 2 +- test/parallel/test-repl-use-global.js | 85 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-repl-use-global.js diff --git a/lib/internal/repl.js b/lib/internal/repl.js index dd14f42fa5273c..d59b40eb763bdb 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -22,7 +22,7 @@ function createRepl(env, opts, cb) { opts = opts || { ignoreUndefined: false, terminal: process.stdout.isTTY, - useGlobal: true, + useGlobal: false, breakEvalOnSigint: true }; diff --git a/test/parallel/test-repl-use-global.js b/test/parallel/test-repl-use-global.js new file mode 100644 index 00000000000000..86c646cc767361 --- /dev/null +++ b/test/parallel/test-repl-use-global.js @@ -0,0 +1,85 @@ +'use strict'; + +// Flags: --expose-internals + +const common = require('../common'); +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'], + [true, '\'tacos\''], + [undefined, 'undefined'] +]; + +const globalTest = (useGlobal, cb, output) => (err, repl) => { + if (err) + return cb(err); + + let str = ''; + output.on('data', (data) => (str += data)); + global.lunch = 'tacos'; + repl.write('global.lunch;\n'); + repl.close(); + delete global.lunch; + cb(null, str.trim()); +}; + +// Test how the global object behaves in each state for useGlobal +for (const [option, expected] of globalTestCases) { + runRepl(option, globalTest, common.mustCall((err, output) => { + assert.ifError(err); + assert.strictEqual(output, expected); + })); +} + +// Test how shadowing the process object via `let` +// behaves in each useGlobal state. Note: we can't +// actually test the state when useGlobal is true, +// because the exception that's generated is caught +// (see below), but errors are printed, and the test +// suite is aware of it, causing a failure to be flagged. +// +const processTestCases = [false, undefined]; +const processTest = (useGlobal, cb, output) => (err, repl) => { + if (err) + return cb(err); + + let str = ''; + output.on('data', (data) => (str += data)); + + // if useGlobal is false, then `let process` should work + repl.write('let process;\n'); + repl.write('21 * 2;\n'); + repl.close(); + cb(null, str.trim()); +}; + +for (const option of processTestCases) { + runRepl(option, processTest, common.mustCall((err, output) => { + assert.ifError(err); + assert.strictEqual(output, 'undefined\n42'); + })); +} + +function runRepl(useGlobal, testFunc, cb) { + const inputStream = new stream.PassThrough(); + const outputStream = new stream.PassThrough(); + const opts = { + input: inputStream, + output: outputStream, + useGlobal: useGlobal, + useColors: false, + terminal: false, + prompt: '' + }; + + repl.createInternalRepl( + process.env, + opts, + testFunc(useGlobal, cb, opts.output)); +}