Skip to content

enzyme -> RTL: convert the CredentialType screen suites - #433

Merged
cigamit merged 2 commits into
ctrliq:mainfrom
blaipr:feature/rtl-batch-credential-type
Jun 16, 2026
Merged

enzyme -> RTL: convert the CredentialType screen suites#433
cigamit merged 2 commits into
ctrliq:mainfrom
blaipr:feature/rtl-batch-credential-type

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/CredentialType suites. This follows the infrastructure in #385 and the first directory batch (screens/User) in #398.

  • Converts all six enzyme suites in screens/CredentialType to renderWithContexts: CredentialTypeListItem, CredentialTypeList, CredentialTypeDetails, CredentialTypeAdd, CredentialTypeEdit, and the shared CredentialTypeForm.
  • The container suites (Add/Edit) stub the shared form and drive its onSubmit/onCancel/submitError props — the same black-box treatment the enzyme tests used (the form has its own suite).
  • Delete flows now interact for real. PF4's Modal aria-hides the rest of the tree under jsdom, so the confirm button is queried by label and clicked via fireEvent (a reusable pattern for the remaining directories).
  • Drops one prop-inspection-only assertion (ToolbarDeleteButton deleteDetailsRequests count) that has no DOM-observable behavior.

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

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • UI
ASCENDER VERSION
awx: 25.4.1.dev49+g0e26da4e2e
ADDITIONAL INFORMATION
  • All 33 tests in screens/CredentialType 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 six enzyme test suites in screens/CredentialType to React
Testing Library via renderWithContexts (continuing the step-3 directory
conversions after screens/User in ctrliq#398):

- CredentialTypeListItem, CredentialTypeList, CredentialTypeDetails,
  CredentialTypeAdd, CredentialTypeEdit, and the shared CredentialTypeForm.
- Container suites (Add/Edit) stub the shared form and drive its
  onSubmit/onCancel/submitError props, matching how the enzyme tests treated
  the form as a unit (it has its own suite).
- Delete flows interact for real; PF4's Modal aria-hides the tree under
  jsdom, so the confirm button is queried by label and clicked via fireEvent.
- Drop a prop-inspection-only assertion (ToolbarDeleteButton
  deleteDetailsRequests count) that has no DOM-observable behavior.

33 tests pass; screens/CredentialType 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/CredentialType test suites to use renderWithContexts/RTL queries and interactions, including PatternFly modal delete-confirm flows.

Changes:

  • Converted the CredentialType screen test suites (list, list item, details, add, edit, shared form) from Enzyme helpers to RTL via renderWithContexts.
  • Updated delete-confirm interactions to work with PatternFly modal aria-hidden behavior in jsdom (query by aria-label + fireEvent where needed).
  • Simplified/modernized test data fixtures and removed Enzyme-only assertions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
awx/ui/src/screens/CredentialType/shared/CredentialTypeForm.test.js Converts shared form suite to RTL and updates interaction assertions.
awx/ui/src/screens/CredentialType/CredentialTypeList/CredentialTypeListItem.test.js Converts list item suite to RTL queries/assertions.
awx/ui/src/screens/CredentialType/CredentialTypeList/CredentialTypeList.test.js Converts list suite to RTL, including real delete-confirm modal interactions.
awx/ui/src/screens/CredentialType/CredentialTypeEdit/CredentialTypeEdit.test.js Converts edit container suite to RTL; stubs shared form to drive submit/cancel/error paths.
awx/ui/src/screens/CredentialType/CredentialTypeDetails/CredentialTypeDetails.test.js Converts details suite to RTL; uses shared assertDetail and updates delete-confirm interaction.
awx/ui/src/screens/CredentialType/CredentialTypeAdd/CredentialTypeAdd.test.js Converts add container suite to RTL; stubs shared form to drive submit/cancel/error paths.
awx/ui/src/screens/CredentialType/CredentialType.test.js Minor comment wording tweak related to route param typing.

Comment on lines +77 to +81
const { user, container } = renderForm();
const nameField = container.querySelector('#credential-type-name');
await user.clear(nameField);
await user.type(nameField, 'Foo');
expect(nameField).toHaveValue('Foo');
Comment on lines +19 to +25
<CredentialTypeListItem
credentialType={credential_type}
detailUrl="credential_types/1/details"
isSelected={false}
onSelect={() => {}}
{...props}
/>
renderItem();
expect(screen.getByText('Foo')).toBeInTheDocument();
expect(screen.getByLabelText('Edit credential type')).toBeInTheDocument();
expect(screen.getByRole('checkbox')).not.toBeChecked();
Comment on lines +59 to +61
const checkboxes = screen.getAllByRole('checkbox');
await user.click(checkboxes[1]);
expect(checkboxes[1]).toBeChecked();
Comment on lines +94 to +96
await user.click(screen.getAllByRole('checkbox')[1]);
await user.click(screen.getByRole('button', { name: 'Delete' }));
fireEvent.click(await screen.findByLabelText('confirm delete'));
@cigamit

cigamit commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Copilot had a few suggestions but otherwise this seems to test out just fine. Can approve after you resolve or discard the suggestions above.

- CredentialTypeForm: the value-update test now exercises both Name and
  Description and asserts the inputs exist before typing.
- CredentialTypeListItem: pass rowIndex so the selection checkbox has a
  stable accessible name, and assert it by name ('Select row 0').
- CredentialTypeList: select row checkboxes by accessible name
  ('Select row 0') instead of a brittle getAllByRole('checkbox') index.
@blaipr

blaipr commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the Copilot suggestions:

  • CredentialTypeForm value-update test now exercises both Name and Description and asserts the inputs exist before typing.
  • CredentialTypeListItem passes rowIndex and asserts the selection checkbox by its accessible name (Select row 0).
  • CredentialTypeList selects row checkboxes by accessible name instead of a positional getAllByRole('checkbox') index.

All 33 tests in screens/CredentialType still pass.

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

Convert the four enzyme test suites in screens/NotificationTemplate to React
Testing Library via renderWithContexts (continuing step 3 after #398/#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.

* Address Copilot review comments
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 pushed a commit that referenced this pull request Jun 18, 2026
* enzyme -> RTL: convert the Template screen suites

Convert all 47 enzyme test suites under screens/Template to React Testing
Library (the last remaining screens/ directory), matching the
renderWithContexts + screen/fireEvent/waitFor pattern of the prior
conversions (#398, #433-#453).

Covers: Templates / Template / WorkflowJobTemplate + TemplateSurvey, the
JobTemplate and WorkflowJobTemplate Add/Edit/Detail screens, the shared
JobTemplateForm / WorkflowJobTemplateForm / WebhookSubForm, the Survey editor
(list/item/question add-edit-form/reorder/toolbar/multiple-choice), and the
entire WorkflowJobTemplateVisualizer tree (graph/node/link/start/toolbar, the
link and node modals, and the node-type resource lists).

d3/SVG and PatternFly portals are asserted via element ids,
data-cy / data-ouia-component-id, foreignObject content and accessible names
rather than geometry or component-name lookups. Two interactions that are
infeasible in jsdom (native drag reorder; one visualizer save-error scenario
that passes in the browser) are kept as documented skips.

48 suites / 257 tests pass (2 documented skips); no production code changed.

* Address Copilot review feedback on PR #483 RTL tests

Remove the duplicated Delete All dispatch test; fix the 'incrimented' and 'survery' test-name typos.

* Remove accidentally committed node_modules symlink

awx/ui/node_modules was committed as a self-referential symlink; .gitignore only excludes the directory contents, not the symlink itself. Untrack it so checkout doesn't clobber a real node_modules.

* Cede the 3 Template entry/detail test files to PR #484

Templates.test.js, Template.test.js and WorkflowJobTemplate.test.js are the only files this PR shared with #484 (Templates route tree -> v6). #484 now owns and RTL-converts those three (mounted for v6). Revert them here so the two PRs touch disjoint files and are independently mergeable in either order; this PR still converts the rest of screens/Template to RTL.
@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