You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test: isolate color environment in configureOutput tests - #2608
Running the tests from a shell with NO_COLOR=1 makes the two configureOutput force-color cases fail, even though Commander correctly gives NO_COLOR precedence. With FORCE_COLOR=0, the CLICOLOR_FORCE case also inherits a conflicting setting. The tests currently save and replace only the variable they are testing.
Clear all three color variables before each subtest and restore their original values once in after when the group finishes, so inherited settings cannot change the assertions and a failed assertion still restores the environment. This changes only test setup and cleanup.
Validation on develop 97411c6:
Reproduced two failures with NO_COLOR=1 node --test tests/command.configureOutput.test.js before the change.
The focused file passes with no color variables, each of NO_COLOR=1, FORCE_COLOR=0, FORCE_COLOR=1, CLICOLOR_FORCE=1, and all three conflicting settings together.
npm test: 1,420 passing with the original NO_COLOR=1 environment.
npm run check: types, lint and formatting pass.
The focused file also passes on the minimum supported Node 22.12.0.
Suggested changelog summary: isolate configureOutput tests from inherited color environment variables.
Why use afterEach instead of after? The beforeEach is going to delete the keys anyway, so there is no need to reset them to the original value after every iteration.
afterEach was more cleanup than this serial group needs. I've changed it to t.after() in f57c87b: beforeEach still clears all three controls before each case, and the original environment is restored once when the group finishes, including if a subtest fails. This also matches the cleanup scope used in useColor.test.js.
Rechecked all six environment configurations, the full 1,420-test suite, types/lint/format, and the 24-test focused file on Node 22.12.0. All pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running the tests from a shell with
NO_COLOR=1makes the two configureOutput force-color cases fail, even though Commander correctly givesNO_COLORprecedence. WithFORCE_COLOR=0, the CLICOLOR_FORCE case also inherits a conflicting setting. The tests currently save and replace only the variable they are testing.Clear all three color variables before each subtest and restore their original values once in
afterwhen the group finishes, so inherited settings cannot change the assertions and a failed assertion still restores the environment. This changes only test setup and cleanup.Validation on develop
97411c6:NO_COLOR=1 node --test tests/command.configureOutput.test.jsbefore the change.NO_COLOR=1,FORCE_COLOR=0,FORCE_COLOR=1,CLICOLOR_FORCE=1, and all three conflicting settings together.npm test: 1,420 passing with the originalNO_COLOR=1environment.npm run check: types, lint and formatting pass.Suggested changelog summary: isolate configureOutput tests from inherited color environment variables.
Implemented and validated with Codex.