[Tests] Make getPackageManager environment fallback test deterministic - #8601
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 test asserted `getPackageManager` returns 'pnpm' for a directory with no package.json. Nothing in the fixture produces that result — it came from the ambient `npm_config_user_agent` of whichever package manager launched vitest. The test passes under `pnpm test` and fails under any other runner, and it never exercised the `unknown -> npm` fallback it appeared to be covering. Replace it with two tests that stub the user agent explicitly: one for an unrecognized user agent, one for an empty one. Both assert the 'npm' fallback, and both fail if that branch is removed. 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
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?
getPackageManagerwalks up from a directory looking for a lockfile, and when it finds none it falls back topackageManagerFromUserAgent(), then to'npm'if that returns'unknown'.The test named "tries to guess the package manager from the environment if it can't find a package.json" created an empty temp directory and asserted the result was
'pnpm', with the comment "pnpm is used locally and in CI". Nothing in the fixture produces'pnpm'— the value came from the ambientnpm_config_user_agentof whichever package manager happened to launch vitest. The test passes underpnpm testand fails under any other runner:Two problems follow from that. The suite is not runnable outside pnpm, and the assertion pinned an environment value rather than behavior — the
unknown -> npmfallback the test appeared to cover was never executed, since the user agent always resolved to a known manager.WHAT is this pull request doing?
Replaces that test with two that stub
npm_config_user_agentexplicitly instead of reading whatever the runner set:some-other-tool/1.0.0), andboth asserting the
'npm'fallback. Each restores the environment in afinallyblock, matching the surrounding tests in the file, and neither depends on the ambient runner. Verified they pass under pnpm, npm,npm_config_user_agent=yarn/1.22.0, and an empty user agent, and that both fail if theunknown -> npmfallback is removed fromgetPackageManager.No production code is changed.
How to manually test your changes?
CI
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add