Skip to content

test(source): fix cross-platform path assertion mismatch on Windows - #175

Merged
KayleeWilliams merged 1 commit into
inthhq:mainfrom
Adityakk9031:issue-174
Aug 9, 2026
Merged

test(source): fix cross-platform path assertion mismatch on Windows#175
KayleeWilliams merged 1 commit into
inthhq:mainfrom
Adityakk9031:issue-174

Conversation

@Adityakk9031

@Adityakk9031 Adityakk9031 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Problem

When running the test suite (bun run test) on Windows, packages/leadtype/src/source/source.test.ts fails in the OpenAPI overlay test (createDocsSource > overlays generated OpenAPI pages while keeping authored pages live):

AssertionError: expected 'C:/Users/user/AppData/Local/Temp/lead...' to be 'C:\Users\user\AppData\Local\Temp\lead...' // Object.is equality

- Expected: "C:\Users\user\AppData\Local\Temp\leadtype-source-LinCaf\guide.mdx"
+ Received: "C:/Users/user/AppData/Local/Temp/leadtype-source-LinCaf/guide.mdx"

 ❯ src/source/source.test.ts:454:37
    452|         throw new Error("Expected authored and generated metadata.");
    453|       }
    454|       expect(authoredMeta.filePath).toBe(authoredPath);
    455|       expect(generatedMeta.filePath).toContain("leadtype-openapi-");
    456|       expect(generatedMeta.filePath.startsWith(contentDir)).toBe(false);

Root Cause:

  1. createDocsSource() returns metadata where filePath is POSIX-normalized (using / slashes).
  2. In source.test.ts, authoredPath and contentDir are created using path.join(), which outputs OS-native backslashes (\) on Windows.
  3. Strict string equality (.toBe(authoredPath)) and prefix checking (.startsWith(contentDir)) fail because of the slash separator mismatch (/ vs \).

Solution

Normalize authoredPath and contentDir in packages/leadtype/src/source/source.test.ts to POSIX forward slashes (.replaceAll("\\", "/")) before running path equality and prefix assertions, ensuring consistent cross-platform test behavior on Windows and Linux.

Resolves #174.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Adityakk9031, you've reached your PR review limit, so we couldn't start this review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 65ac8dd0-7a71-4284-8959-cdf967d28458

📥 Commits

Reviewing files that changed from the base of the PR and between b5857b4 and dfdc8a0.

📒 Files selected for processing (1)
  • packages/leadtype/src/source/source.test.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 pathsauthoredPath and the new posixContentDir are converted to forward slashes before the equality and prefix assertions, matching the separator style that DocsPageMeta.filePath actually carries.
  • Prefix assertion retargetedexpect(generatedMeta.filePath.startsWith(...)) now compares against posixContentDir instead of the native-separator contentDir.

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`)?

Pullfrog  | View workflow run | Using Claude Opus𝕏

@Adityakk9031

Copy link
Copy Markdown
Contributor Author

@KayleeWilliams have a look

@KayleeWilliams
KayleeWilliams merged commit 66aa3ac into inthhq:main Aug 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(source): fix cross-platform path assertion mismatch on Windows

2 participants