test(source): fix cross-platform path assertion mismatch on Windows - #175
test(source): fix cross-platform path assertion mismatch on Windows#175Adityakk9031 wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — a test-only fix to one assertion block in packages/leadtype/src/source/source.test.ts, resolving the Windows failure reported in #174.
- POSIX-normalized expected paths —
authoredPathand the newposixContentDirare converted to forward slashes before the equality and prefix assertions, matching the separator style thatDocsPageMeta.filePathactually carries. - Prefix assertion retargeted —
expect(generatedMeta.filePath.startsWith(...))now compares againstposixContentDirinstead of the native-separatorcontentDir.
The diagnosis holds up: filePath originates from tinyglobby's fg(..., { absolute: true }) at src/source/index.ts:501, which emits forward-slash absolute paths on Windows, while path.join emits native ones. On Linux both replaceAll calls are no-ops so nothing changes (confirmed — the test still passes, and biome check on the file is clean), and on Windows the change actually strengthens the startsWith assertion, which previously could never match and so passed vacuously. Reusing the normalized authoredPath as the writeMdx target is fine since Windows fs APIs accept forward slashes.
Worth noting for the record: a Windows-only separator fix can't be pinned by a test that runs on Linux, so there is no regression test to ask for here — the underlying gap is that no CI workflow runs on Windows.
ℹ️ filePath's separator contract is undocumented, and inconsistent with contentDir on Windows
This test now encodes "filePath is POSIX-separated" as expected behavior, but nothing in the source promises it — the JSDoc at src/source/index.ts:106 says only "Absolute path of the source file", and the value is whatever tinyglobby happens to return. Meanwhile source.contentDir is path.resolve(config.contentDir) (src/source/index.ts:425), so on Windows the public DocsSource surface hands consumers a native-separator contentDir alongside a POSIX-separator filePath. Any adapter doing meta.filePath.startsWith(source.contentDir) hits exactly the bug this PR just worked around in the test.
Nothing to change in this PR — it is scoped correctly as a test fix. Flagging it as a follow-up decision for a maintainer.
Technical details
# `DocsPageMeta.filePath` separator contract is implicit
## Affected sites
- `packages/leadtype/src/source/index.ts:106` — `filePath` JSDoc makes no separator guarantee.
- `packages/leadtype/src/source/index.ts:501` — value comes straight from `tinyglobby` (`absolute: true`), which is POSIX-separated on Windows.
- `packages/leadtype/src/source/index.ts:425` — `contentDir` is `path.resolve(...)`, i.e. native-separated, so the two disagree on Windows.
- `packages/leadtype/src/source/source.test.ts:457-459` — this PR pins the POSIX form in a test without the source documenting it.
## Required outcome
- A single, stated answer to "what separator style does `filePath` use?", so downstream adapters can rely on it rather than discovering it on a Windows machine.
## Suggested approach (optional)
- Either document `filePath` as always POSIX-separated and normalize `contentDir` (and any other exposed filesystem path) to match, or normalize `filePath` to native separators at the boundary in `selectSourceFiles`.
## Open questions for the human (optional)
- Is POSIX the intended contract for the whole public path surface, or is native-per-platform preferred with POSIX reserved for URL/relative paths (as `normalizeDocsPath` already does for `relativePath`)?Claude Opus | 𝕏
|
@KayleeWilliams have a look |

Problem
When running the test suite (
bun run test) on Windows,packages/leadtype/src/source/source.test.tsfails in the OpenAPI overlay test (createDocsSource > overlays generated OpenAPI pages while keeping authored pages live):Root Cause:
createDocsSource()returns metadata wherefilePathis POSIX-normalized (using/slashes).source.test.ts,authoredPathandcontentDirare created usingpath.join(), which outputs OS-native backslashes (\) on Windows..toBe(authoredPath)) and prefix checking (.startsWith(contentDir)) fail because of the slash separator mismatch (/vs\).Solution
Normalize
authoredPathandcontentDirinpackages/leadtype/src/source/source.test.tsto POSIX forward slashes (.replaceAll("\\", "/")) before running path equality and prefix assertions, ensuring consistent cross-platform test behavior on Windows and Linux.Resolves #174.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.