Skip to content

test(jsdom): resettable scrollIntoView spy + dedupe matchMedia stub#639

Merged
BigSimmo merged 1 commit into
mainfrom
claude/jsdom-setup-cleanup
Jul 14, 2026
Merged

test(jsdom): resettable scrollIntoView spy + dedupe matchMedia stub#639
BigSimmo merged 1 commit into
mainfrom
claude/jsdom-setup-cleanup

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

Small follow-up to #636 addressing the one CodeRabbit nitpick (from the now-closed, superseded #623) that applies to code actually on main — the jsdom test setup.

  • tests/setup/jsdom.setup.tsscrollIntoView was assigned once as a plain vi.fn(), but vi.restoreAllMocks() (afterEach) only restores vi.spyOn spies, so its call history never reset between tests (a latent isolation gap for any future test asserting on scrollIntoView). Now ensures a base impl exists, then re-spies via vi.spyOn(...).mockImplementation(() => {}) each beforeEach so it resets consistently. Also exports installMatchMediaStub for reuse.
  • tests/accessible-table.dom.test.tsx — reuses the shared installMatchMediaStub instead of a copy-pasted local setMatchMedia.

No behavior change to product code — test-infra only.

Verification

  • ✅ jsdom vitest project — 4/4 pass
  • lint, typecheck, format:check — clean

Clinical Governance Preflight

N/A — test infrastructure only; no ingestion, answer generation, search, privacy, or clinical output touched.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Improved test setup consistency for simulated browser media queries.
    • Ensured scroll behavior spies are reset between tests.
    • Continued validating accessible table rendering and mobile fullscreen expansion.

Addresses the CodeRabbit nitpick from the (superseded) #623 review on the jsdom
test setup that shipped in #636:
- scrollIntoView was assigned once as a plain vi.fn(); vi.restoreAllMocks() only
  restores vi.spyOn spies, so its call history never reset between tests. Ensure a
  base impl exists, then re-spy each test via vi.spyOn so it resets consistently.
- Export installMatchMediaStub and reuse it in accessible-table.dom.test.tsx
  instead of the copy-pasted local setMatchMedia.

Verified: jsdom project 4/4 pass, lint / typecheck / format clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@supabase

supabase Bot commented Jul 14, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6b8b93d2-d634-46e0-bc5e-8174f593793e

📥 Commits

Reviewing files that changed from the base of the PR and between 6f3644c and 8e87160.

📒 Files selected for processing (2)
  • tests/accessible-table.dom.test.tsx
  • tests/setup/jsdom.setup.ts

📝 Walkthrough

Walkthrough

The JSDOM setup now exports a reusable matchMedia stub and resets scrollIntoView spies for each test. Accessible table tests use the shared stub instead of defining a local helper.

Changes

JSDOM test utilities

Layer / File(s) Summary
Update browser API stubs
tests/setup/jsdom.setup.ts
Exports installMatchMediaStub and recreates the scrollIntoView spy before each test.
Reuse matchMedia stub
tests/accessible-table.dom.test.tsx
Replaces the local matchMedia helper with the shared setup utility and removes the unused vi import.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the test-only jsdom changes, including the resettable scrollIntoView spy and shared matchMedia stub.
Description check ✅ Passed The description covers the summary, verification, and governance context, with only minor template omissions like a filled Notes section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/jsdom-setup-cleanup

Comment @coderabbitai help to get the list of available commands.

@BigSimmo
BigSimmo merged commit 983b00b into main Jul 14, 2026
16 checks passed
@BigSimmo
BigSimmo deleted the claude/jsdom-setup-cleanup branch July 14, 2026 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant