diff --git a/plugins/pf-react/README.md b/plugins/pf-react/README.md index 6c4c930..0b596e5 100644 --- a/plugins/pf-react/README.md +++ b/plugins/pf-react/README.md @@ -26,6 +26,15 @@ Skills are tasks that produce a result. **Unit Test Generator** (`/pf-react:unit-test-generator`) — Generates a complete unit test file for a given React component, following Testing Library best practices. +**PF Library Test Writer** (`/pf-react:pf-library-test-writer`) — Writes unit tests for components and features across the PatternFly ecosystem (patternfly-react, patternfly-chatbot, etc.), strictly following the [official testing guidelines](https://github.com/patternfly/patternfly-react/wiki/React-Testing-Library-Basics,-Best-Practices,-and-Guidelines): + +- Separate test files per exported component +- `test()` outside `describe()`, `it()` inside +- `screen` for queries, `asFragment` for snapshots only +- Snapshots for structure, `toHaveClass` for CSS classes +- Mock child components for unit isolation +- Default to inline setup (no premature abstraction) + ### Agents Agents are domain knowledge the AI follows. @@ -67,7 +76,9 @@ pf-react/ │ ├── coding-standards.md │ └── unit-test-standards.md ├── skills/ -│ └── unit-test-generator/ +│ ├── unit-test-generator/ +│ │ └── SKILL.md +│ └── pf-library-test-writer/ │ └── SKILL.md └── README.md ``` diff --git a/plugins/pf-react/skills/pf-library-test-writer/SKILL.md b/plugins/pf-react/skills/pf-library-test-writer/SKILL.md new file mode 100644 index 0000000..b8363a4 --- /dev/null +++ b/plugins/pf-react/skills/pf-library-test-writer/SKILL.md @@ -0,0 +1,250 @@ +--- +name: pf-library-test-writer +description: Write unit tests for PatternFly ecosystem components following official testing standards +--- + +Write unit tests for components and features within the PatternFly ecosystem (patternfly-react, patternfly-chatbot, patternfly-virtual-assistant, and other JS/TS-based PatternFly libraries). All tests must strictly follow the [official PatternFly testing guidelines](https://github.com/patternfly/patternfly-react/wiki/React-Testing-Library-Basics,-Best-Practices,-and-Guidelines). + +## Input + +The user will provide a component file path, component code, or describe a new feature. Read the component source before writing tests. + +## How to Write Tests + +1. Read the component source to understand its props, children, and behavior. +2. If the PatternFly MCP server is available, use `usePatternFlyDocs` to check the component's props schema, variants, and accessibility requirements. +3. Generate a complete test file following every rule below. + +## Test File Structure + +```typescript +import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ComponentName } from '../ComponentName'; +``` + +## File Organization + +- **Separate test file per exported component.** Do not create monolithic test files. +- Name the file `ComponentName.test.tsx` next to the source file. + +``` +Button/ +├── Button.tsx +├── Button.test.tsx +├── ButtonVariant.tsx +└── ButtonVariant.test.tsx +``` + +## Rendering + +- Use RTL's `render`. There is no shallow rendering -- mock child components instead. +- Use the **`screen` object** for all queries. Do NOT assign `render`'s return to a variable for querying. +- The **one exception**: destructure `asFragment` from `render` for snapshot testing. + +```typescript +// Queries -- use screen +render(); +screen.getByRole('button', { name: 'Submit' }); + +// Snapshots -- destructure asFragment +const { asFragment } = render(); +expect(asFragment()).toMatchSnapshot(); +``` + +## Queries + +**Priority -- always try in this order:** + +1. `getByRole` with `name` option (default choice for almost everything) +2. `getByLabelText` +3. `getByPlaceholderText` +4. `getByText` +5. `getByDisplayValue` +6. `getByAltText` +7. `getByTitle` +8. `getByTestId` (last resort only) + +```typescript +screen.getByRole('button', { name: 'Save' }); +screen.getByRole('heading', { name: 'Dashboard' }); +screen.getByRole('checkbox', { name: 'Accept terms' }); +``` + +**Query types:** + +- `getBy` / `getAllBy` -- default. Throws if not found. +- `queryBy` / `queryAllBy` -- ONLY for asserting absence. +- `findBy` / `findAllBy` -- for async elements. Returns a promise. + +```typescript +expect(screen.getByRole('button')).toBeInTheDocument(); +expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); +expect(await screen.findByText('Loaded')).toBeInTheDocument(); +``` + +**Never use:** array indexing (`getAllBy[1]`), class selectors (`.querySelector`), or internal DOM structure. + +## User Interactions + +Always `userEvent`, never `fireEvent`: + +```typescript +const user = userEvent.setup(); +await user.click(screen.getByRole('button', { name: 'Submit' })); +await user.type(screen.getByRole('textbox', { name: 'Name' }), 'alice'); +await user.keyboard('{Escape}'); +``` + +## Using act() + +RTL handles `act()` automatically. If you get "not wrapped in act" warnings, resolve in this order: + +1. Use a `find` query instead of `get` +2. Use `waitFor` +3. Mock the operation +4. Wrap in `act()` only as a last resort + +**Exception:** wrap `advanceTimersByTime` in `act()` when using Jest fake timers. + +## Assertions + +Use [jest-dom matchers](https://github.com/testing-library/jest-dom#table-of-contents), not generic Jest matchers. Use the most specific matcher: + +```typescript +// Correct +expect(button).toBeDisabled(); +expect(element).toHaveClass('pf-m-primary'); +expect(input).toHaveValue('hello'); + +// Wrong +expect(button.disabled).toBe(true); +expect(element.className).toContain('pf-m-primary'); +``` + +## Snapshots + +- **DO** use snapshots for component structure and element ordering. +- **DO NOT** use snapshots to verify CSS classes. Use `toHaveClass` instead. + +```typescript +// Structure -- snapshot is appropriate +const { asFragment } = render(); +expect(asFragment()).toMatchSnapshot(); + +// Classes -- use toHaveClass, not snapshot +expect(screen.getByRole('button')).toHaveClass('pf-m-primary'); +``` + +## Mocking + +### Child components + +Default to **mocking child components** for unit testing prop-passing behavior: + +```typescript +jest.mock('../RandomHeader', () => () =>

Header text

); +``` + +With props: + +```typescript +jest.mock('../Header', () => ({ + Header: ({ children, ...props }) =>

{children}

+})); +``` + +### Callbacks + +```typescript +const onClickMock = jest.fn(); +render(Text); +const user = userEvent.setup(); +await user.click(screen.getByRole('button', { name: 'Text' })); +expect(onClickMock).toHaveBeenCalledTimes(1); +``` + +Also test that callbacks are NOT called when the interaction doesn't happen. + +### Contexts + +Wrap the component in the context provider with a `value` prop: + +```typescript +render( + + + +); +``` + +## Async Patterns + +Prefer `findBy*` over `waitFor` for elements: + +```typescript +expect(await screen.findByText('Loaded')).toBeInTheDocument(); +``` + +Use `waitFor` for non-query assertions: + +```typescript +await waitFor(() => { + expect(onComplete).toHaveBeenCalled(); +}); +``` + +## Test Nesting + +- **Do NOT** wrap all tests in a `describe()` that just names the component. +- **Do** use `describe()` to group tests that share setup. +- Use `test()` outside `describe()`, `it()` inside `describe()`. + +```typescript +test('renders with default props', () => { ... }); +test('applies custom className', () => { ... }); + +describe('when disabled', () => { + it('has disabled attribute', () => { ... }); + it('does not fire onClick', () => { ... }); +}); +``` + +## Test Abstraction + +Default to **NOT abstracting** setup. Keep setup inline in each test unless: +- It excessively bloats the suite +- Abstracting clarifies which setup elements vary per test + +`it.each` is acceptable when the pattern is clear: + +```typescript +describe('variants', () => { + it.each([ + ['primary', 'pf-m-primary'], + ['secondary', 'pf-m-secondary'], + ])('applies %s class', (variant, expectedClass) => { + render(); + expect(screen.getByRole('button')).toHaveClass(expectedClass); + }); +}); +``` + +## Coverage Checklist + +Cover these for every component: + +1. **Default rendering** -- renders with only required props +2. **Prop variations** -- each prop value produces expected output +3. **Custom className** -- merges with internal classes +4. **Spread props** -- extra props forwarded to root element +5. **Children** -- renders children correctly +6. **Callbacks** -- event handlers fire with correct arguments (and don't fire when they shouldn't) +7. **Conditional rendering** -- elements show/hide based on props +8. **Accessibility** -- ARIA roles, labels, keyboard interaction +9. **Snapshot** -- structure verification where appropriate (NOT for classes) + +## Output + +Output the complete test file ready to save. Name it `ComponentName.test.tsx`.