diff --git a/.claude/skills/frontend-testing/CHECKLIST.md b/.claude/skills/frontend-testing/CHECKLIST.md new file mode 100644 index 0000000000..95e04aec3f --- /dev/null +++ b/.claude/skills/frontend-testing/CHECKLIST.md @@ -0,0 +1,205 @@ +# Test Generation Checklist + +Use this checklist when generating or reviewing tests for Dify frontend components. + +## Pre-Generation + +- [ ] Read the component source code completely +- [ ] Identify component type (component, hook, utility, page) +- [ ] Run `pnpm analyze-component ` if available +- [ ] Note complexity score and features detected +- [ ] Check for existing tests in the same directory +- [ ] **Identify ALL files in the directory** that need testing (not just index) + +## Testing Strategy + +### ⚠️ Incremental Workflow (CRITICAL for Multi-File) + +- [ ] **NEVER generate all tests at once** - process one file at a time +- [ ] Order files by complexity: utilities → hooks → simple → complex → integration +- [ ] Create a todo list to track progress before starting +- [ ] For EACH file: write → run test → verify pass → then next +- [ ] **DO NOT proceed** to next file until current one passes + +### Path-Level Coverage + +- [ ] **Test ALL files** in the assigned directory/path +- [ ] List all components, hooks, utilities that need coverage +- [ ] Decide: single spec file (integration) or multiple spec files (unit) + +### Complexity Assessment + +- [ ] Run `pnpm analyze-component ` for complexity score +- [ ] **Complexity > 50**: Consider refactoring before testing +- [ ] **500+ lines**: Consider splitting before testing +- [ ] **30-50 complexity**: Use multiple describe blocks, organized structure + +### Integration vs Mocking + +- [ ] **DO NOT mock base components** (`Loading`, `Button`, `Tooltip`, etc.) +- [ ] Import real project components instead of mocking +- [ ] Only mock: API calls, complex context providers, third-party libs with side effects +- [ ] Prefer integration testing when using single spec file + +## Required Test Sections + +### All Components MUST Have + +- [ ] **Rendering tests** - Component renders without crashing +- [ ] **Props tests** - Required props, optional props, default values +- [ ] **Edge cases** - null, undefined, empty values, boundaries + +### Conditional Sections (Add When Feature Present) + +| Feature | Add Tests For | +|---------|---------------| +| `useState` | Initial state, transitions, cleanup | +| `useEffect` | Execution, dependencies, cleanup | +| Event handlers | onClick, onChange, onSubmit, keyboard | +| API calls | Loading, success, error states | +| Routing | Navigation, params, query strings | +| `useCallback`/`useMemo` | Referential equality | +| Context | Provider values, consumer behavior | +| Forms | Validation, submission, error display | + +## Code Quality Checklist + +### Structure + +- [ ] Uses `describe` blocks to group related tests +- [ ] Test names follow `should when ` pattern +- [ ] AAA pattern (Arrange-Act-Assert) is clear +- [ ] Comments explain complex test scenarios + +### Mocks + +- [ ] **DO NOT mock base components** (`@/app/components/base/*`) +- [ ] `jest.clearAllMocks()` in `beforeEach` (not `afterEach`) +- [ ] Shared mock state reset in `beforeEach` +- [ ] i18n mock returns keys (not empty strings) +- [ ] Router mocks match actual Next.js API +- [ ] Mocks reflect actual component conditional behavior +- [ ] Only mock: API services, complex context providers, third-party libs + +### Queries + +- [ ] Prefer semantic queries (`getByRole`, `getByLabelText`) +- [ ] Use `queryBy*` for absence assertions +- [ ] Use `findBy*` for async elements +- [ ] `getByTestId` only as last resort + +### Async + +- [ ] All async tests use `async/await` +- [ ] `waitFor` wraps async assertions +- [ ] Fake timers properly setup/teardown +- [ ] No floating promises + +### TypeScript + +- [ ] No `any` types without justification +- [ ] Mock data uses actual types from source +- [ ] Factory functions have proper return types + +## Coverage Goals (Per File) + +For the current file being tested: + +- [ ] 100% function coverage +- [ ] 100% statement coverage +- [ ] >95% branch coverage +- [ ] >95% line coverage + +## Post-Generation (Per File) + +**Run these checks after EACH test file, not just at the end:** + +- [ ] Run `pnpm test -- path/to/file.spec.tsx` - **MUST PASS before next file** +- [ ] Fix any failures immediately +- [ ] Mark file as complete in todo list +- [ ] Only then proceed to next file + +### After All Files Complete + +- [ ] Run full directory test: `pnpm test -- path/to/directory/` +- [ ] Check coverage report: `pnpm test -- --coverage` +- [ ] Run `pnpm lint:fix` on all test files +- [ ] Run `pnpm type-check:tsgo` + +## Common Issues to Watch + +### False Positives + +```typescript +// ❌ Mock doesn't match actual behavior +jest.mock('./Component', () => () =>
Mocked
) + +// ✅ Mock matches actual conditional logic +jest.mock('./Component', () => ({ isOpen }: any) => + isOpen ?
Content
: null +) +``` + +### State Leakage + +```typescript +// ❌ Shared state not reset +let mockState = false +jest.mock('./useHook', () => () => mockState) + +// ✅ Reset in beforeEach +beforeEach(() => { + mockState = false +}) +``` + +### Async Race Conditions + +```typescript +// ❌ Not awaited +it('loads data', () => { + render() + expect(screen.getByText('Data')).toBeInTheDocument() +}) + +// ✅ Properly awaited +it('loads data', async () => { + render() + await waitFor(() => { + expect(screen.getByText('Data')).toBeInTheDocument() + }) +}) +``` + +### Missing Edge Cases + +Always test these scenarios: + +- `null` / `undefined` inputs +- Empty strings / arrays / objects +- Boundary values (0, -1, MAX_INT) +- Error states +- Loading states +- Disabled states + +## Quick Commands + +```bash +# Run specific test +pnpm test -- path/to/file.spec.tsx + +# Run with coverage +pnpm test -- --coverage path/to/file.spec.tsx + +# Watch mode +pnpm test -- --watch path/to/file.spec.tsx + +# Update snapshots (use sparingly) +pnpm test -- -u path/to/file.spec.tsx + +# Analyze component +pnpm analyze-component path/to/component.tsx + +# Review existing test +pnpm analyze-component path/to/component.tsx --review +``` diff --git a/.claude/skills/frontend-testing/SKILL.md b/.claude/skills/frontend-testing/SKILL.md new file mode 100644 index 0000000000..dac604ac4b --- /dev/null +++ b/.claude/skills/frontend-testing/SKILL.md @@ -0,0 +1,320 @@ +--- +name: Dify Frontend Testing +description: Generate Jest + React Testing Library tests for Dify frontend components, hooks, and utilities. Triggers on testing, spec files, coverage, Jest, RTL, unit tests, integration tests, or write/review test requests. +--- + +# Dify Frontend Testing Skill + +This skill enables Claude to generate high-quality, comprehensive frontend tests for the Dify project following established conventions and best practices. + +> **⚠️ Authoritative Source**: This skill is derived from `web/testing/testing.md`. When in doubt, always refer to that document as the canonical specification. + +## When to Apply This Skill + +Apply this skill when the user: + +- Asks to **write tests** for a component, hook, or utility +- Asks to **review existing tests** for completeness +- Mentions **Jest**, **React Testing Library**, **RTL**, or **spec files** +- Requests **test coverage** improvement +- Uses `pnpm analyze-component` output as context +- Mentions **testing**, **unit tests**, or **integration tests** for frontend code +- Wants to understand **testing patterns** in the Dify codebase + +**Do NOT apply** when: + +- User is asking about backend/API tests (Python/pytest) +- User is asking about E2E tests (Playwright/Cypress) +- User is only asking conceptual questions without code context + +## Quick Reference + +### Tech Stack + +| Tool | Version | Purpose | +|------|---------|---------| +| Jest | 29.7 | Test runner | +| React Testing Library | 16.0 | Component testing | +| happy-dom | - | Test environment | +| nock | 14.0 | HTTP mocking | +| TypeScript | 5.x | Type safety | + +### Key Commands + +```bash +# Run all tests +pnpm test + +# Watch mode +pnpm test -- --watch + +# Run specific file +pnpm test -- path/to/file.spec.tsx + +# Generate coverage report +pnpm test -- --coverage + +# Analyze component complexity +pnpm analyze-component + +# Review existing test +pnpm analyze-component --review +``` + +### File Naming + +- Test files: `ComponentName.spec.tsx` (same directory as component) +- Integration tests: `web/__tests__/` directory + +## Test Structure Template + +```typescript +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import Component from './index' + +// ✅ Import real project components (DO NOT mock these) +// import Loading from '@/app/components/base/loading' +// import { ChildComponent } from './child-component' + +// ✅ Mock external dependencies only +jest.mock('@/service/api') +jest.mock('next/navigation', () => ({ + useRouter: () => ({ push: jest.fn() }), + usePathname: () => '/test', +})) + +// Shared state for mocks (if needed) +let mockSharedState = false + +describe('ComponentName', () => { + beforeEach(() => { + jest.clearAllMocks() // ✅ Reset mocks BEFORE each test + mockSharedState = false // ✅ Reset shared state + }) + + // Rendering tests (REQUIRED) + describe('Rendering', () => { + it('should render without crashing', () => { + // Arrange + const props = { title: 'Test' } + + // Act + render() + + // Assert + expect(screen.getByText('Test')).toBeInTheDocument() + }) + }) + + // Props tests (REQUIRED) + describe('Props', () => { + it('should apply custom className', () => { + render() + expect(screen.getByRole('button')).toHaveClass('custom') + }) + }) + + // User Interactions + describe('User Interactions', () => { + it('should handle click events', () => { + const handleClick = jest.fn() + render() + + fireEvent.click(screen.getByRole('button')) + + expect(handleClick).toHaveBeenCalledTimes(1) + }) + }) + + // Edge Cases (REQUIRED) + describe('Edge Cases', () => { + it('should handle null data', () => { + render() + expect(screen.getByText(/no data/i)).toBeInTheDocument() + }) + + it('should handle empty array', () => { + render() + expect(screen.getByText(/empty/i)).toBeInTheDocument() + }) + }) +}) +``` + +## Testing Workflow (CRITICAL) + +### ⚠️ Incremental Approach Required + +**NEVER generate all test files at once.** For complex components or multi-file directories: + +1. **Analyze & Plan**: List all files, order by complexity (simple → complex) +1. **Process ONE at a time**: Write test → Run test → Fix if needed → Next +1. **Verify before proceeding**: Do NOT continue to next file until current passes + +``` +For each file: + ┌────────────────────────────────────────┐ + │ 1. Write test │ + │ 2. Run: pnpm test -- .spec.tsx │ + │ 3. PASS? → Mark complete, next file │ + │ FAIL? → Fix first, then continue │ + └────────────────────────────────────────┘ +``` + +### Complexity-Based Order + +Process in this order for multi-file testing: + +1. 🟢 Utility functions (simplest) +1. 🟢 Custom hooks +1. 🟡 Simple components (presentational) +1. 🟡 Medium components (state, effects) +1. 🔴 Complex components (API, routing) +1. 🔴 Integration tests (index files - last) + +### When to Refactor First + +- **Complexity > 50**: Break into smaller pieces before testing +- **500+ lines**: Consider splitting before testing +- **Many dependencies**: Extract logic into hooks first + +> 📖 See `guides/workflow.md` for complete workflow details and todo list format. + +## Testing Strategy + +### Path-Level Testing (Directory Testing) + +When assigned to test a directory/path, test **ALL content** within that path: + +- Test all components, hooks, utilities in the directory (not just `index` file) +- Use incremental approach: one file at a time, verify each before proceeding +- Goal: 100% coverage of ALL files in the directory + +### Integration Testing First + +**Prefer integration testing** when writing tests for a directory: + +- ✅ **Import real project components** directly (including base components and siblings) +- ✅ **Only mock**: API services (`@/service/*`), `next/navigation`, complex context providers +- ❌ **DO NOT mock** base components (`@/app/components/base/*`) +- ❌ **DO NOT mock** sibling/child components in the same directory + +> See [Test Structure Template](#test-structure-template) for correct import/mock patterns. + +## Core Principles + +### 1. AAA Pattern (Arrange-Act-Assert) + +Every test should clearly separate: + +- **Arrange**: Setup test data and render component +- **Act**: Perform user actions +- **Assert**: Verify expected outcomes + +### 2. Black-Box Testing + +- Test observable behavior, not implementation details +- Use semantic queries (getByRole, getByLabelText) +- Avoid testing internal state directly +- **Prefer pattern matching over hardcoded strings** in assertions: + +```typescript +// ❌ Avoid: hardcoded text assertions +expect(screen.getByText('Loading...')).toBeInTheDocument() + +// ✅ Better: role-based queries +expect(screen.getByRole('status')).toBeInTheDocument() + +// ✅ Better: pattern matching +expect(screen.getByText(/loading/i)).toBeInTheDocument() +``` + +### 3. Single Behavior Per Test + +Each test verifies ONE user-observable behavior: + +```typescript +// ✅ Good: One behavior +it('should disable button when loading', () => { + render( + + + ) + + // Focus should cycle within modal + await user.tab() + expect(screen.getByText('First')).toHaveFocus() + + await user.tab() + expect(screen.getByText('Second')).toHaveFocus() + + await user.tab() + expect(screen.getByText('First')).toHaveFocus() // Cycles back + }) +}) +``` + +## Form Testing + +```typescript +describe('LoginForm', () => { + it('should submit valid form', async () => { + const user = userEvent.setup() + const onSubmit = jest.fn() + + render() + + await user.type(screen.getByLabelText(/email/i), 'test@example.com') + await user.type(screen.getByLabelText(/password/i), 'password123') + await user.click(screen.getByRole('button', { name: /sign in/i })) + + expect(onSubmit).toHaveBeenCalledWith({ + email: 'test@example.com', + password: 'password123', + }) + }) + + it('should show validation errors', async () => { + const user = userEvent.setup() + + render() + + // Submit empty form + await user.click(screen.getByRole('button', { name: /sign in/i })) + + expect(screen.getByText(/email is required/i)).toBeInTheDocument() + expect(screen.getByText(/password is required/i)).toBeInTheDocument() + }) + + it('should validate email format', async () => { + const user = userEvent.setup() + + render() + + await user.type(screen.getByLabelText(/email/i), 'invalid-email') + await user.click(screen.getByRole('button', { name: /sign in/i })) + + expect(screen.getByText(/invalid email/i)).toBeInTheDocument() + }) + + it('should disable submit button while submitting', async () => { + const user = userEvent.setup() + const onSubmit = jest.fn(() => new Promise(resolve => setTimeout(resolve, 100))) + + render() + + await user.type(screen.getByLabelText(/email/i), 'test@example.com') + await user.type(screen.getByLabelText(/password/i), 'password123') + await user.click(screen.getByRole('button', { name: /sign in/i })) + + expect(screen.getByRole('button', { name: /signing in/i })).toBeDisabled() + + await waitFor(() => { + expect(screen.getByRole('button', { name: /sign in/i })).toBeEnabled() + }) + }) +}) +``` + +## Data-Driven Tests with test.each + +```typescript +describe('StatusBadge', () => { + test.each([ + ['success', 'bg-green-500'], + ['warning', 'bg-yellow-500'], + ['error', 'bg-red-500'], + ['info', 'bg-blue-500'], + ])('should apply correct class for %s status', (status, expectedClass) => { + render() + + expect(screen.getByTestId('status-badge')).toHaveClass(expectedClass) + }) + + test.each([ + { input: null, expected: 'Unknown' }, + { input: undefined, expected: 'Unknown' }, + { input: '', expected: 'Unknown' }, + { input: 'invalid', expected: 'Unknown' }, + ])('should show "Unknown" for invalid input: $input', ({ input, expected }) => { + render() + + expect(screen.getByText(expected)).toBeInTheDocument() + }) +}) +``` + +## Debugging Tips + +```typescript +// Print entire DOM +screen.debug() + +// Print specific element +screen.debug(screen.getByRole('button')) + +// Log testing playground URL +screen.logTestingPlaygroundURL() + +// Pretty print DOM +import { prettyDOM } from '@testing-library/react' +console.log(prettyDOM(screen.getByRole('dialog'))) + +// Check available roles +import { getRoles } from '@testing-library/react' +console.log(getRoles(container)) +``` + +## Common Mistakes to Avoid + +### ❌ Don't Use Implementation Details + +```typescript +// Bad - testing implementation +expect(component.state.isOpen).toBe(true) +expect(wrapper.find('.internal-class').length).toBe(1) + +// Good - testing behavior +expect(screen.getByRole('dialog')).toBeInTheDocument() +``` + +### ❌ Don't Forget Cleanup + +```typescript +// Bad - may leak state between tests +it('test 1', () => { + render() +}) + +// Good - cleanup is automatic with RTL, but reset mocks +beforeEach(() => { + jest.clearAllMocks() +}) +``` + +### ❌ Don't Use Exact String Matching (Prefer Black-Box Assertions) + +```typescript +// ❌ Bad - hardcoded strings are brittle +expect(screen.getByText('Submit Form')).toBeInTheDocument() +expect(screen.getByText('Loading...')).toBeInTheDocument() + +// ✅ Good - role-based queries (most semantic) +expect(screen.getByRole('button', { name: /submit/i })).toBeInTheDocument() +expect(screen.getByRole('status')).toBeInTheDocument() + +// ✅ Good - pattern matching (flexible) +expect(screen.getByText(/submit/i)).toBeInTheDocument() +expect(screen.getByText(/loading/i)).toBeInTheDocument() + +// ✅ Good - test behavior, not exact UI text +expect(screen.getByRole('button')).toBeDisabled() +expect(screen.getByRole('alert')).toBeInTheDocument() +``` + +**Why prefer black-box assertions?** + +- Text content may change (i18n, copy updates) +- Role-based queries test accessibility +- Pattern matching is resilient to minor changes +- Tests focus on behavior, not implementation details + +### ❌ Don't Assert on Absence Without Query + +```typescript +// Bad - throws if not found +expect(screen.getByText('Error')).not.toBeInTheDocument() // Error! + +// Good - use queryBy for absence assertions +expect(screen.queryByText('Error')).not.toBeInTheDocument() +``` diff --git a/.claude/skills/frontend-testing/guides/domain-components.md b/.claude/skills/frontend-testing/guides/domain-components.md new file mode 100644 index 0000000000..ed2cc6eb8a --- /dev/null +++ b/.claude/skills/frontend-testing/guides/domain-components.md @@ -0,0 +1,523 @@ +# Domain-Specific Component Testing + +This guide covers testing patterns for Dify's domain-specific components. + +## Workflow Components (`workflow/`) + +Workflow components handle node configuration, data flow, and graph operations. + +### Key Test Areas + +1. **Node Configuration** +1. **Data Validation** +1. **Variable Passing** +1. **Edge Connections** +1. **Error Handling** + +### Example: Node Configuration Panel + +```typescript +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import NodeConfigPanel from './node-config-panel' +import { createMockNode, createMockWorkflowContext } from '@/__mocks__/workflow' + +// Mock workflow context +jest.mock('@/app/components/workflow/hooks', () => ({ + useWorkflowStore: () => mockWorkflowStore, + useNodesInteractions: () => mockNodesInteractions, +})) + +let mockWorkflowStore = { + nodes: [], + edges: [], + updateNode: jest.fn(), +} + +let mockNodesInteractions = { + handleNodeSelect: jest.fn(), + handleNodeDelete: jest.fn(), +} + +describe('NodeConfigPanel', () => { + beforeEach(() => { + jest.clearAllMocks() + mockWorkflowStore = { + nodes: [], + edges: [], + updateNode: jest.fn(), + } + }) + + describe('Node Configuration', () => { + it('should render node type selector', () => { + const node = createMockNode({ type: 'llm' }) + render() + + expect(screen.getByLabelText(/model/i)).toBeInTheDocument() + }) + + it('should update node config on change', async () => { + const user = userEvent.setup() + const node = createMockNode({ type: 'llm' }) + + render() + + await user.selectOptions(screen.getByLabelText(/model/i), 'gpt-4') + + expect(mockWorkflowStore.updateNode).toHaveBeenCalledWith( + node.id, + expect.objectContaining({ model: 'gpt-4' }) + ) + }) + }) + + describe('Data Validation', () => { + it('should show error for invalid input', async () => { + const user = userEvent.setup() + const node = createMockNode({ type: 'code' }) + + render() + + // Enter invalid code + const codeInput = screen.getByLabelText(/code/i) + await user.clear(codeInput) + await user.type(codeInput, 'invalid syntax {{{') + + await waitFor(() => { + expect(screen.getByText(/syntax error/i)).toBeInTheDocument() + }) + }) + + it('should validate required fields', async () => { + const node = createMockNode({ type: 'http', data: { url: '' } }) + + render() + + fireEvent.click(screen.getByRole('button', { name: /save/i })) + + await waitFor(() => { + expect(screen.getByText(/url is required/i)).toBeInTheDocument() + }) + }) + }) + + describe('Variable Passing', () => { + it('should display available variables from upstream nodes', () => { + const upstreamNode = createMockNode({ + id: 'node-1', + type: 'start', + data: { outputs: [{ name: 'user_input', type: 'string' }] }, + }) + const currentNode = createMockNode({ + id: 'node-2', + type: 'llm', + }) + + mockWorkflowStore.nodes = [upstreamNode, currentNode] + mockWorkflowStore.edges = [{ source: 'node-1', target: 'node-2' }] + + render() + + // Variable selector should show upstream variables + fireEvent.click(screen.getByRole('button', { name: /add variable/i })) + + expect(screen.getByText('user_input')).toBeInTheDocument() + }) + + it('should insert variable into prompt template', async () => { + const user = userEvent.setup() + const node = createMockNode({ type: 'llm' }) + + render() + + // Click variable button + await user.click(screen.getByRole('button', { name: /insert variable/i })) + await user.click(screen.getByText('user_input')) + + const promptInput = screen.getByLabelText(/prompt/i) + expect(promptInput).toHaveValue(expect.stringContaining('{{user_input}}')) + }) + }) +}) +``` + +## Dataset Components (`dataset/`) + +Dataset components handle file uploads, data display, and search/filter operations. + +### Key Test Areas + +1. **File Upload** +1. **File Type Validation** +1. **Pagination** +1. **Search & Filtering** +1. **Data Format Handling** + +### Example: Document Uploader + +```typescript +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import DocumentUploader from './document-uploader' + +jest.mock('@/service/datasets', () => ({ + uploadDocument: jest.fn(), + parseDocument: jest.fn(), +})) + +import * as datasetService from '@/service/datasets' +const mockedService = datasetService as jest.Mocked + +describe('DocumentUploader', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + describe('File Upload', () => { + it('should accept valid file types', async () => { + const user = userEvent.setup() + const onUpload = jest.fn() + mockedService.uploadDocument.mockResolvedValue({ id: 'doc-1' }) + + render() + + const file = new File(['content'], 'test.pdf', { type: 'application/pdf' }) + const input = screen.getByLabelText(/upload/i) + + await user.upload(input, file) + + await waitFor(() => { + expect(mockedService.uploadDocument).toHaveBeenCalledWith( + expect.any(FormData) + ) + }) + }) + + it('should reject invalid file types', async () => { + const user = userEvent.setup() + + render() + + const file = new File(['content'], 'test.exe', { type: 'application/x-msdownload' }) + const input = screen.getByLabelText(/upload/i) + + await user.upload(input, file) + + expect(screen.getByText(/unsupported file type/i)).toBeInTheDocument() + expect(mockedService.uploadDocument).not.toHaveBeenCalled() + }) + + it('should show upload progress', async () => { + const user = userEvent.setup() + + // Mock upload with progress + mockedService.uploadDocument.mockImplementation(() => { + return new Promise((resolve) => { + setTimeout(() => resolve({ id: 'doc-1' }), 100) + }) + }) + + render() + + const file = new File(['content'], 'test.pdf', { type: 'application/pdf' }) + await user.upload(screen.getByLabelText(/upload/i), file) + + expect(screen.getByRole('progressbar')).toBeInTheDocument() + + await waitFor(() => { + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + }) + }) + }) + + describe('Error Handling', () => { + it('should handle upload failure', async () => { + const user = userEvent.setup() + mockedService.uploadDocument.mockRejectedValue(new Error('Upload failed')) + + render() + + const file = new File(['content'], 'test.pdf', { type: 'application/pdf' }) + await user.upload(screen.getByLabelText(/upload/i), file) + + await waitFor(() => { + expect(screen.getByText(/upload failed/i)).toBeInTheDocument() + }) + }) + + it('should allow retry after failure', async () => { + const user = userEvent.setup() + mockedService.uploadDocument + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValueOnce({ id: 'doc-1' }) + + render() + + const file = new File(['content'], 'test.pdf', { type: 'application/pdf' }) + await user.upload(screen.getByLabelText(/upload/i), file) + + await waitFor(() => { + expect(screen.getByRole('button', { name: /retry/i })).toBeInTheDocument() + }) + + await user.click(screen.getByRole('button', { name: /retry/i })) + + await waitFor(() => { + expect(screen.getByText(/uploaded successfully/i)).toBeInTheDocument() + }) + }) + }) +}) +``` + +### Example: Document List with Pagination + +```typescript +describe('DocumentList', () => { + describe('Pagination', () => { + it('should load first page on mount', async () => { + mockedService.getDocuments.mockResolvedValue({ + data: [{ id: '1', name: 'Doc 1' }], + total: 50, + page: 1, + pageSize: 10, + }) + + render() + + await waitFor(() => { + expect(screen.getByText('Doc 1')).toBeInTheDocument() + }) + + expect(mockedService.getDocuments).toHaveBeenCalledWith('ds-1', { page: 1 }) + }) + + it('should navigate to next page', async () => { + const user = userEvent.setup() + mockedService.getDocuments.mockResolvedValue({ + data: [{ id: '1', name: 'Doc 1' }], + total: 50, + page: 1, + pageSize: 10, + }) + + render() + + await waitFor(() => { + expect(screen.getByText('Doc 1')).toBeInTheDocument() + }) + + mockedService.getDocuments.mockResolvedValue({ + data: [{ id: '11', name: 'Doc 11' }], + total: 50, + page: 2, + pageSize: 10, + }) + + await user.click(screen.getByRole('button', { name: /next/i })) + + await waitFor(() => { + expect(screen.getByText('Doc 11')).toBeInTheDocument() + }) + }) + }) + + describe('Search & Filtering', () => { + it('should filter by search query', async () => { + const user = userEvent.setup() + jest.useFakeTimers() + + render() + + await user.type(screen.getByPlaceholderText(/search/i), 'test query') + + // Debounce + jest.advanceTimersByTime(300) + + await waitFor(() => { + expect(mockedService.getDocuments).toHaveBeenCalledWith( + 'ds-1', + expect.objectContaining({ search: 'test query' }) + ) + }) + + jest.useRealTimers() + }) + }) +}) +``` + +## Configuration Components (`app/configuration/`, `config/`) + +Configuration components handle forms, validation, and data persistence. + +### Key Test Areas + +1. **Form Validation** +1. **Save/Reset** +1. **Required vs Optional Fields** +1. **Configuration Persistence** +1. **Error Feedback** + +### Example: App Configuration Form + +```typescript +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import AppConfigForm from './app-config-form' + +jest.mock('@/service/apps', () => ({ + updateAppConfig: jest.fn(), + getAppConfig: jest.fn(), +})) + +import * as appService from '@/service/apps' +const mockedService = appService as jest.Mocked + +describe('AppConfigForm', () => { + const defaultConfig = { + name: 'My App', + description: '', + icon: 'default', + openingStatement: '', + } + + beforeEach(() => { + jest.clearAllMocks() + mockedService.getAppConfig.mockResolvedValue(defaultConfig) + }) + + describe('Form Validation', () => { + it('should require app name', async () => { + const user = userEvent.setup() + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + // Clear name field + await user.clear(screen.getByLabelText(/name/i)) + await user.click(screen.getByRole('button', { name: /save/i })) + + expect(screen.getByText(/name is required/i)).toBeInTheDocument() + expect(mockedService.updateAppConfig).not.toHaveBeenCalled() + }) + + it('should validate name length', async () => { + const user = userEvent.setup() + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toBeInTheDocument() + }) + + // Enter very long name + await user.clear(screen.getByLabelText(/name/i)) + await user.type(screen.getByLabelText(/name/i), 'a'.repeat(101)) + + expect(screen.getByText(/name must be less than 100 characters/i)).toBeInTheDocument() + }) + + it('should allow empty optional fields', async () => { + const user = userEvent.setup() + mockedService.updateAppConfig.mockResolvedValue({ success: true }) + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + // Leave description empty (optional) + await user.click(screen.getByRole('button', { name: /save/i })) + + await waitFor(() => { + expect(mockedService.updateAppConfig).toHaveBeenCalled() + }) + }) + }) + + describe('Save/Reset Functionality', () => { + it('should save configuration', async () => { + const user = userEvent.setup() + mockedService.updateAppConfig.mockResolvedValue({ success: true }) + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + await user.clear(screen.getByLabelText(/name/i)) + await user.type(screen.getByLabelText(/name/i), 'Updated App') + await user.click(screen.getByRole('button', { name: /save/i })) + + await waitFor(() => { + expect(mockedService.updateAppConfig).toHaveBeenCalledWith( + 'app-1', + expect.objectContaining({ name: 'Updated App' }) + ) + }) + + expect(screen.getByText(/saved successfully/i)).toBeInTheDocument() + }) + + it('should reset to default values', async () => { + const user = userEvent.setup() + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + // Make changes + await user.clear(screen.getByLabelText(/name/i)) + await user.type(screen.getByLabelText(/name/i), 'Changed Name') + + // Reset + await user.click(screen.getByRole('button', { name: /reset/i })) + + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + it('should show unsaved changes warning', async () => { + const user = userEvent.setup() + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + // Make changes + await user.type(screen.getByLabelText(/name/i), ' Updated') + + expect(screen.getByText(/unsaved changes/i)).toBeInTheDocument() + }) + }) + + describe('Error Handling', () => { + it('should show error on save failure', async () => { + const user = userEvent.setup() + mockedService.updateAppConfig.mockRejectedValue(new Error('Server error')) + + render() + + await waitFor(() => { + expect(screen.getByLabelText(/name/i)).toHaveValue('My App') + }) + + await user.click(screen.getByRole('button', { name: /save/i })) + + await waitFor(() => { + expect(screen.getByText(/failed to save/i)).toBeInTheDocument() + }) + }) + }) +}) +``` diff --git a/.claude/skills/frontend-testing/guides/mocking.md b/.claude/skills/frontend-testing/guides/mocking.md new file mode 100644 index 0000000000..6b2c517cb6 --- /dev/null +++ b/.claude/skills/frontend-testing/guides/mocking.md @@ -0,0 +1,353 @@ +# Mocking Guide for Dify Frontend Tests + +## ⚠️ Important: What NOT to Mock + +### DO NOT Mock Base Components + +**Never mock components from `@/app/components/base/`** such as: + +- `Loading`, `Spinner` +- `Button`, `Input`, `Select` +- `Tooltip`, `Modal`, `Dropdown` +- `Icon`, `Badge`, `Tag` + +**Why?** + +- Base components will have their own dedicated tests +- Mocking them creates false positives (tests pass but real integration fails) +- Using real components tests actual integration behavior + +```typescript +// ❌ WRONG: Don't mock base components +jest.mock('@/app/components/base/loading', () => () =>
Loading
) +jest.mock('@/app/components/base/button', () => ({ children }: any) => ) + +// ✅ CORRECT: Import and use real base components +import Loading from '@/app/components/base/loading' +import Button from '@/app/components/base/button' +// They will render normally in tests +``` + +### What TO Mock + +Only mock these categories: + +1. **API services** (`@/service/*`) - Network calls +1. **Complex context providers** - When setup is too difficult +1. **Third-party libraries with side effects** - `next/navigation`, external SDKs +1. **i18n** - Always mock to return keys + +## Mock Placement + +| Location | Purpose | +|----------|---------| +| `web/__mocks__/` | Reusable mocks shared across multiple test files | +| Test file | Test-specific mocks, inline with `jest.mock()` | + +## Essential Mocks + +### 1. i18n (Always Required) + +```typescript +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})) +``` + +### 2. Next.js Router + +```typescript +const mockPush = jest.fn() +const mockReplace = jest.fn() + +jest.mock('next/navigation', () => ({ + useRouter: () => ({ + push: mockPush, + replace: mockReplace, + back: jest.fn(), + prefetch: jest.fn(), + }), + usePathname: () => '/current-path', + useSearchParams: () => new URLSearchParams('?key=value'), +})) + +describe('Component', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('should navigate on click', () => { + render() + fireEvent.click(screen.getByRole('button')) + expect(mockPush).toHaveBeenCalledWith('/expected-path') + }) +}) +``` + +### 3. Portal Components (with Shared State) + +```typescript +// ⚠️ Important: Use shared state for components that depend on each other +let mockPortalOpenState = false + +jest.mock('@/app/components/base/portal-to-follow-elem', () => ({ + PortalToFollowElem: ({ children, open, ...props }: any) => { + mockPortalOpenState = open || false // Update shared state + return
{children}
+ }, + PortalToFollowElemContent: ({ children }: any) => { + // ✅ Matches actual: returns null when portal is closed + if (!mockPortalOpenState) return null + return
{children}
+ }, + PortalToFollowElemTrigger: ({ children }: any) => ( +
{children}
+ ), +})) + +describe('Component', () => { + beforeEach(() => { + jest.clearAllMocks() + mockPortalOpenState = false // ✅ Reset shared state + }) +}) +``` + +### 4. API Service Mocks + +```typescript +import * as api from '@/service/api' + +jest.mock('@/service/api') + +const mockedApi = api as jest.Mocked + +describe('Component', () => { + beforeEach(() => { + jest.clearAllMocks() + + // Setup default mock implementation + mockedApi.fetchData.mockResolvedValue({ data: [] }) + }) + + it('should show data on success', async () => { + mockedApi.fetchData.mockResolvedValue({ data: [{ id: 1 }] }) + + render() + + await waitFor(() => { + expect(screen.getByText('1')).toBeInTheDocument() + }) + }) + + it('should show error on failure', async () => { + mockedApi.fetchData.mockRejectedValue(new Error('Network error')) + + render() + + await waitFor(() => { + expect(screen.getByText(/error/i)).toBeInTheDocument() + }) + }) +}) +``` + +### 5. HTTP Mocking with Nock + +```typescript +import nock from 'nock' + +const GITHUB_HOST = 'https://api.github.com' +const GITHUB_PATH = '/repos/owner/repo' + +const mockGithubApi = (status: number, body: Record, delayMs = 0) => { + return nock(GITHUB_HOST) + .get(GITHUB_PATH) + .delay(delayMs) + .reply(status, body) +} + +describe('GithubComponent', () => { + afterEach(() => { + nock.cleanAll() + }) + + it('should display repo info', async () => { + mockGithubApi(200, { name: 'dify', stars: 1000 }) + + render() + + await waitFor(() => { + expect(screen.getByText('dify')).toBeInTheDocument() + }) + }) + + it('should handle API error', async () => { + mockGithubApi(500, { message: 'Server error' }) + + render() + + await waitFor(() => { + expect(screen.getByText(/error/i)).toBeInTheDocument() + }) + }) +}) +``` + +### 6. Context Providers + +```typescript +import { ProviderContext } from '@/context/provider-context' +import { createMockProviderContextValue, createMockPlan } from '@/__mocks__/provider-context' + +describe('Component with Context', () => { + it('should render for free plan', () => { + const mockContext = createMockPlan('sandbox') + + render( + + + + ) + + expect(screen.getByText('Upgrade')).toBeInTheDocument() + }) + + it('should render for pro plan', () => { + const mockContext = createMockPlan('professional') + + render( + + + + ) + + expect(screen.queryByText('Upgrade')).not.toBeInTheDocument() + }) +}) +``` + +### 7. SWR / React Query + +```typescript +// SWR +jest.mock('swr', () => ({ + __esModule: true, + default: jest.fn(), +})) + +import useSWR from 'swr' +const mockedUseSWR = useSWR as jest.Mock + +describe('Component with SWR', () => { + it('should show loading state', () => { + mockedUseSWR.mockReturnValue({ + data: undefined, + error: undefined, + isLoading: true, + }) + + render() + expect(screen.getByText(/loading/i)).toBeInTheDocument() + }) +}) + +// React Query +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' + +const createTestQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, +}) + +const renderWithQueryClient = (ui: React.ReactElement) => { + const queryClient = createTestQueryClient() + return render( + + {ui} + + ) +} +``` + +## Mock Best Practices + +### ✅ DO + +1. **Use real base components** - Import from `@/app/components/base/` directly +1. **Use real project components** - Prefer importing over mocking +1. **Reset mocks in `beforeEach`**, not `afterEach` +1. **Match actual component behavior** in mocks (when mocking is necessary) +1. **Use factory functions** for complex mock data +1. **Import actual types** for type safety +1. **Reset shared mock state** in `beforeEach` + +### ❌ DON'T + +1. **Don't mock base components** (`Loading`, `Button`, `Tooltip`, etc.) +1. Don't mock components you can import directly +1. Don't create overly simplified mocks that miss conditional logic +1. Don't forget to clean up nock after each test +1. Don't use `any` types in mocks without necessity + +### Mock Decision Tree + +``` +Need to use a component in test? +│ +├─ Is it from @/app/components/base/*? +│ └─ YES → Import real component, DO NOT mock +│ +├─ Is it a project component? +│ └─ YES → Prefer importing real component +│ Only mock if setup is extremely complex +│ +├─ Is it an API service (@/service/*)? +│ └─ YES → Mock it +│ +├─ Is it a third-party lib with side effects? +│ └─ YES → Mock it (next/navigation, external SDKs) +│ +└─ Is it i18n? + └─ YES → Mock to return keys +``` + +## Factory Function Pattern + +```typescript +// __mocks__/data-factories.ts +import type { User, Project } from '@/types' + +export const createMockUser = (overrides: Partial = {}): User => ({ + id: 'user-1', + name: 'Test User', + email: 'test@example.com', + role: 'member', + createdAt: new Date().toISOString(), + ...overrides, +}) + +export const createMockProject = (overrides: Partial = {}): Project => ({ + id: 'project-1', + name: 'Test Project', + description: 'A test project', + owner: createMockUser(), + members: [], + createdAt: new Date().toISOString(), + ...overrides, +}) + +// Usage in tests +it('should display project owner', () => { + const project = createMockProject({ + owner: createMockUser({ name: 'John Doe' }), + }) + + render() + expect(screen.getByText('John Doe')).toBeInTheDocument() +}) +``` diff --git a/.claude/skills/frontend-testing/guides/workflow.md b/.claude/skills/frontend-testing/guides/workflow.md new file mode 100644 index 0000000000..b0f2994bde --- /dev/null +++ b/.claude/skills/frontend-testing/guides/workflow.md @@ -0,0 +1,269 @@ +# Testing Workflow Guide + +This guide defines the workflow for generating tests, especially for complex components or directories with multiple files. + +## Scope Clarification + +This guide addresses **multi-file workflow** (how to process multiple test files). For coverage requirements within a single test file, see `web/testing/testing.md` § Coverage Goals. + +| Scope | Rule | +|-------|------| +| **Single file** | Complete coverage in one generation (100% function, >95% branch) | +| **Multi-file directory** | Process one file at a time, verify each before proceeding | + +## ⚠️ Critical Rule: Incremental Approach for Multi-File Testing + +When testing a **directory with multiple files**, **NEVER generate all test files at once.** Use an incremental, verify-as-you-go approach. + +### Why Incremental? + +| Batch Approach (❌) | Incremental Approach (✅) | +|---------------------|---------------------------| +| Generate 5+ tests at once | Generate 1 test at a time | +| Run tests only at the end | Run test immediately after each file | +| Multiple failures compound | Single point of failure, easy to debug | +| Hard to identify root cause | Clear cause-effect relationship | +| Mock issues affect many files | Mock issues caught early | +| Messy git history | Clean, atomic commits possible | + +## Single File Workflow + +When testing a **single component, hook, or utility**: + +``` +1. Read source code completely +2. Run `pnpm analyze-component ` (if available) +3. Check complexity score and features detected +4. Write the test file +5. Run test: `pnpm test -- .spec.tsx` +6. Fix any failures +7. Verify coverage meets goals (100% function, >95% branch) +``` + +## Directory/Multi-File Workflow (MUST FOLLOW) + +When testing a **directory or multiple files**, follow this strict workflow: + +### Step 1: Analyze and Plan + +1. **List all files** that need tests in the directory +1. **Categorize by complexity**: + - 🟢 **Simple**: Utility functions, simple hooks, presentational components + - 🟡 **Medium**: Components with state, effects, or event handlers + - 🔴 **Complex**: Components with API calls, routing, or many dependencies +1. **Order by dependency**: Test dependencies before dependents +1. **Create a todo list** to track progress + +### Step 2: Determine Processing Order + +Process files in this recommended order: + +``` +1. Utility functions (simplest, no React) +2. Custom hooks (isolated logic) +3. Simple presentational components (few/no props) +4. Medium complexity components (state, effects) +5. Complex components (API, routing, many deps) +6. Container/index components (integration tests - last) +``` + +**Rationale**: + +- Simpler files help establish mock patterns +- Hooks used by components should be tested first +- Integration tests (index files) depend on child components working + +### Step 3: Process Each File Incrementally + +**For EACH file in the ordered list:** + +``` +┌─────────────────────────────────────────────┐ +│ 1. Write test file │ +│ 2. Run: pnpm test -- .spec.tsx │ +│ 3. If FAIL → Fix immediately, re-run │ +│ 4. If PASS → Mark complete in todo list │ +│ 5. ONLY THEN proceed to next file │ +└─────────────────────────────────────────────┘ +``` + +**DO NOT proceed to the next file until the current one passes.** + +### Step 4: Final Verification + +After all individual tests pass: + +```bash +# Run all tests in the directory together +pnpm test -- path/to/directory/ + +# Check coverage +pnpm test -- --coverage path/to/directory/ +``` + +## Component Complexity Guidelines + +Use `pnpm analyze-component ` to assess complexity before testing. + +### 🔴 Very Complex Components (Complexity > 50) + +**Consider refactoring BEFORE testing:** + +- Break component into smaller, testable pieces +- Extract complex logic into custom hooks +- Separate container and presentational layers + +**If testing as-is:** + +- Use integration tests for complex workflows +- Use `test.each()` for data-driven testing +- Multiple `describe` blocks for organization +- Consider testing major sections separately + +### 🟡 Medium Complexity (Complexity 30-50) + +- Group related tests in `describe` blocks +- Test integration scenarios between internal parts +- Focus on state transitions and side effects +- Use helper functions to reduce test complexity + +### 🟢 Simple Components (Complexity < 30) + +- Standard test structure +- Focus on props, rendering, and edge cases +- Usually straightforward to test + +### 📏 Large Files (500+ lines) + +Regardless of complexity score: + +- **Strongly consider refactoring** before testing +- If testing as-is, test major sections separately +- Create helper functions for test setup +- May need multiple test files + +## Todo List Format + +When testing multiple files, use a todo list like this: + +``` +Testing: path/to/directory/ + +Ordered by complexity (simple → complex): + +☐ utils/helper.ts [utility, simple] +☐ hooks/use-custom-hook.ts [hook, simple] +☐ empty-state.tsx [component, simple] +☐ item-card.tsx [component, medium] +☐ list.tsx [component, complex] +☐ index.tsx [integration] + +Progress: 0/6 complete +``` + +Update status as you complete each: + +- ☐ → ⏳ (in progress) +- ⏳ → ✅ (complete and verified) +- ⏳ → ❌ (blocked, needs attention) + +## When to Stop and Verify + +**Always run tests after:** + +- Completing a test file +- Making changes to fix a failure +- Modifying shared mocks +- Updating test utilities or helpers + +**Signs you should pause:** + +- More than 2 consecutive test failures +- Mock-related errors appearing +- Unclear why a test is failing +- Test passing but coverage unexpectedly low + +## Common Pitfalls to Avoid + +### ❌ Don't: Generate Everything First + +``` +# BAD: Writing all files then testing +Write component-a.spec.tsx +Write component-b.spec.tsx +Write component-c.spec.tsx +Write component-d.spec.tsx +Run pnpm test ← Multiple failures, hard to debug +``` + +### ✅ Do: Verify Each Step + +``` +# GOOD: Incremental with verification +Write component-a.spec.tsx +Run pnpm test -- component-a.spec.tsx ✅ +Write component-b.spec.tsx +Run pnpm test -- component-b.spec.tsx ✅ +...continue... +``` + +### ❌ Don't: Skip Verification for "Simple" Components + +Even simple components can have: + +- Import errors +- Missing mock setup +- Incorrect assumptions about props + +**Always verify, regardless of perceived simplicity.** + +### ❌ Don't: Continue When Tests Fail + +Failing tests compound: + +- A mock issue in file A affects files B, C, D +- Fixing A later requires revisiting all dependent tests +- Time wasted on debugging cascading failures + +**Fix failures immediately before proceeding.** + +## Integration with Claude's Todo Feature + +When using Claude for multi-file testing: + +1. **Ask Claude to create a todo list** before starting +1. **Request one file at a time** or ensure Claude processes incrementally +1. **Verify each test passes** before asking for the next +1. **Mark todos complete** as you progress + +Example prompt: + +``` +Test all components in `path/to/directory/`. +First, analyze the directory and create a todo list ordered by complexity. +Then, process ONE file at a time, waiting for my confirmation that tests pass +before proceeding to the next. +``` + +## Summary Checklist + +Before starting multi-file testing: + +- [ ] Listed all files needing tests +- [ ] Ordered by complexity (simple → complex) +- [ ] Created todo list for tracking +- [ ] Understand dependencies between files + +During testing: + +- [ ] Processing ONE file at a time +- [ ] Running tests after EACH file +- [ ] Fixing failures BEFORE proceeding +- [ ] Updating todo list progress + +After completion: + +- [ ] All individual tests pass +- [ ] Full directory test run passes +- [ ] Coverage goals met +- [ ] Todo list shows all complete diff --git a/.claude/skills/frontend-testing/templates/component-test.template.tsx b/.claude/skills/frontend-testing/templates/component-test.template.tsx new file mode 100644 index 0000000000..9b1542b676 --- /dev/null +++ b/.claude/skills/frontend-testing/templates/component-test.template.tsx @@ -0,0 +1,289 @@ +/** + * Test Template for React Components + * + * WHY THIS STRUCTURE? + * - Organized sections make tests easy to navigate and maintain + * - Mocks at top ensure consistent test isolation + * - Factory functions reduce duplication and improve readability + * - describe blocks group related scenarios for better debugging + * + * INSTRUCTIONS: + * 1. Replace `ComponentName` with your component name + * 2. Update import path + * 3. Add/remove test sections based on component features (use analyze-component) + * 4. Follow AAA pattern: Arrange → Act → Assert + * + * RUN FIRST: pnpm analyze-component to identify required test scenarios + */ + +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +// import ComponentName from './index' + +// ============================================================================ +// Mocks +// ============================================================================ +// WHY: Mocks must be hoisted to top of file (Jest requirement). +// They run BEFORE imports, so keep them before component imports. + +// i18n (always required in Dify) +// WHY: Returns key instead of translation so tests don't depend on i18n files +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})) + +// Router (if component uses useRouter, usePathname, useSearchParams) +// WHY: Isolates tests from Next.js routing, enables testing navigation behavior +// const mockPush = jest.fn() +// jest.mock('next/navigation', () => ({ +// useRouter: () => ({ push: mockPush }), +// usePathname: () => '/test-path', +// })) + +// API services (if component fetches data) +// WHY: Prevents real network calls, enables testing all states (loading/success/error) +// jest.mock('@/service/api') +// import * as api from '@/service/api' +// const mockedApi = api as jest.Mocked + +// Shared mock state (for portal/dropdown components) +// WHY: Portal components like PortalToFollowElem need shared state between +// parent and child mocks to correctly simulate open/close behavior +// let mockOpenState = false + +// ============================================================================ +// Test Data Factories +// ============================================================================ +// WHY FACTORIES? +// - Avoid hard-coded test data scattered across tests +// - Easy to create variations with overrides +// - Type-safe when using actual types from source +// - Single source of truth for default test values + +// const createMockProps = (overrides = {}) => ({ +// // Default props that make component render successfully +// ...overrides, +// }) + +// const createMockItem = (overrides = {}) => ({ +// id: 'item-1', +// name: 'Test Item', +// ...overrides, +// }) + +// ============================================================================ +// Test Helpers +// ============================================================================ + +// const renderComponent = (props = {}) => { +// return render() +// } + +// ============================================================================ +// Tests +// ============================================================================ + +describe('ComponentName', () => { + // WHY beforeEach with clearAllMocks? + // - Ensures each test starts with clean slate + // - Prevents mock call history from leaking between tests + // - MUST be beforeEach (not afterEach) to reset BEFORE assertions like toHaveBeenCalledTimes + beforeEach(() => { + jest.clearAllMocks() + // Reset shared mock state if used (CRITICAL for portal/dropdown tests) + // mockOpenState = false + }) + + // -------------------------------------------------------------------------- + // Rendering Tests (REQUIRED - Every component MUST have these) + // -------------------------------------------------------------------------- + // WHY: Catches import errors, missing providers, and basic render issues + describe('Rendering', () => { + it('should render without crashing', () => { + // Arrange - Setup data and mocks + // const props = createMockProps() + + // Act - Render the component + // render() + + // Assert - Verify expected output + // Prefer getByRole for accessibility; it's what users "see" + // expect(screen.getByRole('...')).toBeInTheDocument() + }) + + it('should render with default props', () => { + // WHY: Verifies component works without optional props + // render() + // expect(screen.getByText('...')).toBeInTheDocument() + }) + }) + + // -------------------------------------------------------------------------- + // Props Tests (REQUIRED - Every component MUST test prop behavior) + // -------------------------------------------------------------------------- + // WHY: Props are the component's API contract. Test them thoroughly. + describe('Props', () => { + it('should apply custom className', () => { + // WHY: Common pattern in Dify - components should merge custom classes + // render() + // expect(screen.getByTestId('component')).toHaveClass('custom-class') + }) + + it('should use default values for optional props', () => { + // WHY: Verifies TypeScript defaults work at runtime + // render() + // expect(screen.getByRole('...')).toHaveAttribute('...', 'default-value') + }) + }) + + // -------------------------------------------------------------------------- + // User Interactions (if component has event handlers - on*, handle*) + // -------------------------------------------------------------------------- + // WHY: Event handlers are core functionality. Test from user's perspective. + describe('User Interactions', () => { + it('should call onClick when clicked', async () => { + // WHY userEvent over fireEvent? + // - userEvent simulates real user behavior (focus, hover, then click) + // - fireEvent is lower-level, doesn't trigger all browser events + // const user = userEvent.setup() + // const handleClick = jest.fn() + // render() + // + // await user.click(screen.getByRole('button')) + // + // expect(handleClick).toHaveBeenCalledTimes(1) + }) + + it('should call onChange when value changes', async () => { + // const user = userEvent.setup() + // const handleChange = jest.fn() + // render() + // + // await user.type(screen.getByRole('textbox'), 'new value') + // + // expect(handleChange).toHaveBeenCalled() + }) + }) + + // -------------------------------------------------------------------------- + // State Management (if component uses useState/useReducer) + // -------------------------------------------------------------------------- + // WHY: Test state through observable UI changes, not internal state values + describe('State Management', () => { + it('should update state on interaction', async () => { + // WHY test via UI, not state? + // - State is implementation detail; UI is what users see + // - If UI works correctly, state must be correct + // const user = userEvent.setup() + // render() + // + // // Initial state - verify what user sees + // expect(screen.getByText('Initial')).toBeInTheDocument() + // + // // Trigger state change via user action + // await user.click(screen.getByRole('button')) + // + // // New state - verify UI updated + // expect(screen.getByText('Updated')).toBeInTheDocument() + }) + }) + + // -------------------------------------------------------------------------- + // Async Operations (if component fetches data - useSWR, useQuery, fetch) + // -------------------------------------------------------------------------- + // WHY: Async operations have 3 states users experience: loading, success, error + describe('Async Operations', () => { + it('should show loading state', () => { + // WHY never-resolving promise? + // - Keeps component in loading state for assertion + // - Alternative: use fake timers + // mockedApi.fetchData.mockImplementation(() => new Promise(() => {})) + // render() + // + // expect(screen.getByText(/loading/i)).toBeInTheDocument() + }) + + it('should show data on success', async () => { + // WHY waitFor? + // - Component updates asynchronously after fetch resolves + // - waitFor retries assertion until it passes or times out + // mockedApi.fetchData.mockResolvedValue({ items: ['Item 1'] }) + // render() + // + // await waitFor(() => { + // expect(screen.getByText('Item 1')).toBeInTheDocument() + // }) + }) + + it('should show error on failure', async () => { + // mockedApi.fetchData.mockRejectedValue(new Error('Network error')) + // render() + // + // await waitFor(() => { + // expect(screen.getByText(/error/i)).toBeInTheDocument() + // }) + }) + }) + + // -------------------------------------------------------------------------- + // Edge Cases (REQUIRED - Every component MUST handle edge cases) + // -------------------------------------------------------------------------- + // WHY: Real-world data is messy. Components must handle: + // - Null/undefined from API failures or optional fields + // - Empty arrays/strings from user clearing data + // - Boundary values (0, MAX_INT, special characters) + describe('Edge Cases', () => { + it('should handle null value', () => { + // WHY test null specifically? + // - API might return null for missing data + // - Prevents "Cannot read property of null" in production + // render() + // expect(screen.getByText(/no data/i)).toBeInTheDocument() + }) + + it('should handle undefined value', () => { + // WHY test undefined separately from null? + // - TypeScript treats them differently + // - Optional props are undefined, not null + // render() + // expect(screen.getByText(/no data/i)).toBeInTheDocument() + }) + + it('should handle empty array', () => { + // WHY: Empty state often needs special UI (e.g., "No items yet") + // render() + // expect(screen.getByText(/empty/i)).toBeInTheDocument() + }) + + it('should handle empty string', () => { + // WHY: Empty strings are truthy in JS but visually empty + // render() + // expect(screen.getByText(/placeholder/i)).toBeInTheDocument() + }) + }) + + // -------------------------------------------------------------------------- + // Accessibility (optional but recommended for Dify's enterprise users) + // -------------------------------------------------------------------------- + // WHY: Dify has enterprise customers who may require accessibility compliance + describe('Accessibility', () => { + it('should have accessible name', () => { + // WHY getByRole with name? + // - Tests that screen readers can identify the element + // - Enforces proper labeling practices + // render() + // expect(screen.getByRole('button', { name: /test label/i })).toBeInTheDocument() + }) + + it('should support keyboard navigation', async () => { + // WHY: Some users can't use a mouse + // const user = userEvent.setup() + // render() + // + // await user.tab() + // expect(screen.getByRole('button')).toHaveFocus() + }) + }) +}) diff --git a/.claude/skills/frontend-testing/templates/hook-test.template.ts b/.claude/skills/frontend-testing/templates/hook-test.template.ts new file mode 100644 index 0000000000..4fb7fd21ec --- /dev/null +++ b/.claude/skills/frontend-testing/templates/hook-test.template.ts @@ -0,0 +1,207 @@ +/** + * Test Template for Custom Hooks + * + * Instructions: + * 1. Replace `useHookName` with your hook name + * 2. Update import path + * 3. Add/remove test sections based on hook features + */ + +import { renderHook, act, waitFor } from '@testing-library/react' +// import { useHookName } from './use-hook-name' + +// ============================================================================ +// Mocks +// ============================================================================ + +// API services (if hook fetches data) +// jest.mock('@/service/api') +// import * as api from '@/service/api' +// const mockedApi = api as jest.Mocked + +// ============================================================================ +// Test Helpers +// ============================================================================ + +// Wrapper for hooks that need context +// const createWrapper = (contextValue = {}) => { +// return ({ children }: { children: React.ReactNode }) => ( +// +// {children} +// +// ) +// } + +// ============================================================================ +// Tests +// ============================================================================ + +describe('useHookName', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + // -------------------------------------------------------------------------- + // Initial State + // -------------------------------------------------------------------------- + describe('Initial State', () => { + it('should return initial state', () => { + // const { result } = renderHook(() => useHookName()) + // + // expect(result.current.value).toBe(initialValue) + // expect(result.current.isLoading).toBe(false) + }) + + it('should accept initial value from props', () => { + // const { result } = renderHook(() => useHookName({ initialValue: 'custom' })) + // + // expect(result.current.value).toBe('custom') + }) + }) + + // -------------------------------------------------------------------------- + // State Updates + // -------------------------------------------------------------------------- + describe('State Updates', () => { + it('should update value when setValue is called', () => { + // const { result } = renderHook(() => useHookName()) + // + // act(() => { + // result.current.setValue('new value') + // }) + // + // expect(result.current.value).toBe('new value') + }) + + it('should reset to initial value', () => { + // const { result } = renderHook(() => useHookName({ initialValue: 'initial' })) + // + // act(() => { + // result.current.setValue('changed') + // }) + // expect(result.current.value).toBe('changed') + // + // act(() => { + // result.current.reset() + // }) + // expect(result.current.value).toBe('initial') + }) + }) + + // -------------------------------------------------------------------------- + // Async Operations + // -------------------------------------------------------------------------- + describe('Async Operations', () => { + it('should fetch data on mount', async () => { + // mockedApi.fetchData.mockResolvedValue({ data: 'test' }) + // + // const { result } = renderHook(() => useHookName()) + // + // // Initially loading + // expect(result.current.isLoading).toBe(true) + // + // // Wait for data + // await waitFor(() => { + // expect(result.current.isLoading).toBe(false) + // }) + // + // expect(result.current.data).toEqual({ data: 'test' }) + }) + + it('should handle fetch error', async () => { + // mockedApi.fetchData.mockRejectedValue(new Error('Network error')) + // + // const { result } = renderHook(() => useHookName()) + // + // await waitFor(() => { + // expect(result.current.error).toBeTruthy() + // }) + // + // expect(result.current.error?.message).toBe('Network error') + }) + + it('should refetch when dependency changes', async () => { + // mockedApi.fetchData.mockResolvedValue({ data: 'test' }) + // + // const { result, rerender } = renderHook( + // ({ id }) => useHookName(id), + // { initialProps: { id: '1' } } + // ) + // + // await waitFor(() => { + // expect(mockedApi.fetchData).toHaveBeenCalledWith('1') + // }) + // + // rerender({ id: '2' }) + // + // await waitFor(() => { + // expect(mockedApi.fetchData).toHaveBeenCalledWith('2') + // }) + }) + }) + + // -------------------------------------------------------------------------- + // Side Effects + // -------------------------------------------------------------------------- + describe('Side Effects', () => { + it('should call callback when value changes', () => { + // const callback = jest.fn() + // const { result } = renderHook(() => useHookName({ onChange: callback })) + // + // act(() => { + // result.current.setValue('new value') + // }) + // + // expect(callback).toHaveBeenCalledWith('new value') + }) + + it('should cleanup on unmount', () => { + // const cleanup = jest.fn() + // jest.spyOn(window, 'addEventListener') + // jest.spyOn(window, 'removeEventListener') + // + // const { unmount } = renderHook(() => useHookName()) + // + // expect(window.addEventListener).toHaveBeenCalled() + // + // unmount() + // + // expect(window.removeEventListener).toHaveBeenCalled() + }) + }) + + // -------------------------------------------------------------------------- + // Edge Cases + // -------------------------------------------------------------------------- + describe('Edge Cases', () => { + it('should handle null input', () => { + // const { result } = renderHook(() => useHookName(null)) + // + // expect(result.current.value).toBeNull() + }) + + it('should handle rapid updates', () => { + // const { result } = renderHook(() => useHookName()) + // + // act(() => { + // result.current.setValue('1') + // result.current.setValue('2') + // result.current.setValue('3') + // }) + // + // expect(result.current.value).toBe('3') + }) + }) + + // -------------------------------------------------------------------------- + // With Context (if hook uses context) + // -------------------------------------------------------------------------- + describe('With Context', () => { + it('should use context value', () => { + // const wrapper = createWrapper({ someValue: 'context-value' }) + // const { result } = renderHook(() => useHookName(), { wrapper }) + // + // expect(result.current.contextValue).toBe('context-value') + }) + }) +}) diff --git a/.claude/skills/frontend-testing/templates/utility-test.template.ts b/.claude/skills/frontend-testing/templates/utility-test.template.ts new file mode 100644 index 0000000000..ec13b5f5bd --- /dev/null +++ b/.claude/skills/frontend-testing/templates/utility-test.template.ts @@ -0,0 +1,154 @@ +/** + * Test Template for Utility Functions + * + * Instructions: + * 1. Replace `utilityFunction` with your function name + * 2. Update import path + * 3. Use test.each for data-driven tests + */ + +// import { utilityFunction } from './utility' + +// ============================================================================ +// Tests +// ============================================================================ + +describe('utilityFunction', () => { + // -------------------------------------------------------------------------- + // Basic Functionality + // -------------------------------------------------------------------------- + describe('Basic Functionality', () => { + it('should return expected result for valid input', () => { + // expect(utilityFunction('input')).toBe('expected-output') + }) + + it('should handle multiple arguments', () => { + // expect(utilityFunction('a', 'b', 'c')).toBe('abc') + }) + }) + + // -------------------------------------------------------------------------- + // Data-Driven Tests + // -------------------------------------------------------------------------- + describe('Input/Output Mapping', () => { + test.each([ + // [input, expected] + ['input1', 'output1'], + ['input2', 'output2'], + ['input3', 'output3'], + ])('should return %s for input %s', (input, expected) => { + // expect(utilityFunction(input)).toBe(expected) + }) + }) + + // -------------------------------------------------------------------------- + // Edge Cases + // -------------------------------------------------------------------------- + describe('Edge Cases', () => { + it('should handle empty string', () => { + // expect(utilityFunction('')).toBe('') + }) + + it('should handle null', () => { + // expect(utilityFunction(null)).toBe(null) + // or + // expect(() => utilityFunction(null)).toThrow() + }) + + it('should handle undefined', () => { + // expect(utilityFunction(undefined)).toBe(undefined) + // or + // expect(() => utilityFunction(undefined)).toThrow() + }) + + it('should handle empty array', () => { + // expect(utilityFunction([])).toEqual([]) + }) + + it('should handle empty object', () => { + // expect(utilityFunction({})).toEqual({}) + }) + }) + + // -------------------------------------------------------------------------- + // Boundary Conditions + // -------------------------------------------------------------------------- + describe('Boundary Conditions', () => { + it('should handle minimum value', () => { + // expect(utilityFunction(0)).toBe(0) + }) + + it('should handle maximum value', () => { + // expect(utilityFunction(Number.MAX_SAFE_INTEGER)).toBe(...) + }) + + it('should handle negative numbers', () => { + // expect(utilityFunction(-1)).toBe(...) + }) + }) + + // -------------------------------------------------------------------------- + // Type Coercion (if applicable) + // -------------------------------------------------------------------------- + describe('Type Handling', () => { + it('should handle numeric string', () => { + // expect(utilityFunction('123')).toBe(123) + }) + + it('should handle boolean', () => { + // expect(utilityFunction(true)).toBe(...) + }) + }) + + // -------------------------------------------------------------------------- + // Error Cases + // -------------------------------------------------------------------------- + describe('Error Handling', () => { + it('should throw for invalid input', () => { + // expect(() => utilityFunction('invalid')).toThrow('Error message') + }) + + it('should throw with specific error type', () => { + // expect(() => utilityFunction('invalid')).toThrow(ValidationError) + }) + }) + + // -------------------------------------------------------------------------- + // Complex Objects (if applicable) + // -------------------------------------------------------------------------- + describe('Object Handling', () => { + it('should preserve object structure', () => { + // const input = { a: 1, b: 2 } + // expect(utilityFunction(input)).toEqual({ a: 1, b: 2 }) + }) + + it('should handle nested objects', () => { + // const input = { nested: { deep: 'value' } } + // expect(utilityFunction(input)).toEqual({ nested: { deep: 'transformed' } }) + }) + + it('should not mutate input', () => { + // const input = { a: 1 } + // const inputCopy = { ...input } + // utilityFunction(input) + // expect(input).toEqual(inputCopy) + }) + }) + + // -------------------------------------------------------------------------- + // Array Handling (if applicable) + // -------------------------------------------------------------------------- + describe('Array Handling', () => { + it('should process all elements', () => { + // expect(utilityFunction([1, 2, 3])).toEqual([2, 4, 6]) + }) + + it('should handle single element array', () => { + // expect(utilityFunction([1])).toEqual([2]) + }) + + it('should preserve order', () => { + // expect(utilityFunction(['c', 'a', 'b'])).toEqual(['c', 'a', 'b']) + }) + }) +}) diff --git a/.github/workflows/autofix.yml b/.github/workflows/autofix.yml index 81392a9734..3563c29577 100644 --- a/.github/workflows/autofix.yml +++ b/.github/workflows/autofix.yml @@ -61,9 +61,10 @@ jobs: find . -name "*.py" -type f -exec sed -i.bak -E 's/"([^"]+)" \| None/Optional["\1"]/g; s/'"'"'([^'"'"']+)'"'"' \| None/Optional['"'"'\1'"'"']/g' {} \; find . -name "*.py.bak" -type f -delete + # mdformat breaks YAML front matter in markdown files. Add --exclude for directories containing YAML front matter. - name: mdformat run: | - uvx mdformat . + uvx --python 3.13 mdformat . --exclude ".claude/skills/**" - name: Install pnpm uses: pnpm/action-setup@v4 diff --git a/web/testing/testing.md b/web/testing/testing.md index bf1b89af00..a08a615e54 100644 --- a/web/testing/testing.md +++ b/web/testing/testing.md @@ -42,7 +42,7 @@ pnpm test -- path/to/file.spec.tsx ## Test Authoring Principles - **Single behavior per test**: Each test verifies one user-observable behavior. -- **Black-box first**: Assert external behavior and observable outputs, avoid internal implementation details. +- **Black-box first**: Assert external behavior and observable outputs, avoid internal implementation details. Prefer role-based queries (`getByRole`) and pattern matching (`/text/i`) over hardcoded string assertions. - **Semantic naming**: Use `should when ` and group related cases with `describe()`. - **AAA / Given–When–Then**: Separate Arrange, Act, and Assert clearly with code blocks or comments. - **Minimal but sufficient assertions**: Keep only the expectations that express the essence of the behavior. @@ -93,6 +93,7 @@ Use `pnpm analyze-component ` to analyze component complexity and adopt di - Testing time-based behavior (delays, animations) - If you mock all time-dependent functions, fake timers are unnecessary 1. **Prefer importing over mocking project components**: When tests need other components from the project, import them directly instead of mocking them. Only mock external dependencies, APIs, or complex context providers that are difficult to set up. +1. **DO NOT mock base components**: Never mock components from `@/app/components/base/` (e.g., `Loading`, `Button`, `Tooltip`, `Modal`). Base components will have their own dedicated tests. Use real components to test actual integration behavior. **Why this matters**: Mocks that don't match actual behavior can lead to: @@ -101,6 +102,43 @@ Use `pnpm analyze-component ` to analyze component complexity and adopt di - **Maintenance burden**: Tests become misleading documentation - **State leakage**: Tests interfere with each other when shared state isn't reset +## Path-Level Testing Strategy + +When assigned to test a **directory/path** (not just a single file), follow these guidelines: + +### Coverage Scope + +- Test **ALL files** in the assigned directory, not just the entry `index` file +- Include all components, hooks, utilities within the path +- Goal: 100% coverage of the entire directory contents + +### Test Organization + +Choose based on directory complexity: + +1. **Single spec file (Integration approach)** - Preferred for related components + + - Minimize mocking - use real project components + - Test actual integration between components + - Only mock: API calls, complex context providers, third-party libs + +1. **Multiple spec files (Unit approach)** - For complex directories + + - One spec file per component/hook/utility + - More isolated testing + - Useful when components are independent + +### Integration Testing First + +When using a single spec file: + +- ✅ **Import real project components** directly (including base components and siblings) +- ✅ **Only mock**: API services (`@/service/*`), `next/navigation`, complex context providers +- ❌ **DO NOT mock** base components (`@/app/components/base/*`) +- ❌ **DO NOT mock** sibling/child components in the same directory + +> See [Example Structure](#example-structure) for correct import/mock patterns. + ## Testing Components with Dedicated Dependencies When a component has dedicated dependencies (custom hooks, managers, utilities) that are **only used by that component**, use the following strategy to balance integration testing and unit testing. @@ -231,8 +269,16 @@ const mockGithubStar = (status: number, body: Record, delayMs = import { render, screen, fireEvent, waitFor } from '@testing-library/react' import Component from './index' -// Mock dependencies +// ✅ Import real project components (DO NOT mock these) +// import Loading from '@/app/components/base/loading' +// import { ChildComponent } from './child-component' + +// ✅ Mock external dependencies only jest.mock('@/service/api') +jest.mock('next/navigation', () => ({ + useRouter: () => ({ push: jest.fn() }), + usePathname: () => '/test', +})) // Shared state for mocks (if needed) let mockSharedState = false @@ -379,9 +425,9 @@ describe('Component', () => { ## Coverage Goals -### ⚠️ MANDATORY: Complete Coverage in Single Generation +### ⚠️ MANDATORY: Complete Coverage Per File -Aim for 100% coverage: +When generating tests for a **single file**, aim for 100% coverage in that generation: - ✅ 100% function coverage (every exported function/method tested) - ✅ 100% statement coverage (every line executed)