Skip to content

enzyme -> RTL: convert the NotificationTemplate screen suites - #434

Merged
cigamit merged 2 commits into
ctrliq:mainfrom
blaipr:feature/rtl-batch-notification-template
Jun 18, 2026
Merged

enzyme -> RTL: convert the NotificationTemplate screen suites#434
cigamit merged 2 commits into
ctrliq:mainfrom
blaipr:feature/rtl-batch-notification-template

Conversation

@blaipr

@blaipr blaipr commented Jun 16, 2026

Copy link
Copy Markdown
Contributor
SUMMARY

Continue the enzyme → React Testing Library migration (step 3 of the React modernization) by converting the screens/NotificationTemplate suites. Follows the infrastructure in #385, the screens/User batch (#398), and screens/CredentialType (#433).

  • Converts the four enzyme suites to renderWithContexts: NotificationTemplateListItem, NotificationTemplateList, NotificationTemplateDetail, and the shared NotificationTemplateForm.
  • Bulk delete uses the established PF4-modal-in-jsdom pattern (confirm by label + fireEvent).
  • The "test notification" toast is asserted by its title text, since the PF AlertGroup toast does not expose role="alert"; the poll-and-toast chain (test()setTimeoutreadDetailonAddToast) is drained with jest.runAllTimersAsync under fake timers.
  • In the form suite, the react-ace editors are stubbed to render their value (jsdom cannot query ace content), OrganizationLookup's async fetch is settled in act, and secret-revert is covered for all six notification types.

No application code changes — test-only. screens/NotificationTemplate no longer references enzyme.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • UI
ASCENDER VERSION
awx: 25.4.1.dev49+g717565d78d
ADDITIONAL INFORMATION
  • All 31 tests in screens/NotificationTemplate pass (npm test).
  • Test files are not linted (ignored by the eslint flat config) and there are no behavior changes, so no browser smoke-test is required for this test-only batch.
  • Part of the step-3 directory-by-directory conversion; both enzyme and RTL helpers coexist until the last enzyme import is removed.

Convert the four enzyme test suites in screens/NotificationTemplate to React
Testing Library via renderWithContexts (continuing step 3 after ctrliq#398/ctrliq#433):

- NotificationTemplateListItem, NotificationTemplateList,
  NotificationTemplateDetail, and the shared NotificationTemplateForm.
- Bulk-delete uses the PF4-modal-in-jsdom pattern (confirm by label +
  fireEvent). The test-notification toast asserts on its title text since the
  PF AlertGroup toast has no role=alert.
- The poll-and-toast flow uses jest.runAllTimersAsync to drain the
  test() -> setTimeout -> readDetail -> onAddToast chain under fake timers.
- The form's react-ace editors are stubbed to render their value (jsdom
  cannot query ace content), and OrganizationLookup's async fetch is settled
  in act; secret-revert is covered for all six notification types.

31 tests pass; screens/NotificationTemplate no longer references enzyme.

Copilot AI left a comment

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.

Pull request overview

Continues the Enzyme → React Testing Library migration by converting the screens/NotificationTemplate test suites to RTL patterns (renderWithContexts, screen, userEvent, timer flushing), with no production UI code changes.

Changes:

  • Converted NotificationTemplateForm, list, list-item, and detail suites from Enzyme helpers to RTL helpers/queries.
  • Updated interaction patterns for PF modal confirmation and async toast polling (fake timers + runAllTimersAsync).
  • Added test stubs for CodeEditor rendering to make editor values assertable in jsdom.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
awx/ui/src/screens/NotificationTemplate/shared/NotificationTemplateForm.test.js Migrates the shared form tests to RTL, including CodeEditor stubbing and secret-revert coverage.
awx/ui/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.test.js Migrates list item row/action tests (copy/test notification) to RTL.
awx/ui/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.js Migrates list screen tests (load/select/delete/add/toast) to RTL; includes modal confirm + timer-driven toast chain.
awx/ui/src/screens/NotificationTemplate/NotificationTemplateDetail/NotificationTemplateDetail.test.js Migrates detail view assertions to RTL and uses shared assertDetail helper.

@blaipr

blaipr commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. The current revision of this branch already uses NotificationTemplatesAPI for the list load, delete, and capabilities paths (there is no OrganizationsAPI left in the suite), the test-notification test awaits the rows before clicking, and assertCommonDetails reads the name and description off the mock object. These points are covered.

@cigamit
cigamit merged commit af0760a into ctrliq:main Jun 18, 2026
cigamit pushed a commit that referenced this pull request Jun 18, 2026
* enzyme -> RTL: convert the ExecutionEnvironment screen suites

Convert the eight enzyme test suites in screens/ExecutionEnvironment to React
Testing Library via renderWithContexts (continuing step 3 after #433/#434):

- ListItem, List, Details, Add, Edit, the EE-template sub-list (List +
  ListItem), and the shared ExecutionEnvironmentForm.
- Add/Edit stub the shared form and drive its onSubmit/onCancel/submitError
  props (Add also exposes the query-param image prefill).
- Details/List delete flows use the PF4-modal-in-jsdom confirm-by-label +
  fireEvent pattern; for Details the DeleteButton related-count fetch is
  short-circuited with an empty request list so the confirm modal opens.
- Form disabled-state assertions target the field inputs and each Lookup's
  search button by ouiaId; drop two prop-inspection-only delete-detail-count
  assertions with no DOM-observable behavior.

43 tests pass; screens/ExecutionEnvironment no longer references enzyme.

* Address Copilot review comments
@cigamit cigamit self-assigned this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants