Repository navigation
test: golden-master interaction traces with a browser cross-check - #838
Merged
Merged
Conversation
Batched drag helpers let movement arrive before React commits the start state, leaving bounds and user callbacks untested. Record each input in its own act and retain a batched cadence to pin that behavior too. Capture ordered callbacks, DOM output and actual listener attachments, with lossless special-value encoding and an explicit golden update gate.
Refactors need an observable baseline for bounds, slack, touch identity, callback vetoes and teardown. Add 164 short interaction scenarios across both component layers and event cadences, retaining today's bugs as named metadata. Run the traces in the normal test suite and regenerate them only through golden:update. Bypassing the bounds call at Draggable.tsx:318 causes 10 trace failures; restoring it returns the suite to green.
Synthetic DOM input cannot establish browser layout fidelity. Add a separate offline Playwright project comparing 16 shared mouse scenarios against committed jsdom traces, using local React and library source while preserving the existing Puppeteer suite. Add two isolated React 18 goldens that exercise the real findDOMNode fallback. Both pass against a matching cached runtime. The browser fixture compiles and collects, but Chromium launch is blocked by macOS MachPort permissions, so real browser parity remains unverified.
Behavior changes need an explicit review trail before the planned refactors. Document the trace format, per-event and batched timing, known-bug convention, update commands and isolated source cross-check projects. Record the verified React 18 path and the sandbox limitation on browser execution so compilation is not mistaken for browser parity.
Sentinels ({$type}, {$number}) are plain objects, so a trace value could change to a lookalike without changing its golden file, and array holes encoded as null. Ordinary objects with a "$" key are now wrapped in {$escaped}, and holes encode as {$hole: true}. Same fix as react-grid-layout#2320 round 1. No golden file changed; a new serializer test covers each collision.
The browser cross-check loaded Playwright from PLAYWRIGHT_MODULE_PATH only, so CI could not run it. Pin playwright@1.62.1, the version react-grid-layout and react-resizable use.
Install Playwright Chromium and run yarn golden:browser after the Puppeteer browser tests. The github-actions reporter turns each parity failure into a check annotation. Locally: 16/16 traces match real Chromium input.
|
Caution Review failedThe pull request is closed.
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. |
Review round 1 (P1): every trace recorded package.json's version and the browser check asserted 4.7.2, so the release command's version bump would break the golden suite on the release commit itself. Traces now carry only schemaVersion. Sweep: rg for package version use in test/golden found runScenario.jsx, legacy.golden.test.jsx and crosscheck.test.js; react-grid-layout and react-resizable goldens record no package version. 166 golden files lose their version line and nothing else; the two React 18 legacy goldens were edited the same way because regenerating them needs a separate React 18 install. P2: the legacy runner required exactly React 18.3.1 although its config accepts any matching React 18; it now checks the major version. P2: the browser check left its temporary Vite cache directory behind; afterAll now removes it, also when launch fails. A run now leaves no directory behind.
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
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.
Pins every observable output of
DraggableandDraggableCorebefore the perf and clarity work. No file underlib/changes.The old tests send mousedown and mousemove in one
act(), so the move lands before the drag state commits andDraggable.onDrag(bounds, slack, the user callback) never runs in jsdom. This harness dispatches oneact()per event, and also pins the batched cadence, since some consumers hit it.166 scenarios (145 per-event, 21 batched): both components, controlled and uncontrolled,
defaultPosition,positionOffset, everyaxis, parent / selector / object bounds, grid, scale 0.5 and 2, handle and cancel,allowAnyClick, disabled,nodeRefvs findDOMNode,enableUserSelectHack,allowMobileScroll, ctrl+click, return-false from each callback, multi-touch, mouse+touch hybrids, unmount mid-drag, StrictMode. An optional project replays the findDOMNode path under React 18.Coverage:
Draggable.tsx62.5% -> 97.2% lines,DraggableCore.tsx83.2% -> 97.5%. Mutation check: dropping the bounds call atlib/Draggable.tsx:318fails 10 traces.Bugs pinned with
"knownBug", not fixed: R1 stuck-dragging batch, R2 onStop false called twice, R3 touch drag never stops after onDrag false, R4 second pointer fires onStart again, R5 idle unmount clears selection mid-drag, R6/R7 uncontrolled onStop position and bounds, R9 touch breaks after the child node changes, R10 hybrid touchend gives NaN, R11.The serializer keeps sentinels distinct from lookalike objects and keeps array holes, the same fix react-grid-layout#2320 needed. CI installs Playwright Chromium and runs
yarn golden:browserwith thegithub-actionsreporter.Status - 2026-10-31
yarn testyarn golden:browserReview gate:
codex-review.shwithtest/golden/__golden__/**excluded as generated.Test plan