Skip to content

test(snapshots): Snapshot the snapshots toolbar#116479

Draft
mtopo27 wants to merge 2 commits into
max/snapshots-harness-ssrfrom
max/snapshots-toolbar-tests
Draft

test(snapshots): Snapshot the snapshots toolbar#116479
mtopo27 wants to merge 2 commits into
max/snapshots-harness-ssrfrom
max/snapshots-toolbar-tests

Conversation

@mtopo27
Copy link
Copy Markdown
Contributor

@mtopo27 mtopo27 commented May 29, 2026

PR 3 of 3 in the stack splitting #115893 — base: #116478.

Adds snapshot coverage for the preprod snapshots toolbar, rendering the real presentational components from `snapshotsToolbar.tsx` (PR 1) via the SSR harness (PR 2).

What changes

  • New `snapshotsToolbar.snapshots.tsx` — 8 snapshots: four content states (`all-controls`, `no-diff-controls`, `solo-base`, `minimal`) × light/dark themes.
  • Only the irreducibly DOM-bound `Tooltip` (portals to `document.body`) and `CompactSelect` (reads `document` for its overlay) stay mocked — both render their visible trigger so the snapshot still reflects production.
  • The test-only `SnapshotsToolbarWithControls` composition helper lives in the test file, mapping a flat props API onto the slot-based `ToolbarContainer`. This keeps the production module free of test-only exports.

Verification

  • `pnpm snapshots snapshotsToolbar` — 8/8 pass.
  • typecheck, `lint:js`, and knip all clean.
  • The Sentry "Snapshot Testing" visual check runs in CI on this PR.

Stack

Add snapshot coverage for the preprod snapshots toolbar across four
content states (all-controls, no-diff-controls, solo-base, minimal) in
both light and dark themes, rendering the real presentational components
from snapshotsToolbar.tsx. Only the irreducibly DOM-bound Tooltip and
CompactSelect (overlay/portal) stay mocked, via the SSR harness.

The SnapshotsToolbarWithControls composition helper lives in the test
file, keeping the production module free of test-only exports.
@mtopo27 mtopo27 requested a review from a team as a code owner May 29, 2026 14:11
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.59%

@mtopo27 mtopo27 marked this pull request as draft May 29, 2026 14:23
Tooltip and CompactSelect can't server-render (Tooltip portals to
document.body, CompactSelect reads document for its overlay), so snapshot
tests mocked them inline — the same passthrough/trigger fakes were
copy-pasted across files and drifting.

Extract them into a shared tests/js/sentry-test/snapshots/mocks/ library:

- Tooltip is mocked globally in snapshot-framework.ts. It's never visible
  in a static snapshot, so a global passthrough is zero coverage loss and
  removes per-file boilerplate. Drops the now-redundant inline mocks from
  snapshotCards and preprodBuildsSnapshotTable.
- CompactSelect stays opt-in (consumed in snapshotsToolbar.snapshots.tsx).
  Its trigger is visible UI, so a future test should be able to snapshot
  the real component rather than always get the mock.

Both are pulled in via real named imports (not requireActual strings) so
knip can track them without a config entry.

Classify static/app/**/*.snapshots.tsx as the test-sentry eslint boundary
element (it only covered *.spec.tsx) so snapshot files may import
sentry-test mocks. This reclassifies badge.snapshots.tsx from core to
test-sentry, turning its deliberate deep core import into a cross-boundary
one that the core entry-point isolation rule flags; add a
boundaries/dependencies disable alongside the no-core-import disable it
already carried on that line.

Co-Authored-By: Claude <noreply@anthropic.com>
};
});

jest.mock('@sentry/scraps/tooltip', () => ({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

benefit of the global tooltip mock, can delete here

),
}));

jest.mock('@sentry/scraps/tooltip', () => ({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

benefit of the global tooltip mock, can delete here

@@ -0,0 +1,5 @@
import type {ReactNode} from 'react';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocker, but should we rename this to "MockedPassthroughComponent" or something since:

  1. it's not specific tooltips
  2. could be used for other components similar to tooltips etc

Idk if there would be another component thatd we'd want to be totally mocked but invisible like this case though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually yea nvm this isnt really a big deal, feel free to ignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants