Skip to content

Commit

Permalink
chore: remove hard set tap colors
Browse files Browse the repository at this point in the history
The original PR #2225 indicated that this
would prevent snapshot failures in CI with no color, but I hard set this
to "0" and ran tests and they passed.  This will make debugging tests in
CI environments like the Node.js CITGM much easier.

The reason for the failures is because even if we hard set environment
variables in tests to force color one way or another, chalk has already
made up its mind so changing those in the same process won't affect
anything.  The fix is to hard set two values in the actual `npm test`
script to make `chalk` and `supports-color` do what we need.

The `sudo` test commands were also removed because we do not want to
encourage running npm w/ sudo.
  • Loading branch information
wraithgar committed Apr 6, 2022
1 parent 840c338 commit 2dd7775
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 14 deletions.
7 changes: 2 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,11 @@
"dumpconf": "env | grep npm | sort | uniq",
"preversion": "bash scripts/update-authors.sh && git add AUTHORS && git commit -m \"chore: update AUTHORS\" || true",
"licenses": "licensee --production --errors-only",
"test": "tap",
"test": "FORCE_COLOR=true NO_COLOR='' tap -Rclassic",
"test-all": "npm run test --if-present --workspaces --include-workspace-root",
"snap": "tap",
"snap": "FORCE_COLOR=true NO_COLOR='' tap -Rclassic",
"postsnap": "make -s mandocs",
"test:nocleanup": "NO_TEST_CLEANUP=1 npm run test --",
"sudotest": "sudo npm run test --",
"sudotest:nocleanup": "sudo NO_TEST_CLEANUP=1 npm run test --",
"posttest": "npm run lint",
"lint": "eslint \"**/*.js\"",
"lintfix": "npm run lint -- --fix",
Expand All @@ -232,7 +230,6 @@
"test-env": [
"LC_ALL=sk"
],
"color": 1,
"files": "test/{lib,bin,index.js}",
"coverage-map": "test/coverage-map.js",
"timeout": 600
Expand Down
8 changes: 4 additions & 4 deletions test/lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,19 +377,19 @@ t.test('color', t => {
setTTY('stdout', false)
obj.color = true
definitions.color.flatten('color', obj, flat)
t.strictSame(flat, { color: false, logColor: false }, 'no color when stdout not tty')
t.match(flat, { color: false }, 'no color when stdout not tty')
setTTY('stdout', true)
definitions.color.flatten('color', obj, flat)
t.strictSame(flat, { color: true, logColor: false }, '--color turns on color when stdout is tty')
t.match(flat, { color: true }, '--color turns on color when stdout is tty')
setTTY('stdout', false)

setTTY('stderr', false)
obj.color = true
definitions.color.flatten('color', obj, flat)
t.strictSame(flat, { color: false, logColor: false }, 'no color when stderr not tty')
t.match(flat, { logColor: false }, 'no logColor when stderr not tty')
setTTY('stderr', true)
definitions.color.flatten('color', obj, flat)
t.strictSame(flat, { color: false, logColor: true }, '--color turns on color when stderr is tty')
t.match(flat, { logColor: true }, '--color turns on logColor when stderr is tty')
setTTY('stderr', false)

const setColor = (value) => mockGlobals(t, { 'process.env.NO_COLOR': value })
Expand Down
7 changes: 2 additions & 5 deletions test/lib/utils/error-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,6 @@ t.test('explain ERESOLVE errors', async t => {
},
},
}))
t.match(EXPLAIN_CALLED, [[
er,
false,
path.resolve(npm.cache, 'eresolve-report.txt'),
]])
t.equal(EXPLAIN_CALLED[0][0], er)
t.equal(EXPLAIN_CALLED[0][2], path.resolve(npm.cache, 'eresolve-report.txt'))
})

0 comments on commit 2dd7775

Please sign in to comment.