Repository navigation
[Tests] Cover DoctorSuite filesystem assertions - #8748
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
The DoctorSuite filesystem assertions had no coverage, and the test file auto-mocked ../fs.js, which is what made them untestable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 of 4 tasks
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?
DoctorSuiteis the base class that release-verification suites (ThemeInitTests,ThemePushTests) extend to assert on files produced by CLI commands. Its three filesystem assertions —assertFile,assertNoFile, andassertDirectory— had no test coverage at all, so regressions in path resolution, content matching, or pass/fail polarity would land silently and surface as wrong doctor results rather than a failing build.The gap was held open by the test file itself:
framework.test.tsauto-mocked../fs.jswithout ever configuring the mock, which made every filesystem assertion unobservable and effectively untestable.The seven-day review of Main tests runs (2026-09-28 to 2026-10-05 UTC, 29 runs, 9 failed jobs) found no actionable flake left to fix: all nine failures were
version.test.tssubprocess timeouts and anode-package-managercache assertion, both already fixed onmainby #8669, plus oneconf-storerate-limit failure onstable/4.8already fixed by #8705. With no remaining flake, this closes the coverage gap instead.WHAT is this pull request doing?
Removes the unused
vi.mock('../fs.js')and covers the three filesystem assertions against real files in temporary directories:assertFilewith no pattern, a matchingRegExp, a matching string, and a non-matching stringassertFileon a missing file, confirming it reportsfile not foundand never reads contentassertFilewith an absolute path and a custom messageassertNoFileandassertDirectoryfor both the present and absent casesEach test writes its own fixtures inside
inTemporaryDirectoryand asserts on the recordeddescription/passed/actualcontract rather than on internal calls, so the assertions stay refactorable. No production code changed, and the existingvi.mock('../system.js')is kept since command execution is out of scope here.Validation: the suite was sanity-checked by temporarily inverting
assertNoFile'spassedflag and dropping theisAbsolutePathbranch inassertFile— two of the new tests failed, and both pass again once reverted.pnpm vitest run packages/cli-kit/src/public/node/doctor/passes 25 tests (up from 17), including with--sequence.shuffle, alongside ESLint andtsc --noEmitoncli-kit. Verified locally on Linux / Node 22; the OS and Node matrix is left to CI.How to manually test your changes?
CI
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add