Add React Testing Library migration infrastructure and first conversions - #385
Merged
Conversation
Groundwork for retiring enzyme (dead upstream, no adapter beyond React
17 - it is the hard blocker for React 18+):
- testUtils/rtlContexts.js: renderWithContexts(), the RTL counterpart of
mountWithContexts. Same context defaults and override mechanism
(i18n, session, config, router with optional custom history), plus a
ready userEvent instance and the history object in the return value.
- @testing-library/jest-dom matchers are loaded globally in setupTests
(individual suites no longer need to import them).
- Seven representative suites converted from enzyme to RTL as the
pattern: ContentEmpty, ContentLoading, AlertModal, FormActionGroup,
ChipGroup, ToolbarSyncSourceButton (interaction) and DeleteButton
(modal open, confirm flow, async error). Where the enzyme versions
only asserted wrapper length or props, the RTL versions assert
rendered behavior; ChipGroup's old test asserted a prop on an empty
render and now renders actual chips and asserts the visible overflow
text.
Conversion recipe:
mountWithContexts(<X/>) -> renderWithContexts(<X/>)
wrapper.find('button').simulate -> await user.click(screen.getByRole('button', { name }))
waitForElement(wrapper, 'X') -> await screen.findBy...
wrapper.find(...).props() asserts -> assert visible text/roles instead
act(...) wrapping -> usually unnecessary (RTL wraps)
545 suites remain on enzyme; they convert incrementally with both
helpers coexisting.
This was referenced Jun 11, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR lays the groundwork for incrementally migrating Ascender’s UI test suite from Enzyme (React 17-bound) to React Testing Library by introducing a shared RTL render helper and converting an initial set of representative component tests.
Changes:
- Added
renderWithContexts()helper to mirror Enzyme’smountWithContexts()provider wiring for RTL tests. - Loaded
@testing-library/jest-dommatchers globally viasetupTests.js. - Converted 7 component test suites from Enzyme to RTL patterns (behavior/DOM assertions +
userEventinteractions).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| awx/ui/testUtils/rtlContexts.js | Adds RTL context render helper used for incremental Enzyme → RTL conversions. |
| awx/ui/src/setupTests.js | Globally registers @testing-library/jest-dom matchers for all UI tests. |
| awx/ui/src/components/PaginatedTable/ToolbarSyncSourceButton.test.js | Converts suite to RTL and userEvent click interaction. |
| awx/ui/src/components/FormActionGroup/FormActionGroup.test.js | Converts to RTL with accessible-button clicks and handler assertions. |
| awx/ui/src/components/DeleteButton/DeleteButton.test.js | Converts modal flow tests to RTL patterns (findByRole, waitFor). |
| awx/ui/src/components/ContentLoading/ContentLoading.test.js | Converts to RTL with role-based assertion. |
| awx/ui/src/components/ContentEmpty/ContentEmpty.test.js | Converts to RTL with visible-text assertion. |
| awx/ui/src/components/ChipGroup/ChipGroup.test.js | Converts to RTL and asserts visible collapsed count text. |
| awx/ui/src/components/AlertModal/AlertModal.test.js | Converts to RTL and asserts dialog/title/body rendering. |
- rtlContexts now loads Lingui plural rules like enzymeHelpers does, behind a guard that degrades cleanly when the Lingui 6 upgrade removes loadLocaleData and the make-plural dependency - adds testUtils/rtlContexts.test.js covering context defaults and overrides, router/history wiring and plural rendering - and extends the jest roots/testMatch to <rootDir>/testUtils, which revealed that enzymeHelpers.test.js had never been executed (the old async-done test style is rejected by modern Jest; both tests fixed, 8 tests revived) - ToolbarSyncSourceButton test queries the button by accessible name - DeleteButton test attaches .response to a real Error so the network error path is actually exercised; fixes the URL typo
Contributor
Author
|
All four review comments addressed: (1) |
Contributor
Author
|
All review feedback on this PR is addressed, and after the latest merges to main (#376, #380, #386, #389, #391, #394) this branch has been re-verified: it merges cleanly into current main (51188f0) and is conflict-free against every other open PR (git merge-tree, all pairs). Ready to merge — in any order relative to #385/#390/#392/#393/#395. |
cigamit
approved these changes
Jun 12, 2026
cigamit
left a comment
cigamit
left a comment
Collaborator
There was a problem hiding this comment.
Tested and working. We do get this warning, but I can remove the file in another PR.
Snapshot Summary
› 1 snapshot file obsolete from 1 test suite. To remove it, re-run jest with -u.
↳ • testUtils/snapshots/enzymeHelpers.test.jsx.snap
blaipr
added a commit
to blaipr/ascender
that referenced
this pull request
Jun 13, 2026
renderWithContexts mounted only the v5 Router, so components migrated to the react-router-dom-v5-compat APIs threw 'useNavigate() may be used only in the context of a <Router>' under the RTL suites that ctrliq#385 introduced (rtlContexts.js did not exist when this branch was cut). Nest the same controlled v6 Router used by enzymeHelpers — location from the v5 context, navigator = the shared history — and add a regression test exercising useNavigate/useLocation from the compat package. Also drops the dead Lingui v5 make-plural guard and fixes the stale import/* eslint rule names, matching the identical cleanup already on the rtl-batch-user branch so the hunks merge cleanly.
cigamit
pushed a commit
that referenced
this pull request
Jun 13, 2026
…tree files) (#392) * Migrate react-router usage to v6 compat APIs (bridge + all non-route-tree files) Combines the v5-compat bridge and the three migration batches into one change covering ~180 of the 237 files that used the router APIs removed in react-router 6/7 (Switch, useHistory, useRouteMatch, Redirect, withRouter). The bridge: - react-router-dom-v5-compat added (router v6 running alongside v5, React 17 compatible) with CompatRouter mounted inside the app's HashRouter, so components migrate file by file - testUtils/enzymeHelpers renders a controlled nested v6 Router (location from the v5 context, navigator = the shared history) inside the v5 Router: no second history subscription, so no act() warnings, and enzyme shallow rendering keeps working - licenses/ui entry for the new dependency (test_licenses enforces it) The migration (components, contexts, hooks and all screen directories): - useHistory() -> useNavigate(); history.push/replace/goBack -> navigate(...) with { replace: true } where applicable; history.go() -> navigate(0) - history.location reads -> useLocation() - zero-argument useRouteMatch used only for .params -> useParams() - conditional Redirect -> Navigate (only in files without a Switch) - withRouter wrappers replaced with hooks, or dropped where the component never read the injected props (AppContainer, Templates, Organizations, Projects) - contexts/Session's history.listen POP detection rewritten with useLocation/useNavigationType, skipping the initial render to keep v5's fire-on-change-only semantics - UIEdit's custom hardReload flag moves to navigation state (v6 drops custom location fields); TopologyView's redirectToDetailsPage helper takes navigate instead of a history object Two correctness findings baked in: - navigate() from the compat package is not referentially stable (its identity changes with the location), so redirect-on-result effects that previously listed the stable v5 history in their deps drop navigate with an explanatory eslint-disable - including it refires the redirect after unrelated navigations (caught by an existing cancel-flow test) - screens/Login migrated as the first slice surfaced a test.only that had silenced 14 Login tests since the initial import; it is removed, two assertions that rotted while dark are fixed, and the suite now runs 2869 tests (14 more than before) Deliberately left on v5 for the final route-tree phase: Switch/Route definitions, Redirect from= entries inside Switch, match.path/match.url route building, and argument-form useRouteMatch prefix checks. After that phase, react-router-dom flips to v6 and the compat package is removed. * Address review: fix resource-param check, consolidate useLocation - JobTemplateAdd: location.search.includes('resource_id' && 'resource_name') only ever tested for resource_name ('a' && 'b' evaluates to 'b'); now requires both params before building resource defaults - PaginatedTable: single useLocation() call instead of two * Address review: fix eslint-disable placement for navigate dep comments The explanation for omitting navigate from effect deps was wrapped onto comment lines after the eslint-disable-next-line directive, which makes the directive target the comment instead of the dependency array. Move the explanation above the directive and keep the directive as a single bare line directly before the code it suppresses. Comment-only change, no runtime impact. * Wire the v5-compat bridge into the RTL test harness renderWithContexts mounted only the v5 Router, so components migrated to the react-router-dom-v5-compat APIs threw 'useNavigate() may be used only in the context of a <Router>' under the RTL suites that #385 introduced (rtlContexts.js did not exist when this branch was cut). Nest the same controlled v6 Router used by enzymeHelpers — location from the v5 context, navigator = the shared history — and add a regression test exercising useNavigate/useLocation from the compat package. Also drops the dead Lingui v5 make-plural guard and fixes the stale import/* eslint rule names, matching the identical cleanup already on the rtl-batch-user branch so the hunks merge cleanly. * Remove the orphaned enzymeHelpers.test.jsx.snap The suite lives in enzymeHelpers.test.js; the .jsx twin snapshot is obsolete and fails the run under jest's ci mode.
cigamit
pushed a commit
that referenced
this pull request
Jun 14, 2026
…ary (#398) * Convert the User screen test suites from Enzyme to React Testing Library Converts all 22 test suites under screens/User/ to RTL, continuing the migration started in #385. Tests drive the real UI through roles and labels instead of reaching into component props. Infrastructure fixes surfaced by the conversion: - setupTests.js: mockument's createRange stub lacks cloneRange and most of the Range API, crashing @testing-library/user-event's pointer handling. Restore jsdom's real Range with the two rect-method stubs the editor components need. - testUtils/rtlContexts.js gains settleTooltips(): a PF Modal close restores focus to a Tooltip-wrapped button, and a tooltip pending at unmount logs a state-update warning into the NEXT test, producing order-dependent failures. Applied in the three list suites that end right after closing an error modal. - testUtils/rtlContexts.js gains assertDetail() shared by the detail suites. - DisassociateButton: drop the invalid ouiaId prop on PF Tooltip that leaked to the DOM (enzyme never opened the tooltip; user-event's real hover exposed the warning). - Delete the orphaned enzymeHelpers.test.jsx.snap behind the perpetual 'snapshot file obsolete' notice. * Build UserForm submit payload from a copy instead of mutating Formik values The submit handler mutated Formik's values object (deleting password, assigning is_superuser/is_system_auditor). Deleting password flips the still-mounted password field from controlled to uncontrolled after a successful submit, which React warns about — surfaced once the merged react-router-dom-v5-compat bridge drives a re-render of the mounted form on navigate(). Destructure into a fresh submitValues and mutate only the copy; Formik's own state is left intact.
This was referenced Jun 16, 2026
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.
SUMMARY
Stage 3 groundwork of the React stack modernization: start retiring enzyme, which is dead upstream and has no adapter beyond React 17 — it is the single hard blocker for ever moving to React 18/19 (495 test files depend on it).
This PR makes incremental conversion possible and establishes the pattern:
testUtils/rtlContexts.js—renderWithContexts(), the React Testing Library counterpart ofmountWithContexts: same context defaults and override mechanism (i18n, session, config, router with optional custom history), plus a readyuserEventinstance and thehistoryobject in the return value.@testing-library/jest-dommatchers loaded globally insetupTests.js(suites no longer import them individually; existing per-file imports keep working).Conversion recipe
mountWithContexts(<X/>)renderWithContexts(<X/>)wrapper.find('button').simulate('click')await user.click(screen.getByRole('button', { name }))waitForElement(wrapper, 'X')await screen.findBy....props()/.lengthact(...)wrappingBoth helpers coexist; the remaining ~490 enzyme suites convert incrementally (screen directory at a time works well). When the last one converts, enzyme + the React 17 adapter come out of package.json — unblocking React 18.
Independent of all open PRs — test-only changes (no production code, no dependency changes). Note for the future: when the react-router compat migration (#392) merges,
rtlContextsshould gain the same controlled v6 Router layer its enzyme counterpart gets there — a 5-line follow-up.ISSUE TYPE
COMPONENT NAME
ASCENDER VERSION
ADDITIONAL INFORMATION
Production build unaffected (test files and test utilities only).