test(recording): make the re-index suite run off Linux - #260
Merged
Conversation
reindexRecordingOnDisk is Linux-gated by intent — Windows and macOS record through native helpers that write indexed files at the source, so the wrapper returns `unsupported-platform` before touching anything else. The suite never accounted for that: six of its cases inject a fake remux service and assert the remuxed result, so on a macOS or Windows checkout they stop at the guard and fail. They were red on a clean tree, for environmental reasons, with nothing to distinguish them from a real regression. Pin process.platform to linux in beforeEach and restore the real value in afterEach, so the cases exercise the wrapper's own logic on every platform. The technique is the one the suite's last case already used inline; hoisting it removes the per-test save/restore boilerplate and makes the restore hold even when a case throws part-way. The guard itself stays covered, now over both platforms it exists for rather than win32 alone, so pinning to Linux can't quietly become the only thing the suite exercises. No production code changes. Verified the cases still have teeth: removing the empty-output size check from reindexRecordingOnDisk fails the truncated-file case, which had been passing vacuously on macOS. `npx vitest --run` is now green on macOS: 137 files, 1626 passing.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
1 participant
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.
npx vitest --runcurrently shows 6 red tests on a clean checkout for anyone on macOS or Windows. They are environmental, not a regression — but nothing says so, and working that out by hand costs every new contributor the same half hour.Cause
reindexRecordingOnDiskis Linux-gated by intent (webm-seek-index.ts:62) — Windows and macOS record through native helpers that already write indexed files, so re-muxing their output would be pure cost:Six cases in the suite inject a fake remux service and assert the remuxed result. Off Linux they stop at that guard and fail. Worth noting they weren't just failing — they were asserting nothing about the wrapper at all on those platforms.
Fix
Pin
process.platformtolinuxinbeforeEach, restore the real value inafterEach. The technique is the one the suite's own last case already used inline; hoisting it drops the per-test save/restore boilerplate and makes the restore hold even when a case throws part-way through.The guard stays covered — now parametrised over both platforms it exists for rather than
win32alone, so the Linux pin can't quietly become the only thing the suite ever exercises.No production code changes. Test file only.
Verification
The cases still have teeth — this is the part that mattered, since they had been passing vacuously. Removing the empty-output size check from
reindexRecordingOnDiskfails the truncated-file case:Implementation restored byte-identical afterwards (empty diff).
Full suite on macOS, from 6 failures to none:
tsc --noEmitclean, Biome clean (13 warnings, unchanged from baseline).