Add UnifiedDiffView tests for the diff annotation surface - #931
Merged
Conversation
useDiffWidgets and UnifiedDiffView had no direct coverage. Between them they own where every review annotation lands: the hook keys widgets by react-diff-view change key, the view turns those keys back into rows, and the gutter handler owns the line-selection semantics behind the comment affordance. The tests mount the real tree (react-diff-view, the hook, and the three inline annotation components) with only the HTTP seam and the animation layer mocked, so a mis-keyed widget cannot pass. They pin widget anchoring to the last changed line in a range, the feedback grouping and skip rules, draft composition on a shared anchor, the three-way gating and callback contract of the inline comment form, and gutter selection (single, shift-extend in both directions, click-to-clear, delete-line no-op). One current behavior is pinned rather than fixed: the comment form replaces any feedback or draft annotation sharing its anchor instead of composing with it. That is a product/UX call, tracked as DIS-168. Mutation battery: 25/25 mutants killed.
Capture the original scrollIntoView property descriptor and put it back in afterEach so the prototype patch cannot outlive the file (vi.spyOn cannot be used — jsdom never defines the method, so there is nothing to wrap). The floating comment affordance lookup now throws when more than one direct-child button exists instead of silently returning the first, so a future sibling button fails loudly rather than turning these assertions into false negatives. Widget adjacency is left strict on purpose, with a comment explaining why: immediate succession is how react-diff-view expresses "anchored to this line", which is the contract under test.
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 freeto 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.
Recurring Test Enforcer run. The local suite was green on first pass (23rd consecutive run), so this PR is coverage only — no product code changed.
Local validation
pnpm run checkpnpm run testpnpm run test:e2epnpm run test:e2e:livee2e-*tmux sessions (14th consecutive)pnpm run finalize:webgh run list --limit 50)Both entries on the flake list are recognition-only records of already-fixed infra problems; neither recurred. The
server-tests-isolated.shwrapper from #922 provisioned its own Postgres and left nothing behind, and the three orphaneddispatch-postgres-test-*containers noted last run are gone.What this adds
apps/web/src/components/app/unified-diff-view.test.tsx— 31 tests covering two modules that had no direct coverage:use-diff-widgets.tsx— builds thechangeKey → ReactElementmap of inline annotations (review feedback, draft comments, the inline comment form).unified-diff-view.tsx— renders the diff, owns gutter-click line-selection semantics, and feeds the widget map to react-diff-view.Between them they decide where every review annotation lands, which is exactly the kind of wiring a stubbed test would miss. So the tests mount the real tree — react-diff-view, the real hook, and the real
InlineFeedbackAnnotation/InlineDraftAnnotation/InlineCommentForm— with only the HTTP seam and the animation layer mocked. Rows are located by their code text and widgets by row adjacency, so no change keys are hardcoded and a mis-keyed widget cannot pass.Pinned behavior:
ln >= nullwould otherwise coerce toln >= 0), and ranges covering no rendered changeMutation battery: 25/25 behavior-changing mutants killed across both source files.
Follow-up filed, not fixed here
DIS-168 — opening the inline comment form hides any feedback or draft annotation anchored to the same line. The draft pass explicitly composes with what is already at a key; the comment-form pass assigns unconditionally and clobbers it. The mechanical fix is one line, but whether the form should take over the row is a UX call, so current behavior is pinned with a comment saying to invert that assertion once it is decided.
Also noted while writing these:
parseDiffreturns one hunk-less file even for junk input, so the view's"Unable to parse diff"fallback only fires ifparseDiffthrows. What an unparseable payload actually renders is an empty table — pinned as-is with a comment.Review
One
code-reviewpersona pass (review 651): 3 items, all test hygiene, all resolved.scrollIntoViewprototype patch via its captured property descriptor (vi.spyOncan't be used — jsdom never defines the method, so there is nothing to wrap)🤖 Generated with Claude Code