fix: normalize paths in Windows tests - #1078
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1078 +/- ##
==========================================
+ Coverage 90.57% 90.60% +0.02%
==========================================
Files 217 217
Lines 20755 20799 +44
Branches 1969 1974 +5
==========================================
+ Hits 18799 18844 +45
+ Misses 1949 1948 -1
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| File | Main | PR | Change |
|---|---|---|---|
orama-db.json |
9.26 MB | 9.26 MB | -2.00 B (-0.0%) |
Performance estimate (single CI run)
- Generation time: 51.2% slower (6.85 s → 10.36 s)
- Peak memory: 10.9% lower (1.84 GB → 1.64 GB)
web Generator
Output size: 1 file changed · net +106.00 B
File size details
| File | Main | PR | Change |
|---|---|---|---|
all.html |
32.21 MB | 32.22 MB | +106.00 B (+0.0%) |
Performance estimate (single CI run)
- Generation time: 18.2% slower (69.00 s → 81.57 s)
- Peak memory: 4.3% lower (6.11 GB → 5.85 GB)
|
Could you please share the error / what this is fixing? |
|
Hi! This fixes a Windows-specific path separator issue in the comparator output. Initially, 4 test cases were failing due to this path normalization issue. I verified the fix locally on Windows by running 'npm test', and after the change, all 598 tests pass. Before this change, the test comparators report added and removed output files failed on Windows because the generated paths contained backslashes: | while the test expected forward slashes: | This resulted in an assertion error because the output did not match the expected path format. The change normalizes the paths so the comparator output is consistent across operating systems. |
All tests already passed in the past. Ifn this "fixes" something on Window, please attach logs of it failing on Windows, or is this fix theoretical? (aka you're assuming it is fixing something from Windows?) Bug Fixes in general must attach reproduction, logs/errors... Don't get me wrong, just trying to understand what's the actual fix :)
Noted, but is this coming from an actual run? Could you share that (aka, how you noticed this is happening? A local run or something?) |
| @@ -65,7 +65,7 @@ export async function processChunk(inputSlice, itemIndices) { | |||
| ); | |||
|
|
|||
| // The path is the relative path minus the extension | |||
| const relativePath = sep + withExt(relative(parent, path)); | |||
| const relativePath = `/${withExt(relative(parent, path)).replaceAll(sep, '/')}`; | |||
There was a problem hiding this comment.
It sounds like it is doing the opposite, converting any sep into / but that feels ... wrong? In Windows the \ sep should be used and not the / one 😅
There was a problem hiding this comment.
this feels backwards - sep is the right way to do this AFAIK
|
Hi, thanks for the clarification! I went back and reproduced the issue on Windows. I checked out main at commit da5d8e6 (before my changes) and ran npm test. The attached screenshots show:
After switching back to my PR branch and running the tests again, all 598 tests pass. So this is reproducible on Windows rather than a theoretical fix. Thanks again for pointing out that I should provide the reproduction details! |
|
I understand now, thanks for explaining, but here unironically the tests failing is correct in the sense that the test is wrong not the source. So the tests should be updated so the paths they're testing again are correct per environment -- TL;DR the source is correct, but the test assertion that is wrong (on Windows) -- so the test assertion should have two cases, one for unix-like and one for windows, or you put the {sep} within the test assertion instead of |
|
Ah, understood — thank you for clarifying! That makes sense. I reproduced the Windows failures, but I see now that the source behavior is correct and the tests are making Unix-specific assumptions. I'll update the test expectations to be platform-aware instead of normalizing the source output. |




Description
Normalize file paths to use forward slashes in output comparisons, ensuring tests behave consistently on Windows.
This updates the affected generators and comparators to avoid Windows-specific backslash paths in test output.
Validation
node --run test— all 598 tests passed.node --run format:check— passed.node --run lint— passed.Related Issues
N/A
Check List
node --run testand all tests passed.node --run format:check&node --run lint.