Repository navigation
test(compression): close F5.1 coverage gaps (replay reducer, live accumulator, StatusDot) - #4192
Conversation
…umulator, StatusDot) The F5.1 traceability audit of the compression track found three untested units (code present + correct, no covering test): - F3.2 useCompressionReplay state machine: only buildReplayFrames was tested; the replay reducer (play/pause/tick/auto-stop-at-end/reset/ set-speed) + stepMs pacing were not. Export them as unit-test seams (visibility-only, no logic change) and pin every transition. - F3.3 useLiveCompression: accumulateRun (exported "for unit tests" but never tested) — pins most-recent-first ordering, the maxRuns cap, and immutability of the input list. - F0.2 StatusDot (U0): no dedicated render test — pins the error-color override and sizeClass via createRoot+act (jsdom). Tests only (+ 5 export keywords as a test seam). 17 new tests green (14 node:test + 3 vitest). typecheck:core / lint / check:cycles clean.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request exports internal state machine elements from useCompressionReplay.ts to serve as unit-test seams and introduces new unit tests for the compression replay reducer, the live compression run accumulator, and the StatusDot UI component. The review feedback suggests tracking and properly unmounting React 18 roots inside act during test cleanup to prevent memory leaks and ensure lifecycle effects are flushed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const containers: HTMLElement[] = []; | ||
|
|
||
| function mount(ui: React.ReactElement): HTMLElement { | ||
| const container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
| containers.push(container); | ||
| const root = createRoot(container); | ||
| act(() => { | ||
| root.render(ui); | ||
| }); | ||
| return container; | ||
| } |
There was a problem hiding this comment.
In React 18, when using createRoot, failing to call root.unmount() can lead to memory leaks and unexpected side effects (such as cleanup effects running after the DOM container has been removed). We should track the created roots and unmount them during cleanup.
| const containers: HTMLElement[] = []; | |
| function mount(ui: React.ReactElement): HTMLElement { | |
| const container = document.createElement("div"); | |
| document.body.appendChild(container); | |
| containers.push(container); | |
| const root = createRoot(container); | |
| act(() => { | |
| root.render(ui); | |
| }); | |
| return container; | |
| } | |
| const roots: { root: ReturnType<typeof createRoot>; container: HTMLElement }[] = []; | |
| function mount(ui: React.ReactElement): HTMLElement { | |
| const container = document.createElement("div"); | |
| document.body.appendChild(container); | |
| const root = createRoot(container); | |
| act(() => { | |
| root.render(ui); | |
| }); | |
| roots.push({ root, container }); | |
| return container; | |
| } |
| afterEach(() => { | ||
| while (containers.length > 0) containers.pop()?.remove(); | ||
| document.body.innerHTML = ""; | ||
| }); |
There was a problem hiding this comment.
Unmount the React roots inside act to ensure all React lifecycle cleanups are properly flushed and to prevent memory leaks.
afterEach(() => {
act(() => {
while (roots.length > 0) {
const item = roots.pop();
if (item) {
item.root.unmount();
item.container.remove();
}
}
});
document.body.innerHTML = "";
});
F5.2 review found that changing the replay speed while playing was silently ignored: startTick() reads speedRef.current synchronously, but the ref was only synced in a post-render effect — too late for the in-flight restart, so the ticker kept the old cadence. Set speedRef.current synchronously in handleSetSpeed before restarting the interval. TDD (compression-replay-speed.test.tsx, fake timers + createRoot/act): after setSpeed(3) mid-play, advancing only 133ms advances a frame (proven to fail — frameIndex stuck at 0 — before the fix). typecheck:core / lint clean.
…umulator, StatusDot) (cheaper-inference#4192) Integrated into release/v3.8.29
F5.1 coverage gaps — compression track
The F5.1 traceability audit (item→code→test) of the compression track (rodada4 master
plan, ~19 items) found the track ~16/19 done+tested, with three untested units —
code present and correct, but no covering test. This PR closes them (tests only).
useCompressionReplayreducer +stepMsbuildReplayFrameswas tested; the replay state machine (play / pause / tick / auto-stop-at-last-frame / reset / set-speed) and the speed→interval pacing had zero assertions.useLiveCompression→accumulateRunmaxRunscap (drops oldest), and input-array immutability.StatusDot(U0)sizeClassviacreateRoot+act(jsdom).Scope
Tests only. The single production change is visibility-only:
replayReducer,INITIAL_STATE,stepMs, and theReplayState/ReplayActiontypes are exported fromuseCompressionReplay.tsas unit-test seams (no logic change), matching the codebase'spattern of exposing pure functions for direct testing (cf.
compressionFlowModel).Tests
17 new tests, all green: 14 node:test (
compression-replay-reducer,live-compression-accumulate) + 3 vitest (status-dot).typecheck:core/lint/check:cyclesclean.Context
Part of the F5 consolidation pass. Remaining compression-track items tracked separately:
F2.4 benchmark A/B (VPS-gated), F3.3 real mid-pipeline
compression.stepstreaming(scope decision — current impl is a client-side replay of one
compression.completedevent), and F4.4 quantization (PRs #4187 + #4190).