Repository navigation
[Tests] Isolate configureCLIEnvironment environment and color state - #8811
Draft
github-actions[bot] wants to merge 1 commit into
Draft
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
Replace the whole-process.env swap and afterAll restore with per-test vi.stubEnv, and restore chalk's module-level level around each test that changes it. Also assert the behavior the previous no-color tests named but never exercised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
WHY are these changes introduced?
The seven-day review of Main tests runs (2026-09-30T00:28Z → 2026-10-07T00:28Z UTC, all pages, every failed job and attempt) found three failures, all on
windows-latest:version.test.ts > writes the installed version as JSON without stderr outputtimed out at 20000msnode-package-manager.test.ts > checkForCachedNewVersion > returnes a version string when the cached value is greater than the current versionassertedundefinedinstead of'2.2.3'All three were already fixed by #8669, which is on
main(the three failing commits predate it), and the 19tests-mainruns since have all passed. With no actionable flake left, this picks up a quality issue instead.packages/theme/src/cli/utilities/cli-config.test.tsleaked state two ways. It reassigned the wholeprocess.envobject inbeforeEachand only restored the original inafterAll, so any module that captured a reference toprocess.envsaw a different object for the duration of the file. It also set chalk's module-levelcolors.levelinbeforeEachand never restored it, leaving the singleton at whatever the last test set —0after thenoColor: truecase.Two of the
noColortests were also misnamed and under-asserting: both claimed to check theno-colorenvironment variable, deleted it, and then asserted only oncolors.levelandFORCE_COLOR. Nothing covered the two flags together, or the no-op case.WHAT is this pull request doing?
Replaces the
process.envobject swap with per-testvi.stubEnvplus a singleafterEach(vi.unstubAllEnvs), so each test declares the variables it depends on and nothing outlives it. Wraps the tests that mutatecolors.levelin a small helper that sets the level to1and restores the previous value in afinally, which removes theafterAlland the last shared mutable state in the file.Renames the two
noColortests to match what they assert, and adds three cases for behavior that was previously uncovered: an existing verbose variable survivingverbose: false,verboseandnoColorapplied together, and empty options changing nothing.Coverage is preserved and tightened — no production code changed. Four mutations of
cli-config.ts(always set verbose, always disable colors, drop theFORCE_COLORwrite, drop thecolors.levelwrite) each fail two tests; before this change the first and third were caught but the misnamed tests left the no-op paths unasserted.Validated on Linux (ubuntu, Node 22.20.1) with
theme:lint,theme:type-check, and the file under five shuffled seeds, the fullutilitiesdirectory in boththreadsandforkspools, and a single-worker shuffled run. Three unrelated tests (theme-environment.test.tssnippet-scripts aggregation,html.test.tslastRequestedPath, anddev.test.tsCtrl-C analytics) fail under shuffled order, but they fail identically on a clean checkout with the same seeds, so they are pre-existing and out of scope here. The originally failing platform was Windows; these checks were not run there.How to manually test your changes?
CI
Post-release steps
None.
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add