fix(config): consolidate use of npm.color

The config value for `color` should never be directly read by npm, or
its submodules.  The derived value `npm.color` or
`npm.flatOptions.color` is what we want to use.

This PR consolidates the use of these values, makes sure there is only
one place the value is actually calculated, and stops relying on
duplicated code in the setup-log.js file for setting one of them.

PR-URL: https://github.com/npm/cli/pull/3563
Credit: @wraithgar
Close: #3563
Reviewed-by: @lukekarrys

3 files changed, 23 insertions, 16 deletions

diff --git a/test/lib/exec.js b/test/lib/exec.js
index 6d99c7959..03a1bedf9 100644
--- a/test/lib/exec.js
+++ b/test/lib/exec.js
@@ -25,6 +25,7 @@ const LOG_WARN = []
 let PROGRESS_IGNORED = false
 const flatOptions = {
   npxCache: 'npx-cache-dir',
+  color: false,
   cache: 'cache-dir',
   legacyPeerDeps: false,
   package: [],
@@ -109,12 +110,13 @@ t.afterEach(() => {
   LOG_WARN.length = 0
   PROGRESS_IGNORED = false
   flatOptions.legacyPeerDeps = false
-  config.color = false
+  flatOptions.color = false
   config['script-shell'] = 'shell-cmd'
   config.package = []
   flatOptions.package = []
   config.call = ''
   config.yes = true
+  npm.color = false
   npm.localBin = 'local-bin'
   npm.globalBin = 'global-bin'
 })
@@ -268,7 +270,8 @@ t.test('npm exec <noargs>, run interactive shell', t => {
   t.test('print message with color when tty and not in CI', t => {
     CI_NAME = null
     process.stdin.isTTY = true
-    config.color = true
+    npm.color = true
+    flatOptions.color = true
 
     run(t, true, () => {
       t.strictSame(LOG_WARN, [])
@@ -1204,7 +1207,8 @@ t.test('workspaces', t => {
       })
     })
 
-    config.color = true
+    npm.color = true
+    flatOptions.color = true
     npm._mockOutputs.length = 0
     await new Promise((res, rej) => {
       exec.execWorkspaces([], ['a'], er => {
diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js
index 3fdfb8cc2..908d70fc3 100644
--- a/test/lib/utils/error-message.js
+++ b/test/lib/utils/error-message.js
@@ -15,6 +15,9 @@ const { resolve } = require('path')
 const npm = require('../../../lib/npm.js')
 const CACHE = '/some/cache/dir'
 npm.config = {
+  flat: {
+    color: false,
+  },
   loaded: false,
   localPrefix: '/some/prefix/dir',
   get: key => {
@@ -467,7 +470,7 @@ t.test('explain ERESOLVE errors', t => {
   t.matchSnapshot(errorMessage(er, npm))
   t.match(EXPLAIN_CALLED, [[
     er,
-    undefined,
+    false,
     path.resolve(npm.cache, 'eresolve-report.txt'),
   ]])
   t.end()
diff --git a/test/lib/utils/setup-log.js b/test/lib/utils/setup-log.js
index 86befe6e2..7f907bc7e 100644
--- a/test/lib/utils/setup-log.js
+++ b/test/lib/utils/setup-log.js
@@ -84,12 +84,12 @@ t.test('setup with color=always and unicode', t => {
   t.strictSame(WARN_CALLED, [['ERESOLVE', 'hello', { some: 'object' }]])
   WARN_CALLED.length = 0
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: 'always',
     unicode: true,
     progress: false,
-  })), true)
+  }))
 
   npmlog.warn('ERESOLVE', 'hello', { some: { other: 'object' } })
   t.strictSame(EXPLAIN_CALLED, [[{ some: { other: 'object' } }, true, 2]],
@@ -125,12 +125,12 @@ t.test('setup with color=true, no unicode, and non-TTY terminal', t => {
   process.stderr.isTTY = false
   process.stdout.isTTY = false
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: false,
     progress: false,
     heading: 'asdf',
-  })), false)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -156,12 +156,12 @@ t.test('setup with color=true, no unicode, and dumb TTY terminal', t => {
   process.stdout.isTTY = true
   process.env.TERM = 'dumb'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: false,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -187,12 +187,12 @@ t.test('setup with color=true, no unicode, and non-dumb TTY terminal', t => {
   process.stdout.isTTY = true
   process.env.TERM = 'totes not dum'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -218,12 +218,12 @@ t.test('setup with non-TTY stdout, TTY stderr', t => {
   process.stdout.isTTY = false
   process.env.TERM = 'definitely not a dummy'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), false)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -248,12 +248,12 @@ t.test('setup with TTY stdout, non-TTY stderr', t => {
   process.stderr.isTTY = false
   process.stdout.isTTY = true
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',