test(cli): compare State Root locations with the platform separator - #4471
Closed
Astro-Han wants to merge 1 commit into
Closed
test(cli): compare State Root locations with the platform separator#4471Astro-Han wants to merge 1 commit into
Astro-Han wants to merge 1 commit into
Conversation
`durable state covers the control namespace, not only the State Root` built its expectations with POSIX literals while the code under test builds paths with `path.join`. On Windows the two disagree: `join` returns `\qualification-scope\state-root`, so the equality assertion was always false and `npm run check:release` — which the Windows packaging job runs — failed there. Build the expected State Root with the same `join` the production code uses, and separate the nesting guard with `path.sep`. That guard had the same POSIX assumption: on Windows no golden path can start with `<live>/`, so it passed vacuously and proved nothing. The defect stayed latent because `Release Windows check` only runs when a pull request touches the release packaging inputs, and this test file is not one of them. Generated-by: Claude Code
6 tasks
Contributor
Author
|
Duplicate of #4461, which already carries the identical fix. Closing. |
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.
Summary
durable state covers the control namespace, not only the State RootcompareddurableStateLocations()output against POSIX string literals while the function itself builds paths withpath.join. On Windows those disagree —join('/qualification-scope', 'state-root')is\qualification-scope\state-root— so the equality assertion was always false,npm run check:releasefailed, and with it the Windows packaging job that runs it.The nesting guard in the same block carried the same assumption: on Windows no golden path can start with
`${live}/`, so it passed vacuously and proved nothing. Both now use the platform's own separator.The defect stayed latent on
mainbecauseRelease Windows checkonly runs when a pull request touches the release packaging inputs, and this test file is not one of them. It surfaced on #4468, which does touch them.Refs #4427
Verification
node --test --test-concurrency=1 scripts/qualify-released-cli-state-root.test.mjs— 8/8 pass.biome check scripts/qualify-released-cli-state-root.test.mjs— clean.scripts\qualify-released-cli-state-root.test.mjs:195,actual: false), andpath.win32.join('/qualification-scope', 'state-root')returning\qualification-scope\state-rootis what makes that assertion false. This PR does not tripRelease Windows checkeither, so the end-to-end confirmation comes from rebasing feat(desktop): ship macOS x64 and Linux packages #4468 onto it.Root cause
A test asserted on path syntax instead of on the path constructor the code under test uses.
AI use
Select exactly one:
Tool(s) and scope: Claude Code isolated the failure from the CI log and wrote the change. The contributor reviewed the diff and the reasoning.
Checklist
Does this PR entail a change in behavior?