CONSOLE-5176: Refactor RTL Tests from fireEvent to userEvent

Author: cajieh

.claude/skills/gen-rtl-test/SKILL.md

@@ -59,7 +59,7 @@ RTL emphasizes testing components as users interact with them. Users find button
 
 4. **DRY Helpers** - Use reusable function in frontend/packages/console-shared/src/test-utils directoty and sub-directory if exists else extract repetitive setup into reusable functions
 
-5. **Async-Aware** - Handle asynchronous updates with `findBy*` and `waitFor`
+5. **Async-Aware** - Prefer `findByText` / `findByRole` / `findAllByRole` when waiting for content to **appear**; use `waitFor` when the assertion is **not** expressible as a query (e.g. button **disabled** after validation, **absence**, class names, mocks). See **Rule 11**.
 
 6. **TypeScript Safety** - Use proper types for props, state, and mock data
 
@@ -360,7 +360,7 @@ it('should find the heading', () => {
 **Query Variants:**
 - **`getBy*`** - Element expected to be present synchronously (throws if not found)
 - **`queryBy*`** - Only for asserting element is NOT present
-- **`findBy*`** - Element will appear asynchronously (returns Promise)
+- **`findBy*`** - Element will appear asynchronously (returns Promise). Prefer **`findBy*`** over **`waitFor(() => … getBy* …)`** when the goal is to wait for that node to **appear** (see **Rule 11**).
 
 **Anti-pattern:** Avoid `container.querySelector` - it tests implementation details.
 
@@ -456,7 +456,8 @@ it('should render the Name field', async () => {
   // Don't do this - use verifyInputField instead!
   expect(screen.getByLabelText('Name')).toBeInTheDocument();
   expect(screen.getByLabelText('Name')).toHaveValue('');
-  fireEvent.change(screen.getByLabelText('Name'), { target: { value: 'test' } });
+  const user = userEvent.setup();
+  await user.type(screen.getByLabelText('Name'), 'test');
   expect(screen.getByLabelText('Name')).toHaveValue('test');
   expect(screen.getByText('Unique name for the resource')).toBeInTheDocument();
 });
@@ -482,14 +483,15 @@ When testing form components:
 ### Rule 10: Test Conditional Rendering by Asserting Both States
 
 ```typescript
-it('should show content when expanded', () => {
+it('should show content when expanded', async () => {
+  const user = userEvent.setup();
   render(<Collapsible />);
 
   // 1. Assert initial hidden state
   expect(screen.queryByText('Hidden content')).not.toBeInTheDocument();
 
   // 2. Simulate user action
-  fireEvent.click(screen.getByRole('button', { name: 'Expand' }));
+  await user.click(screen.getByRole('button', { name: 'Expand' }));
 
   // 3. Assert final visible state
   expect(screen.getByText('Hidden content')).toBeVisible();
@@ -498,18 +500,54 @@ it('should show content when expanded', () => {
 
 ### Rule 11: Handle Asynchronous Behavior
 
+#### Prefer `findBy*` over `waitFor` + `getBy*` when waiting for content to appear
+
+`findByText`, `findByRole`, `findAllByRole`, etc. are implemented as **`waitFor` + `getBy*`** (they return a Promise and retry until the element is found or the timeout is reached). When the intent is **“wait until this text/role appears in the DOM,”** use **`await screen.findBy…`** instead of wrapping **`getBy…`** in **`waitFor`**.
+
 ```typescript
-// Use findBy* to wait for an element to appear
-const element = await screen.findByText('Loaded content');
-expect(element).toBeVisible();
+// ✅ GOOD: findBy* expresses “eventually this appears”
+expect(await screen.findByText('Loaded content')).toBeVisible();
+const row = await screen.findByRole('row', { name: /my-resource/i });
+expect(await screen.findAllByRole('button', { name: 'Remove' })).toHaveLength(2);
 
-// Use waitFor for complex assertions
+// ⚠️ EQUIVALENT but more verbose (prefer findBy* above)
 await waitFor(() => {
-  expect(screen.getByText('Updated')).toBeInTheDocument();
+  expect(screen.getByText('Loaded content')).toBeInTheDocument();
 });
 ```
 
-**Avoid Explicit act():** Rarely needed. `render`, `fireEvent`, `findBy*`, and `waitFor` already wrap operations in `act()`.
+#### Keep `waitFor` when `findBy*` cannot express the assertion
+
+**`findBy*` queries wait for presence** of a matching node. They do **not** replace every `waitFor`. Still use **`waitFor`** when you need to wait for something that is **not** “find this text/role in the tree,” for example:
+
+| Situation | Why `findBy*` is not enough | Pattern |
+|-----------|------------------------------|---------|
+| **Control state after async validation** (e.g. Save **disabled** after Formik/yup runs) | Disabled is a **property**, not a new queryable label; there is no `findBy…` for “button became disabled.” | `await waitFor(() => expect(save).toBeDisabled());` — you can still use **`findByText`** / **`findByRole`** in the **same** test for **error messages** that appear as text. |
+| **Eventually absent** (list/link/control **not** in the document) | **`findBy*` waits for presence**, not absence. | `await waitFor(() => expect(screen.queryByRole('list')).not.toBeInTheDocument());` or `waitForElementToBeRemoved` when something was visible first. |
+| **CSS class** or **non-query** DOM state | Not a text/role query. | `await waitFor(() => expect(input).toHaveClass('invalid-tag'));` |
+| **Mock** or **callback** assertions | Not the DOM. | `await waitFor(() => expect(mockFn).toHaveBeenCalledWith(…));` |
+
+```typescript
+// Async validation: message appears → findByText; button disabled → waitFor
+await user.type(cpuRequest, '300');
+await waitFor(() => expect(save).toBeDisabled());
+expect(
+  await screen.findByText('CPU request must be less than or equal to limit.'),
+).toBeVisible();
+```
+
+#### Quick reference
+
+```typescript
+// Prefer findBy* when waiting for content to appear
+const element = await screen.findByText('Loaded content');
+expect(element).toBeVisible();
+
+// Use waitFor for disabled state, absence, classes, mocks (see table above)
+await waitFor(() => expect(save).toBeDisabled());
+```
+
+**Avoid Explicit act():** Rarely needed. `render`, `userEvent` (await its async methods), `findBy*`, and `waitFor` already wrap operations in `act()`.
 
 ### Rule 12: Use Lifecycle Hooks for Setup and Cleanup
 
@@ -553,24 +591,31 @@ expect(userName).toBeVisible();
 expect(editButton).toBeVisible();
 ```
 
-### Rule 14: Simulate User Events with fireEvent
+### Rule 14: Simulate User Events with userEvent
+
+Use `@testing-library/user-event` for clicks, typing, and other interactions. It simulates full event sequences closer to real browser behavior than low-level `fireEvent`.
 
 ```typescript
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 
+const user = userEvent.setup();
 render(<MyForm />);
 
 const input = screen.getByLabelText(/name/i);
 const button = screen.getByRole('button', { name: /submit/i });
 
-// Simulate typing
-fireEvent.change(input, { target: { value: 'John Doe' } });
+// Simulate typing (prefer over fireEvent.change)
+await user.type(input, 'John Doe');
 
 // Simulate clicking
-fireEvent.click(button);
+await user.click(button);
 ```
 
-**Note:** `userEvent` from `@testing-library/user-event` is not supported due to incompatible Jest version (will be updated after Jest upgrade).
+**Conventions:**
+- Call `const user = userEvent.setup()` per test and **await** `user.click`, `user.type`, `user.keyboard`, etc.
+- For text inputs in forms, prefer **`verifyInputField`** (Rule 9) when it applies; use `userEvent` for other controls (selects, checkboxes, buttons, complex widgets).
+- Reserve **`fireEvent`** only for rare cases (e.g., triggering a specific low-level DOM event that `userEvent` does not model).
 
 ### Rule 15: Test "Unhappy Paths" and Error States
 
@@ -648,12 +693,14 @@ Remove any imports that are not used in the test file:
 
 ```typescript
 // ❌ BAD - Unused imports
-import { render, screen, fireEvent, waitFor, within } from '@testing-library/react';
+import { render, screen, waitFor, within } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import { k8sCreate, k8sPatch, k8sUpdate } from '@console/internal/module/k8s';
-// ... but only using render, screen, fireEvent
+// ... but only using render, screen, userEvent
 
 // ✅ GOOD - Only what's needed
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import { k8sCreate } from '@console/internal/module/k8s';
 ```
 
@@ -717,23 +764,25 @@ Clean up any variables that are declared but never used:
 
 ```typescript
 // ❌ BAD - Unused variables
-it('should submit form', () => {
+it('should submit form', async () => {
   const mockData = { foo: 'bar' };
   const unusedSpy = jest.spyOn(console, 'log');
   const onSubmit = jest.fn();
+  const user = userEvent.setup();
 
   render(<Form onSubmit={onSubmit} />);
-  fireEvent.click(screen.getByRole('button'));
+  await user.click(screen.getByRole('button'));
 
   expect(onSubmit).toHaveBeenCalled();
 });
 
 // ✅ GOOD - Only necessary variables
-it('should submit form', () => {
+it('should submit form', async () => {
+  const user = userEvent.setup();
   const onSubmit = jest.fn();
 
   render(<Form onSubmit={onSubmit} />);
-  fireEvent.click(screen.getByRole('button'));
+  await user.click(screen.getByRole('button'));
 
   expect(onSubmit).toHaveBeenCalled();
 });
@@ -950,43 +999,45 @@ act(() => {
 
 **Strategy 1: Wrap async interactions in waitFor**
 ```typescript
+import { screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+const user = userEvent.setup();
+
 // ❌ BAD: Causes act() warning
-fireEvent.click(button);
+await user.click(button);
 expect(screen.getByText('Updated')).toBeInTheDocument();
 
 // ✅ GOOD: Use waitFor for async updates
-fireEvent.click(button);
+await user.click(button);
 await waitFor(() => {
   expect(screen.getByText('Updated')).toBeInTheDocument();
 });
 ```
 
 **Strategy 2: Use findBy* queries (preferred for new elements)**
-```typescript
-// ❌ BAD: Causes act() warning
-fireEvent.click(button);
-expect(screen.getByText('Loaded')).toBeInTheDocument();
 
-// ✅ GOOD: Use findBy* which waits automatically
-fireEvent.click(button);
-expect(await screen.findByText('Loaded')).toBeInTheDocument();
-```
+When waiting for content to appear after user interactions, prefer `findBy*` queries which automatically wait for elements. See **Rule 11** for comprehensive guidance on `findBy*` vs `waitFor`, including when to use each and a detailed comparison table.
 
-**Strategy 3: Wrap test in act() when needed**
+**Strategy 3: Prefer userEvent + findBy* for dropdown/select-style interactions**
 ```typescript
-// Import act from @testing-library/react
-import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
+import { screen, act } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+const user = userEvent.setup();
+
+// ❌ BAD: Asserts before async menu content appears
+await user.click(screen.getByText('Select Option'));
+expect(screen.getByText('Option 1')).toBeInTheDocument();
 
-// ❌ BAD: Causes act() warning for dropdown/select interactions
-const dropdown = screen.getByText('Select Option');
-fireEvent.click(dropdown);
+// ✅ GOOD: Wait for option, then click
+await user.click(screen.getByText('Select Option'));
+await user.click(await screen.findByText('Option 1'));
 
-// ✅ GOOD: Wrap in act() for complex interactions
+// If a third-party widget still warns, wrap only that interaction in act():
 await act(async () => {
-  fireEvent.click(dropdown);
+  await user.click(screen.getByRole('button', { name: /toggle menu/i }));
 });
-const option = await screen.findByText('Option 1');
-fireEvent.click(option);
 ```
 
 **Strategy 4: Mock timers or async operations**
@@ -1004,7 +1055,7 @@ await waitFor(() => {
 #### Common Causes of act() Warnings
 
 1. **Dropdown/Select interactions** - PatternFly Select/Dropdown components
-   - Solution: Wrap in `act()` or use `waitFor` after interaction
+   - Solution: Use `await user.click` / `user.type` with `findBy*` or `waitFor` after opening; wrap in `act()` only if a widget still emits warnings
 
 2. **Async state updates** - useEffect, setTimeout, promises
    - Solution: Use `findBy*` or `waitFor`
@@ -1455,11 +1506,13 @@ When generating tests for React components:
 4. ✅ Mock factories return simple values (null, strings, children) - NO React.createElement
 5. ✅ Cast mocked imports to `jest.Mock` when calling `.mockResolvedValue()` etc.
 6. ✅ **Import `verifyInputField` for form components** - Rule 9 strictly enforced
+7. ✅ **Use `userEvent` from `@testing-library/user-event` for clicks, typing, and similar interactions** - Rule 14 (`userEvent.setup()`, then `await user.click` / `await user.type`, etc.)
 
 **Code Generation Pattern:**
 ```typescript
 // ✅ ALWAYS - ES6 imports at file top
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import { k8sCreate } from '@console/internal/module/k8s';
 import { history } from '@console/internal/components/utils';
 import { verifyInputField } from '@console/shared/src/test-utils/unit-test-utils'; // For form components
@@ -1584,7 +1637,7 @@ If **ANY** `require()` found that is NOT `jest.requireActual` → **IMMEDIATELY
 - Fix **EVERY** act() warning using Rule 23 strategies:
   - Wrap async interactions in `waitFor`
   - Use `findBy*` queries for async elements
-  - Add `await waitFor()` after dropdown/select interactions
+  - After `await user.click` on dropdowns/selects, wait for menu content with `findBy*` or `waitFor`
   - Ensure async state updates are properly awaited
 - Re-run tests after fixing warnings
 - **DO NOT** complete until output has ZERO act() warnings

frontend/packages/console-app/src/__tests__/hooks/useCSPViolationDetector.spec.tsx

@@ -1,4 +1,4 @@
-import { act, fireEvent } from '@testing-library/react';
+import { act } from '@testing-library/react';
 import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
 import {
   newPluginCSPViolationEvent,
@@ -78,7 +78,7 @@ describe('useCSPViolationDetector', () => {
     mockCacheEvent.mockReturnValue(true);
     renderWithProviders(<TestComponent />);
     act(() => {
-      fireEvent(document, testEvent);
+      document.dispatchEvent(testEvent);
     });
     expect(mockCacheEvent).toHaveBeenCalledWith(testPluginEvent);
     expect(mockFireTelemetry).toHaveBeenCalledWith('CSPViolation', testPluginEvent);
@@ -89,7 +89,7 @@ describe('useCSPViolationDetector', () => {
     renderWithProviders(<TestComponent />);
 
     act(() => {
-      fireEvent(document, testEvent);
+      document.dispatchEvent(testEvent);
     });
 
     expect(mockCacheEvent).toHaveBeenCalledWith(testPluginEvent);
@@ -104,7 +104,7 @@ describe('useCSPViolationDetector', () => {
     const expected = newPluginCSPViolationEvent('foo', testEventWithPlugin);
     renderWithProviders(<TestComponent />);
     act(() => {
-      fireEvent(document, testEventWithPlugin);
+      document.dispatchEvent(testEventWithPlugin);
     });
     expect(mockCacheEvent).toHaveBeenCalledWith(expected);
     expect(mockFireTelemetry).toHaveBeenCalledWith('CSPViolation', expected);
@@ -119,7 +119,7 @@ describe('useCSPViolationDetector', () => {
     const expected = newPluginCSPViolationEvent('foo', testEventWithPlugin);
     renderWithProviders(<TestComponent />);
     act(() => {
-      fireEvent(document, testEventWithPlugin);
+      document.dispatchEvent(testEventWithPlugin);
     });
     expect(mockCacheEvent).toHaveBeenCalledWith(expected);
     expect(mockFireTelemetry).toHaveBeenCalledWith('CSPViolation', expected);

frontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsx

@@ -1,5 +1,6 @@
 import type { ComponentProps } from 'react';
-import { screen, fireEvent } from '@testing-library/react';
+import { screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import type { FormikProps, FormikValues } from 'formik';
 import { formikFormProps } from '@console/shared/src/test-utils/formik-props-utils';
 import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
@@ -70,16 +71,18 @@ describe('ResourceLimitsModal Form', () => {
   });
 
   it('calls the cancel function when the Cancel button is clicked', async () => {
+    const user = userEvent.setup();
     renderWithProviders(<ResourceLimitsModal {...formProps} />);
 
-    await fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+    await user.click(screen.getByRole('button', { name: 'Cancel' }));
     expect(formProps.cancel).toHaveBeenCalledTimes(1);
   });
 
   it('calls the handleSubmit function when the form is submitted', async () => {
+    const user = userEvent.setup();
     renderWithProviders(<ResourceLimitsModal {...formProps} />);
 
-    await fireEvent.submit(screen.getByRole('form'));
+    await user.click(screen.getByRole('button', { name: 'Save' }));
     expect(formProps.handleSubmit).toHaveBeenCalledTimes(1);
   });
 });

frontend/packages/console-shared/src/components/alerts/__tests__/SwitchToYAMLAlert.spec.tsx

@@ -1,4 +1,5 @@
-import { screen, fireEvent } from '@testing-library/react';
+import { screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import { renderWithProviders } from '../../../test-utils/unit-test-utils';
 import SwitchToYAMLAlert from '../SwitchToYAMLAlert';
 
@@ -42,12 +43,13 @@ describe('SwitchToYAMLAlert', () => {
     expect(closeButton).toBeInTheDocument();
   });
 
-  it('should call onClose when close button is clicked', () => {
+  it('should call onClose when close button is clicked', async () => {
+    const user = userEvent.setup();
     const mockOnClose = jest.fn();
     renderWithProviders(<SwitchToYAMLAlert onClose={mockOnClose} />);
 
     const closeButton = screen.getByRole('button', { name: /close/i });
-    fireEvent.click(closeButton);
+    await user.click(closeButton);
 
     expect(mockOnClose).toHaveBeenCalledTimes(1);
   });