test: make fs mocks tolerate non-string paths from the module loader#302
Merged
Conversation
The global fs.readFileSync/lstatSync mocks in core.spec.js assumed `path`
was always a string and called `path.includes(...)` on it. The four
`actualConventionalRecommendedBump` tests exercise the real preset loader,
whose dynamic `import('conventional-changelog-conventionalcommits')` reads
the ESM module through fs with a non-string path (URL/Buffer/fd) on Linux.
That threw `TypeError: path.includes is not a function`, which the preset
loader rewrapped as the misleading "Unable to load the preset. Please make
sure it's installed." — failing CI on ubuntu while passing on macOS (which
is not in the CI matrix).
Delegate non-string paths straight to the real fs so module loading works;
string paths still get the fixture handling. Test-only change; no product
code is affected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
macOS was absent from the matrix, which is why the fs-mock preset-loading failure went unnoticed locally (contributors on macOS) while CI on Linux stayed red. Cover all three major platforms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
What
Guard the global
fs.readFileSync/fs.lstatSyncmocks intest/core.spec.jsso non-string paths are delegated to the real fs instead of being fed topath.includes(...).Why
CI has been red on
ubuntu-latestsince the vitest migration (#283) + the conventional-changelog deps modernization, with:The preset is installed. The four
actualConventionalRecommendedBumptests run the real preset loader, whose dynamicimport('conventional-changelog-conventionalcommits')reads the ESM module throughfswith a non-string path (URL/Buffer/fd) on Linux. The mock assumedpathwas always a string, sopath.includes(...)threw — and the preset loader rewrapped it as the misleading "make sure it's installed" message.It passed locally because the CI matrix is
ubuntu-latest+windows-latestonly — no macOS, where the failure doesn't reproduce.This is a test-harness bug, not a product regression — no shipped code changes, and the failure predates the 13.0.0 release.
Change
Non-string paths (module-loader reads) now pass straight to real fs; string paths keep the existing fixture handling. Test-only.