Revert "repl: always check for NODE_REPL_MODE environment variable"

This reverts commit b831b081c499a614c1ee3f0272c9de1783395402.

This presumably unbreaks the ASAN github action build.

Example failure:

    2020-06-25T19:59:15.1448178Z === release test-repl-envvars ===
    2020-06-25T19:59:15.1448872Z Path: parallel/test-repl-envvars
    2020-06-25T19:59:15.1449449Z --- stderr ---
    2020-06-25T19:59:15.1449835Z assert.js:103
    2020-06-25T19:59:15.1450194Z   throw new AssertionError(obj);
    2020-06-25T19:59:15.1450524Z   ^
    2020-06-25T19:59:15.1450817Z
    2020-06-25T19:59:15.1451431Z AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    2020-06-25T19:59:15.1452000Z + actual - expected
    2020-06-25T19:59:15.1452298Z
    2020-06-25T19:59:15.1452634Z   {
    2020-06-25T19:59:15.1452978Z     terminal: true,
    2020-06-25T19:59:15.1453321Z +   useColors: false
    2020-06-25T19:59:15.1453861Z -   useColors: true
    2020-06-25T19:59:15.1454225Z   }
    2020-06-25T19:59:15.1454841Z     at /home/runner/work/node/node/test/parallel/test-repl-envvars.js:55:12
    2020-06-25T19:59:15.1455246Z     at internal/repl.js:33:5

PR-URL: https://github.com/nodejs/node/pull/34058
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

8 files changed, 62 insertions, 47 deletions

diff --git a/test/parallel/test-repl-colors.js b/test/parallel/test-repl-colors.js
index 8b2394e0a4..dd1bdb1a08 100644
--- a/test/parallel/test-repl-colors.js
+++ b/test/parallel/test-repl-colors.js
@@ -20,7 +20,6 @@ inout._write = function(s, _, cb) {
 
 const repl = new REPLServer({ input: inout, output: inout, useColors: true });
 inout.isTTY = true;
-inout.hasColors = () => true;
 const repl2 = new REPLServer({ input: inout, output: inout });
 
 process.on('exit', function() {
diff --git a/test/parallel/test-repl-envvars.js b/test/parallel/test-repl-envvars.js
index acd144b5bb..4fab77b7c0 100644
--- a/test/parallel/test-repl-envvars.js
+++ b/test/parallel/test-repl-envvars.js
@@ -6,11 +6,11 @@ require('../common');
 const stream = require('stream');
 const REPL = require('internal/repl');
 const assert = require('assert');
-const debug = require('util').debuglog('test');
+const inspect = require('util').inspect;
 
 const tests = [
   {
-    env: { TERM: 'xterm-256' },
+    env: {},
     expected: { terminal: true, useColors: true }
   },
   {
@@ -30,33 +30,35 @@ const tests = [
     expected: { terminal: false, useColors: false }
   },
   {
-    env: { NODE_NO_READLINE: '0', TERM: 'xterm-256' },
+    env: { NODE_NO_READLINE: '0' },
     expected: { terminal: true, useColors: true }
   }
 ];
 
-const originalEnv = process.env;
-
 function run(test) {
   const env = test.env;
   const expected = test.expected;
 
-  process.env = { ...originalEnv, ...env };
-
   const opts = {
     terminal: true,
     input: new stream.Readable({ read() {} }),
     output: new stream.Writable({ write() {} })
   };
 
-  REPL.createInternalRepl(opts, (repl) => {
-    debug(env);
-    const { terminal, useColors } = repl;
-    assert.deepStrictEqual({ terminal, useColors }, expected);
+  Object.assign(process.env, env);
+
+  REPL.createInternalRepl(process.env, opts, function(err, repl) {
+    assert.ifError(err);
+
+    assert.strictEqual(repl.terminal, expected.terminal,
+                       `Expected ${inspect(expected)} with ${inspect(env)}`);
+    assert.strictEqual(repl.useColors, expected.useColors,
+                       `Expected ${inspect(expected)} with ${inspect(env)}`);
+    for (const key of Object.keys(env)) {
+      delete process.env[key];
+    }
     repl.close();
-    if (tests.length)
-      run(tests.shift());
   });
 }
 
-run(tests.shift());
+tests.forEach(run);
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index 9d587df7bd..9155ad722b 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -521,8 +521,6 @@ function cleanupTmpFile() {
   return true;
 }
 
-const originalEnv = process.env;
-
 function runTest() {
   const opts = tests.shift();
   if (!opts) return; // All done
@@ -537,9 +535,7 @@ function runTest() {
   const lastChunks = [];
   let i = 0;
 
-  process.env = { ...originalEnv, ...opts.env };
-
-  REPL.createInternalRepl({
+  REPL.createInternalRepl(opts.env, {
     input: new ActionStream(),
     output: new stream.Writable({
       write(chunk, _, next) {
@@ -574,7 +570,12 @@ function runTest() {
     useColors: false,
     preview: opts.preview,
     terminal: true
-  }, function(repl) {
+  }, function(err, repl) {
+    if (err) {
+      console.error(`Failed test # ${numtests - tests.length}`);
+      throw err;
+    }
+
     repl.once('close', () => {
       if (opts.clean)
         cleanupTmpFile();
diff --git a/test/parallel/test-repl-history-perm.js b/test/parallel/test-repl-history-perm.js
index 736b73206b..7297689422 100644
--- a/test/parallel/test-repl-history-perm.js
+++ b/test/parallel/test-repl-history-perm.js
@@ -35,7 +35,9 @@ const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 const replHistoryPath = path.join(tmpdir.path, '.node_repl_history');
 
-const checkResults = common.mustCall((repl) => {
+const checkResults = common.mustCall(function(err, r) {
+  assert.ifError(err);
+
   const stat = fs.statSync(replHistoryPath);
   const fileMode = stat.mode & 0o777;
   assert.strictEqual(
@@ -43,13 +45,12 @@ const checkResults = common.mustCall((repl) => {
     `REPL history file should be mode 0600 but was 0${fileMode.toString(8)}`);
 
   // Close the REPL
-  repl.input.emit('keypress', '', { ctrl: true, name: 'd' });
-  repl.input.end();
+  r.input.emit('keypress', '', { ctrl: true, name: 'd' });
+  r.input.end();
 });
 
-process.env.NODE_REPL_HISTORY = replHistoryPath;
-
 repl.createInternalRepl(
+  { NODE_REPL_HISTORY: replHistoryPath },
   {
     terminal: true,
     input: stream,
diff --git a/test/parallel/test-repl-options.js b/test/parallel/test-repl-options.js
index 6a02b67917..3f6e16ee47 100644
--- a/test/parallel/test-repl-options.js
+++ b/test/parallel/test-repl-options.js
@@ -42,7 +42,6 @@ common.expectWarning({
 
 // Create a dummy stream that does nothing
 const stream = new ArrayStream();
-stream.hasColors = () => true;
 
 // 1, mostly defaults
 const r1 = repl.start({
diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js
index 0ba78c93a4..705a7c7c23 100644
--- a/test/parallel/test-repl-persistent-history.js
+++ b/test/parallel/test-repl-persistent-history.js
@@ -191,8 +191,6 @@ fs.createReadStream(historyFixturePath)
 
 const runTestWrap = common.mustCall(runTest, numtests);
 
-const originalEnv = process.env;
-
 function runTest(assertCleaned) {
   const opts = tests.shift();
   if (!opts) return; // All done
@@ -210,6 +208,7 @@ function runTest(assertCleaned) {
     }
   }
 
+  const env = opts.env;
   const test = opts.test;
   const expected = opts.expected;
   const clean = opts.clean;
@@ -217,9 +216,7 @@ function runTest(assertCleaned) {
 
   if (before) before();
 
-  process.env = { ...originalEnv, ...opts.env };
-
-  REPL.createInternalRepl({
+  REPL.createInternalRepl(env, {
     input: new ActionStream(),
     output: new stream.Writable({
       write(chunk, _, next) {
@@ -242,7 +239,12 @@ function runTest(assertCleaned) {
     prompt,
     useColors: false,
     terminal: true
-  }, function(repl) {
+  }, function(err, repl) {
+    if (err) {
+      console.error(`Failed test # ${numtests - tests.length}`);
+      throw err;
+    }
+
     repl.once('close', () => {
       if (repl._flushing) {
         repl.once('flushHistory', onClose);
diff --git a/test/parallel/test-repl-reverse-search.js b/test/parallel/test-repl-reverse-search.js
index 75364b0ffb..5027bd7da4 100644
--- a/test/parallel/test-repl-reverse-search.js
+++ b/test/parallel/test-repl-reverse-search.js
@@ -282,9 +282,6 @@ function cleanupTmpFile() {
   return true;
 }
 
-
-const originalEnv = { ...process.env };
-
 function runTest() {
   const opts = tests.shift();
   if (!opts) return; // All done
@@ -300,9 +297,7 @@ function runTest() {
   const lastChunks = [];
   let i = 0;
 
-  process.env = { ...originalEnv, ...opts.env };
-
-  REPL.createInternalRepl({
+  REPL.createInternalRepl(opts.env, {
     input: new ActionStream(),
     output: new stream.Writable({
       write(chunk, _, next) {
@@ -335,7 +330,12 @@ function runTest() {
     prompt,
     useColors: opts.useColors || false,
     terminal: true
-  }, function(repl) {
+  }, function(err, repl) {
+    if (err) {
+      console.error(`Failed test # ${numtests - tests.length}`);
+      throw err;
+    }
+
     repl.once('close', () => {
       if (opts.clean)
         cleanupTmpFile();
diff --git a/test/parallel/test-repl-use-global.js b/test/parallel/test-repl-use-global.js
index f979b0c336..00222608de 100644
--- a/test/parallel/test-repl-use-global.js
+++ b/test/parallel/test-repl-use-global.js
@@ -14,19 +14,23 @@ const globalTestCases = [
   [undefined, 'undefined']
 ];
 
-const globalTest = (cb, output) => (repl) => {
+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(str.trim());
+  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((output) => {
+  runRepl(option, globalTest, common.mustCall((err, output) => {
+    assert.ifError(err);
     assert.strictEqual(output, expected);
   }));
 }
@@ -39,7 +43,10 @@ for (const [option, expected] of globalTestCases) {
 // suite is aware of it, causing a failure to be flagged.
 //
 const processTestCases = [false, undefined];
-const processTest = (cb, output) => (repl) => {
+const processTest = (useGlobal, cb, output) => (err, repl) => {
+  if (err)
+    return cb(err);
+
   let str = '';
   output.on('data', (data) => (str += data));
 
@@ -47,11 +54,12 @@ const processTest = (cb, output) => (repl) => {
   repl.write('let process;\n');
   repl.write('21 * 2;\n');
   repl.close();
-  cb(str.trim());
+  cb(null, str.trim());
 };
 
 for (const option of processTestCases) {
-  runRepl(option, processTest, common.mustCall((output) => {
+  runRepl(option, processTest, common.mustCall((err, output) => {
+    assert.ifError(err);
     assert.strictEqual(output, 'undefined\n42');
   }));
 }
@@ -68,5 +76,8 @@ function runRepl(useGlobal, testFunc, cb) {
     prompt: ''
   };
 
-  repl.createInternalRepl(opts, testFunc(cb, opts.output));
+  repl.createInternalRepl(
+    process.env,
+    opts,
+    testFunc(useGlobal, cb, opts.output));
 }