Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 106 additions & 41 deletions .claude/skills/gen-rtl-test/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
Expand All @@ -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

Expand Down Expand Up @@ -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 (or shared setup) 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

Expand Down Expand Up @@ -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';
```

Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -950,43 +999,57 @@ 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
import { screen } 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('Loaded')).toBeInTheDocument();

// ✅ GOOD: Use findBy* which waits automatically
fireEvent.click(button);
await user.click(button);
expect(await screen.findByText('Loaded')).toBeInTheDocument();
```

**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**
Expand All @@ -1004,7 +1067,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`
Expand Down Expand Up @@ -1455,11 +1518,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
Expand Down Expand Up @@ -1584,7 +1649,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
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -89,7 +89,7 @@ describe('useCSPViolationDetector', () => {
renderWithProviders(<TestComponent />);

act(() => {
fireEvent(document, testEvent);
document.dispatchEvent(testEvent);
});

expect(mockCacheEvent).toHaveBeenCalledWith(testPluginEvent);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
});
Expand Down
Loading