From e8397ae7a8ec6b818d2d204a7ec5f82f05839a2e Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Mon, 19 Jan 2026 10:31:34 +0800 Subject: [PATCH] fix(web): Zustand testing best practices and state read optimization (#31163) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .claude/skills/frontend-testing/SKILL.md | 5 +- .../frontend-testing/references/mocking.md | 165 +++++++++++++++++- .../debug-with-multiple-model/index.spec.tsx | 10 +- .../debug-with-single-model/index.spec.tsx | 13 +- .../prompt-value-panel/index.spec.tsx | 20 +-- .../app/switch-app-modal/index.spec.tsx | 16 +- web/app/components/apps/list.spec.tsx | 19 +- .../rag-pipeline/components/index.spec.tsx | 10 ++ .../panel/input-field/index.spec.tsx | 5 + .../publish-as-knowledge-pipeline-modal.tsx | 9 +- .../rag-pipeline-header/index.spec.tsx | 2 + .../components/rag-pipeline-header/index.tsx | 5 +- .../workflow-header/features-trigger.spec.tsx | 13 +- .../components/workflow-header/index.spec.tsx | 46 ++--- .../components/variable-modal.tsx | 5 +- .../panel/env-panel/variable-modal.tsx | 9 +- .../version-history-panel/index.spec.tsx | 2 + web/eslint-suppressions.json | 9 +- 18 files changed, 257 insertions(+), 106 deletions(-) diff --git a/.claude/skills/frontend-testing/SKILL.md b/.claude/skills/frontend-testing/SKILL.md index dd9677a78e..0716c81ef7 100644 --- a/.claude/skills/frontend-testing/SKILL.md +++ b/.claude/skills/frontend-testing/SKILL.md @@ -83,6 +83,9 @@ vi.mock('next/navigation', () => ({ usePathname: () => '/test', })) +// ✅ Zustand stores: Use real stores (auto-mocked globally) +// Set test state with: useAppStore.setState({ ... }) + // Shared state for mocks (if needed) let mockSharedState = false @@ -296,7 +299,7 @@ For each test file generated, aim for: For more detailed information, refer to: - `references/workflow.md` - **Incremental testing workflow** (MUST READ for multi-file testing) -- `references/mocking.md` - Mock patterns and best practices +- `references/mocking.md` - Mock patterns, Zustand store testing, and best practices - `references/async-testing.md` - Async operations and API calls - `references/domain-components.md` - Workflow, Dataset, Configuration testing - `references/common-patterns.md` - Frequently used testing patterns diff --git a/.claude/skills/frontend-testing/references/mocking.md b/.claude/skills/frontend-testing/references/mocking.md index c70bcf0ae5..86bd375987 100644 --- a/.claude/skills/frontend-testing/references/mocking.md +++ b/.claude/skills/frontend-testing/references/mocking.md @@ -37,16 +37,36 @@ Only mock these categories: 1. **Third-party libraries with side effects** - `next/navigation`, external SDKs 1. **i18n** - Always mock to return keys +### Zustand Stores - DO NOT Mock Manually + +**Zustand is globally mocked** in `web/vitest.setup.ts`. Use real stores with `setState()`: + +```typescript +// ✅ CORRECT: Use real store, set test state +import { useAppStore } from '@/app/components/app/store' + +useAppStore.setState({ appDetail: { id: 'test', name: 'Test' } }) +render() + +// ❌ WRONG: Don't mock the store module +vi.mock('@/app/components/app/store', () => ({ ... })) +``` + +See [Zustand Store Testing](#zustand-store-testing) section for full details. + ## Mock Placement | Location | Purpose | |----------|---------| -| `web/vitest.setup.ts` | Global mocks shared by all tests (for example `react-i18next`, `next/image`) | +| `web/vitest.setup.ts` | Global mocks shared by all tests (`react-i18next`, `next/image`, `zustand`) | +| `web/__mocks__/zustand.ts` | Zustand mock implementation (auto-resets stores after each test) | | `web/__mocks__/` | Reusable mock factories shared across multiple test files | | Test file | Test-specific mocks, inline with `vi.mock()` | Modules are not mocked automatically. Use `vi.mock` in test files, or add global mocks in `web/vitest.setup.ts`. +**Note**: Zustand is special - it's globally mocked but you should NOT mock store modules manually. See [Zustand Store Testing](#zustand-store-testing). + ## Essential Mocks ### 1. i18n (Auto-loaded via Global Mock) @@ -276,6 +296,7 @@ const renderWithQueryClient = (ui: React.ReactElement) => { 1. **Use real base components** - Import from `@/app/components/base/` directly 1. **Use real project components** - Prefer importing over mocking +1. **Use real Zustand stores** - Set test state via `store.setState()` 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 @@ -285,6 +306,7 @@ const renderWithQueryClient = (ui: React.ReactElement) => { ### ❌ DON'T 1. **Don't mock base components** (`Loading`, `Button`, `Tooltip`, etc.) +1. **Don't mock Zustand store modules** - Use real stores with `setState()` 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 @@ -308,10 +330,151 @@ Need to use a component in test? ├─ Is it a third-party lib with side effects? │ └─ YES → Mock it (next/navigation, external SDKs) │ +├─ Is it a Zustand store? +│ └─ YES → DO NOT mock the module! +│ Use real store + setState() to set test state +│ (Global mock handles auto-reset) +│ └─ Is it i18n? └─ YES → Uses shared mock (auto-loaded). Override only for custom translations ``` +## Zustand Store Testing + +### Global Zustand Mock (Auto-loaded) + +Zustand is globally mocked in `web/vitest.setup.ts` following the [official Zustand testing guide](https://zustand.docs.pmnd.rs/guides/testing). The mock in `web/__mocks__/zustand.ts` provides: + +- Real store behavior with `getState()`, `setState()`, `subscribe()` methods +- Automatic store reset after each test via `afterEach` +- Proper test isolation between tests + +### ✅ Recommended: Use Real Stores (Official Best Practice) + +**DO NOT mock store modules manually.** Import and use the real store, then use `setState()` to set test state: + +```typescript +// ✅ CORRECT: Use real store with setState +import { useAppStore } from '@/app/components/app/store' + +describe('MyComponent', () => { + it('should render app details', () => { + // Arrange: Set test state via setState + useAppStore.setState({ + appDetail: { + id: 'test-app', + name: 'Test App', + mode: 'chat', + }, + }) + + // Act + render() + + // Assert + expect(screen.getByText('Test App')).toBeInTheDocument() + // Can also verify store state directly + expect(useAppStore.getState().appDetail?.name).toBe('Test App') + }) + + // No cleanup needed - global mock auto-resets after each test +}) +``` + +### ❌ Avoid: Manual Store Module Mocking + +Manual mocking conflicts with the global Zustand mock and loses store functionality: + +```typescript +// ❌ WRONG: Don't mock the store module +vi.mock('@/app/components/app/store', () => ({ + useStore: (selector) => mockSelector(selector), // Missing getState, setState! +})) + +// ❌ WRONG: This conflicts with global zustand mock +vi.mock('@/app/components/workflow/store', () => ({ + useWorkflowStore: vi.fn(() => mockState), +})) +``` + +**Problems with manual mocking:** + +1. Loses `getState()`, `setState()`, `subscribe()` methods +1. Conflicts with global Zustand mock behavior +1. Requires manual maintenance of store API +1. Tests don't reflect actual store behavior + +### When Manual Store Mocking is Necessary + +In rare cases where the store has complex initialization or side effects, you can mock it, but ensure you provide the full store API: + +```typescript +// If you MUST mock (rare), include full store API +const mockStore = { + appDetail: { id: 'test', name: 'Test' }, + setAppDetail: vi.fn(), +} + +vi.mock('@/app/components/app/store', () => ({ + useStore: Object.assign( + (selector: (state: typeof mockStore) => unknown) => selector(mockStore), + { + getState: () => mockStore, + setState: vi.fn(), + subscribe: vi.fn(), + }, + ), +})) +``` + +### Store Testing Decision Tree + +``` +Need to test a component using Zustand store? +│ +├─ Can you use the real store? +│ └─ YES → Use real store + setState (RECOMMENDED) +│ useAppStore.setState({ ... }) +│ +├─ Does the store have complex initialization/side effects? +│ └─ YES → Consider mocking, but include full API +│ (getState, setState, subscribe) +│ +└─ Are you testing the store itself (not a component)? + └─ YES → Test store directly with getState/setState + const store = useMyStore + store.setState({ count: 0 }) + store.getState().increment() + expect(store.getState().count).toBe(1) +``` + +### Example: Testing Store Actions + +```typescript +import { useCounterStore } from '@/stores/counter' + +describe('Counter Store', () => { + it('should increment count', () => { + // Initial state (auto-reset by global mock) + expect(useCounterStore.getState().count).toBe(0) + + // Call action + useCounterStore.getState().increment() + + // Verify state change + expect(useCounterStore.getState().count).toBe(1) + }) + + it('should reset to initial state', () => { + // Set some state + useCounterStore.setState({ count: 100 }) + expect(useCounterStore.getState().count).toBe(100) + + // After this test, global mock will reset to initial state + }) +}) +``` + ## Factory Function Pattern ```typescript diff --git a/web/app/components/app/configuration/debug/debug-with-multiple-model/index.spec.tsx b/web/app/components/app/configuration/debug/debug-with-multiple-model/index.spec.tsx index 05a22c5153..188086246a 100644 --- a/web/app/components/app/configuration/debug/debug-with-multiple-model/index.spec.tsx +++ b/web/app/components/app/configuration/debug/debug-with-multiple-model/index.spec.tsx @@ -7,6 +7,7 @@ import type { FileEntity } from '@/app/components/base/file-uploader/types' import type { Inputs, ModelConfig } from '@/models/debug' import type { PromptVariable } from '@/types/app' import { fireEvent, render, screen } from '@testing-library/react' +import { useStore as useAppStore } from '@/app/components/app/store' import { DEFAULT_AGENT_SETTING, DEFAULT_CHAT_PROMPT_CONFIG, DEFAULT_COMPLETION_PROMPT_CONFIG } from '@/config' import { AppModeEnum, ModelModeType, Resolution, TransferMethod } from '@/types/app' import { APP_CHAT_WITH_MULTIPLE_MODEL } from '../types' @@ -21,9 +22,7 @@ type PromptVariableWithMeta = Omit & { const mockUseDebugConfigurationContext = vi.fn() const mockUseFeaturesSelector = vi.fn() const mockUseEventEmitterContext = vi.fn() -const mockUseAppStoreSelector = vi.fn() const mockEventEmitter = { emit: vi.fn() } -const mockSetShowAppConfigureFeaturesModal = vi.fn() let capturedChatInputProps: MockChatInputAreaProps | null = null let modelIdCounter = 0 let featureState: FeatureStoreState @@ -63,10 +62,6 @@ vi.mock('@/context/event-emitter', () => ({ useEventEmitterContextContext: () => mockUseEventEmitterContext(), })) -vi.mock('@/app/components/app/store', () => ({ - useStore: (selector: (state: { setShowAppConfigureFeaturesModal: typeof mockSetShowAppConfigureFeaturesModal }) => unknown) => mockUseAppStoreSelector(selector), -})) - vi.mock('./debug-item', () => ({ default: ({ modelAndParameter, @@ -191,7 +186,6 @@ describe('DebugWithMultipleModel', () => { featureState = createFeatureState() mockUseFeaturesSelector.mockImplementation(selector => selector(featureState)) mockUseEventEmitterContext.mockReturnValue({ eventEmitter: mockEventEmitter }) - mockUseAppStoreSelector.mockImplementation(selector => selector({ setShowAppConfigureFeaturesModal: mockSetShowAppConfigureFeaturesModal })) mockUseDebugConfigurationContext.mockReturnValue(createDebugConfiguration()) }) @@ -438,7 +432,7 @@ describe('DebugWithMultipleModel', () => { expect(capturedChatInputProps?.showFileUpload).toBe(false) expect(capturedChatInputProps?.speechToTextConfig).toEqual(featureState.features.speech2text) expect(capturedChatInputProps?.visionConfig).toEqual(featureState.features.file) - expect(mockSetShowAppConfigureFeaturesModal).toHaveBeenCalledWith(true) + expect(useAppStore.getState().showAppConfigureFeaturesModal).toBe(true) }) it('should render chat input in agent chat mode', () => { diff --git a/web/app/components/app/configuration/debug/debug-with-single-model/index.spec.tsx b/web/app/components/app/configuration/debug/debug-with-single-model/index.spec.tsx index 151038d787..b9a1c5ba8b 100644 --- a/web/app/components/app/configuration/debug/debug-with-single-model/index.spec.tsx +++ b/web/app/components/app/configuration/debug/debug-with-single-model/index.spec.tsx @@ -7,6 +7,7 @@ import type { ProviderContextState } from '@/context/provider-context' import type { DatasetConfigs, ModelConfig } from '@/models/debug' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import { createRef } from 'react' +import { useStore as useAppStore } from '@/app/components/app/store' import { ConfigurationMethodEnum, ModelFeatureEnum, ModelStatusEnum, ModelTypeEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { CollectionType } from '@/app/components/tools/types' import { PromptMode } from '@/models/debug' @@ -376,15 +377,7 @@ vi.mock('../hooks', () => ({ useFormattingChangedSubscription: mockUseFormattingChangedSubscription, })) -const mockSetShowAppConfigureFeaturesModal = vi.fn() - -vi.mock('@/app/components/app/store', () => ({ - useStore: vi.fn((selector?: (state: { setShowAppConfigureFeaturesModal: typeof mockSetShowAppConfigureFeaturesModal }) => unknown) => { - if (typeof selector === 'function') - return selector({ setShowAppConfigureFeaturesModal: mockSetShowAppConfigureFeaturesModal }) - return mockSetShowAppConfigureFeaturesModal - }), -})) +// Use real store - global zustand mock will auto-reset between tests // Mock event emitter context vi.mock('@/context/event-emitter', () => ({ @@ -659,7 +652,7 @@ describe('DebugWithSingleModel', () => { fireEvent.click(screen.getByTestId('feature-bar-button')) - expect(mockSetShowAppConfigureFeaturesModal).toHaveBeenCalledWith(true) + expect(useAppStore.getState().showAppConfigureFeaturesModal).toBe(true) }) }) diff --git a/web/app/components/app/configuration/prompt-value-panel/index.spec.tsx b/web/app/components/app/configuration/prompt-value-panel/index.spec.tsx index d0c6f02308..96c95e1cc8 100644 --- a/web/app/components/app/configuration/prompt-value-panel/index.spec.tsx +++ b/web/app/components/app/configuration/prompt-value-panel/index.spec.tsx @@ -2,14 +2,11 @@ import type { IPromptValuePanelProps } from './index' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { useStore } from '@/app/components/app/store' import ConfigContext from '@/context/debug-configuration' import { AppModeEnum, ModelModeType, Resolution } from '@/types/app' import PromptValuePanel from './index' -vi.mock('@/app/components/app/store', () => ({ - useStore: vi.fn(), -})) +// Use real store - global zustand mock will auto-reset between tests vi.mock('@/app/components/base/features/new-feature-panel/feature-bar', () => ({ default: ({ onFeatureBarClick }: { onFeatureBarClick: () => void }) => (