Skip to content

Fixes #39477 - Replace enzyme test in ImpersonateIcon with RTL#11062

Open
kfamilonidis wants to merge 1 commit into
theforeman:developfrom
kfamilonidis:pf5-impersonate-icon
Open

Fixes #39477 - Replace enzyme test in ImpersonateIcon with RTL#11062
kfamilonidis wants to merge 1 commit into
theforeman:developfrom
kfamilonidis:pf5-impersonate-icon

Conversation

@kfamilonidis

Copy link
Copy Markdown
Contributor

Fixes #39477: Replace enzyme test in ImpersonateIcon with RTL

@Lukshio

Lukshio commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Real improvement over the old snapshot - now tests actual click interactions (open modal, Confirm calls stopImpersonating) via userEvent, instead of a static render snapshot.

Critical (must fix before merge)

  • None

Suggestions (should fix)

  • The Tooltip mock (Tooltip: ({ children }) => children) drops the tooltip content entirely, so the tooltip text ("You are impersonating another user, click to stop the impersonation...") is no longer covered at all - the old snapshot did verify this text. PR Fixes #39492 - Replace enzyme test in SettingsTable, SettingsCell, SettingsName with RTL #11084 in this same batch (SettingCell/SettingName) mocks Tooltip while preserving content in a data-testid="tooltip-content" node - recommend using that same pattern here so the tooltip copy stays under test.
  • Commit message uses Fixes #39477: ... - the documented convention (see develop history, e.g. Fixes #38653 - Fix settings show endpoint) is Fixes #XXXXX - description with a hyphen, not a colon.

Nice to have (optional)

  • Consider adding a "Cancel" button test (modal closes without calling stopImpersonating) alongside the existing "Confirm" test, for symmetry.
  • The clickable element is a bare <span onClick> with no button role/keyboard handler (pre-existing, not introduced by this PR) - worth a follow-up accessibility ticket, not blocking here.

Checklist gaps

  • Tooltip content assertion dropped (see above).
  • Commit message hyphen convention.

@kfamilonidis
kfamilonidis force-pushed the pf5-impersonate-icon branch from 0fda920 to 5b6d2a2 Compare July 14, 2026 13:16
@kfamilonidis

Copy link
Copy Markdown
Contributor Author

@Lukshio I switched the Tooltip mock from ({ children }) => children (which discarded content) to the content-preserving SettingCell pattern: it renders content into a data-testid="tooltip-content" with its children

@kfamilonidis
kfamilonidis force-pushed the pf5-impersonate-icon branch from 5b6d2a2 to c46142f Compare July 14, 2026 14:03
@kfamilonidis kfamilonidis changed the title Fixes #39477: Replace enzyme test in ImpersonateIcon with RTL Fixes #39477 - Replace enzyme test in ImpersonateIcon with RTL Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants