test: build astro tsdk expectation with component joins (fixes deterministic Windows shard 1/3 failure) - #248
Conversation
astro_uses_the_nearest_project_typescript_sdk built its expected path as
member.join("node_modules/typescript/lib"). Path::join keeps embedded
forward slashes verbatim, so on Windows the expectation held
...\\apps/site\\node_modules/typescript/lib while find_project_typescript_sdk
walks with .join("node_modules").join("typescript").join("lib") and
produces ...\\node_modules\\typescript\\lib. Deterministic mismatch on
Windows, invisible on unix where both spellings coincide.
Build the expectation the way production builds the value.
|
Superseded by Different fix shapes, worth a sentence in case it's useful rather than as an argument for reopening:
The only case they diverge on: if the product ever emitted a genuinely mixed spelling ( The 425-instance scope note from the description still stands independently of which fix landed: #246 rebased onto |
Unit / Cargo integration shard 1/3 (Windows)has been red onmainfor a while. I called it a rotating flake on #246 — that was wrong, and the log on my rebased run shows why.It's deterministic, not timing
node_modules\typescript\libvsnode_modules/typescript/lib. Not a timeout, not ordering — a path-separator mismatch that fails the same way every run.Production walks component-wise (
lsp/manager.rs:2636-2639):The test built its expectation in one hop (
lsp_manager_test.rs:453):Path::joindoesn't normalize embedded separators — it appends the string as given. On Windows that leaves the expectation holding a mixed-separator path that the production walk cannot produce. On unix both spellings coincide, so it passes everywhere except the platform it breaks on.This PR builds the expectation the way production builds the value. Same for the
apps/siteandsrc/page.astrojoins in that test, so the whole test uses native separators consistently.Why it looked like a flake
Two different failures live in that job and
fail-fastsurfaces whichever trips first:Rotating test names read as flake; the cancel point moving (317/493 vs 123/493) is timing, but one of the two underlying failures isn't. The process-exit timeout is still open and unrelated to this.
Verification
Linux, on this branch: 5 passed, 0 failed (
astro_uses_the_nearest_project_typescript_sdk,astro_user_initialization_options_override_the_computed_tsdk,astro_without_project_typescript_is_unavailable_without_spawning).cargo fmt --checkclean.The red control is the CI log above, not a synthetic one — I have no Windows host, so I can't run the before/after locally. The failure reproduces on two different shas, and the fix makes both sides of the comparison use the same construction, so they're identical by construction on every platform. Your Windows runner is the actual green control.
Scope note
The same pattern —
join("a/b/c")with embedded separators — appears 425 times across 49 files incrates/aft/tests/integration/. The vast majority are harmless: paths used forcreate_dir_allorfs::write, where Windows accepts forward slashes fine. Only ones whose value is compared against a production-built path can break, andfail-fastmeans any others are hidden behind the first failure.I've deliberately fixed only the one that's demonstrably red rather than sweeping 425 call sites blind. Flagging the class in case you want a broader pass — a lint or a test helper that builds paths component-wise would prevent the next one.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes a deterministic Windows failure in
astro_uses_the_nearest_project_typescript_sdkby building the expected TypeScript SDK path with component-wise joins. The test previously used a single join with embedded slashes, producing mixed separators on Windows and a guaranteed mismatch with production.join("node_modules").join("typescript").join("lib")to match productionfind_project_typescript_sdk; also updatesapps/siteandsrc/page.astrojoins for consistency.Unit / Cargo integration shard 1/3 (Windows). The separate process-exit timeout remains unrelated.Written for commit f6e2afc. Summary will update on new commits.
Greptile Summary
This PR fixes a deterministic Windows-only failure in the Astro TypeScript SDK integration test.
Confidence Score: 5/5
The PR appears safe to merge with no actionable issues identified.
The changed test constructs and serializes paths consistently with the production SDK lookup, fixing the Windows separator mismatch without changing fixture semantics on other platforms.
Important Files Changed
Reviews (1): Last reviewed commit: "test: build astro tsdk expectation with ..." | Re-trigger Greptile