Skip to content

test: unify i18next mocks into centralized helpers - #56

Open
tomerqodo wants to merge 5 commits into
copilot_combined_20260121_qodo_grep_cursor_copilot_1_base_test_unify_i18next_mocks_into_centralized_helpers_pr437from
copilot_combined_20260121_qodo_grep_cursor_copilot_1_head_test_unify_i18next_mocks_into_centralized_helpers_pr437
Open

test: unify i18next mocks into centralized helpers#56
tomerqodo wants to merge 5 commits into
copilot_combined_20260121_qodo_grep_cursor_copilot_1_base_test_unify_i18next_mocks_into_centralized_helpers_pr437from
copilot_combined_20260121_qodo_grep_cursor_copilot_1_head_test_unify_i18next_mocks_into_centralized_helpers_pr437

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#437

hyoban and others added 5 commits January 21, 2026 15:55
Consolidate scattered i18next mock implementations across test files into
a single source of truth. This reduces duplication and ensures consistent
mock behavior.

- Create test/i18n-mock.ts with reusable factory functions
- Update vitest.setup.ts to use the centralized helpers
- Remove redundant mock definitions from 8 test files
- Update testing.md documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…eateReactI18nextMock` and detail global mock provisions.
Copilot AI review requested due to automatic review settings January 21, 2026 18:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes i18next mocking by introducing reusable helper functions in web/test/i18n-mock.ts and updating the global mock in web/vitest.setup.ts to use these helpers. The goal is to reduce code duplication across test files and standardize i18n mocking patterns.

Changes:

  • Created centralized i18n mock helpers (createTFunction, createUseTranslationMock, createTransMock, createReactI18nextMock)
  • Updated global mock in vitest.setup.ts to use the new helper
  • Removed local i18n mocks from 10+ test files, relying on global mock or new helpers
  • Updated documentation and test templates to reference the centralized approach

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
web/vitest.setup.ts Updated global react-i18next mock to use centralized helper
web/test/i18n-mock.ts New file with centralized i18n mock helper functions
web/testing/testing.md Updated testing guidelines to document centralized mock usage
.claude/skills/frontend-testing/references/mocking.md Updated mocking reference documentation
.claude/skills/frontend-testing/assets/component-test.template.tsx Updated test template with new mocking approach
web/app/components/plugins/plugin-mutation-model/index.spec.tsx Removed local i18n mock (now uses global)
web/app/components/plugins/plugin-detail-panel/subscription-list/edit/index.spec.tsx Removed local i18n mock (now uses global)
web/app/components/plugins/plugin-detail-panel/subscription-list/create/oauth-client.spec.tsx Removed local i18n mock (now uses global)
web/app/components/plugins/plugin-detail-panel/subscription-list/create/common-modal.spec.tsx Removed local i18n mock (now uses global)
web/app/components/plugins/marketplace/index.spec.tsx Updated i18next-config mock and test assertions for namespace handling
web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.spec.tsx Removed local i18n mock (now uses global)
web/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsx Updated to use centralized helper with custom Trans override
web/app/components/plugins/card/index.spec.tsx Removed local i18n mocks including useMixedTranslation and useGetLanguage
web/app/components/datasets/documents/create-from-pipeline/processing/index.spec.tsx Removed local i18n mock (now uses global)
web/app/components/datasets/create/index.spec.tsx Removed local i18n mock (now uses global)
web/app/components/billing/pricing/footer.spec.tsx Removed local i18n mock and translation state variable
web/app/components/base/input/index.spec.tsx Updated to use centralized helper with custom translations
web/app/components/base/input-with-copy/index.spec.tsx Updated to use centralized helper with custom translations
web/app/components/base/inline-delete-confirm/index.spec.tsx Updated to use centralized helper with custom translations
web/app/components/app/configuration/config/config-audio.spec.tsx Removed local i18n mock (now uses global)
web/app/components/app/configuration/config/agent-setting-button.spec.tsx Removed local i18n mock (now uses global)

Comment on lines +31 to +32
// The global mock provides: useTranslation, Trans, useMixedTranslation, useGetLanguage
// No explicit mock needed for most tests

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment incorrectly states that the global mock provides useMixedTranslation and useGetLanguage. Looking at web/vitest.setup.ts, the global mock only covers react-i18next exports (useTranslation, Trans, etc.). The hooks useMixedTranslation (from @/app/components/plugins/marketplace/hooks) and useGetLanguage (from @/context/i18n) are not mocked globally and must be mocked locally in test files that use components which depend on these hooks.

Suggested change
// The global mock provides: useTranslation, Trans, useMixedTranslation, useGetLanguage
// No explicit mock needed for most tests
// The global mock provides core react-i18next exports like useTranslation and Trans
// Hooks like useMixedTranslation and useGetLanguage are NOT globally mocked; mock them locally when needed

Copilot uses AI. Check for mistakes.
Comment thread web/test/i18n-mock.ts

/**
* Create a t function with optional custom translations
* Checks translations[key] first, then translations[ns.key], then returns ns.key as fallback

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment incorrectly describes the lookup order. The code checks translations[fullKey] first (which is ns.key when namespace is present), then translations[key] without namespace. The comment says "Checks translations[key] first, then translations[ns.key]" which is the opposite order. The comment should be corrected to: "Checks translations[ns.key] first, then translations[key], then returns ns.key as fallback"

Suggested change
* Checks translations[key] first, then translations[ns.key], then returns ns.key as fallback
* Checks translations[ns.key] first, then translations[key], then returns ns.key as fallback

Copilot uses AI. Check for mistakes.
Comment thread web/test/i18n-mock.ts
Comment on lines +10 to +29
export function createTFunction(translations: TranslationMap, defaultNs?: string) {
return (key: string, options?: Record<string, unknown>) => {
const ns = (options?.ns as string | undefined) ?? defaultNs
const fullKey = ns ? `${ns}.${key}` : key

// Check custom translations with namespace first
if (translations[fullKey] !== undefined)
return translations[fullKey]

// Check custom translations without namespace
if (translations[key] !== undefined)
return translations[key]

// Serialize params (excluding ns) for test assertions
const params = { ...options }
delete params.ns
const suffix = Object.keys(params).length > 0 ? `:${JSON.stringify(params)}` : ''
return `${fullKey}${suffix}`
}
}

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The createTFunction does not handle the returnObjects option. The old global mock implementation supported returnObjects by returning an array when this option was present. This functionality is used in several test files (e.g., web/app/components/app/overview/settings/index.spec.tsx, web/app/components/billing/pricing/index.spec.tsx). Without this support, tests that rely on returnObjects will fail or behave incorrectly.

The function should check for options?.returnObjects and return an array from the custom translations if available, or a default array pattern if not found in translations.

Copilot uses AI. Check for mistakes.
Comment thread web/test/i18n-mock.ts
Comment on lines +39 to +47
export function createUseTranslationMock(translations: TranslationMap = {}) {
return {
useTranslation: () => ({
t: createTFunction(translations),
i18n: {
language: 'en',
changeLanguage: vi.fn(),
},
}),

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useTranslation mock function should accept a defaultNs parameter to match the react-i18next API. The old global mock implementation accepted this parameter: useTranslation: (defaultNs?: string) => ({...}). This allows tests to specify a default namespace that will be used when no namespace is explicitly provided in translation calls. Without this, tests that pass a default namespace to useTranslation will not work correctly.

Copilot uses AI. Check for mistakes.
Comment thread web/test/i18n-mock.ts
Comment on lines +56 to +61
Trans: ({ i18nKey, children }: {
i18nKey: string
children?: React.ReactNode
}) => {
const text = translations[i18nKey] ?? i18nKey
return React.createElement('span', { 'data-i18n-key': i18nKey }, children ?? text)

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Trans component mock is missing support for the components prop, which is commonly used in react-i18next for rendering specific parts of translated text with custom components. The test file web/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsx explicitly overrides this mock to add components support. The base mock should handle this prop to provide more complete functionality out of the box.

Suggested change
Trans: ({ i18nKey, children }: {
i18nKey: string
children?: React.ReactNode
}) => {
const text = translations[i18nKey] ?? i18nKey
return React.createElement('span', { 'data-i18n-key': i18nKey }, children ?? text)
Trans: ({ i18nKey, children, components }: {
i18nKey: string
children?: React.ReactNode
components?: React.ReactNode | React.ReactNode[]
}) => {
const text = translations[i18nKey] ?? i18nKey
const content = components != null && children == null ? components : (children ?? text)
return React.createElement('span', { 'data-i18n-key': i18nKey }, content)

Copilot uses AI. Check for mistakes.
Comment on lines 21 to 23
// Mock External Dependencies Only
// ================================

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mocks for useMixedTranslation and useGetLanguage were removed from this test file, but these hooks are actually used by the Card component. The component imports useMixedTranslation from @/app/components/plugins/marketplace/hooks (line 56 of index.tsx) and useGetLanguage from @/context/i18n (line 54 of index.tsx). These hooks are not mocked in the global vitest.setup.ts file, so removing these mocks will cause tests to fail when they try to render the Card component.

Copilot uses AI. Check for mistakes.
Comment thread web/testing/testing.md
Comment on lines +336 to +340
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`

**Default behavior**: Most tests should use the global mock (no local override needed).

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation incorrectly states that the global mock provides useMixedTranslation and useGetLanguage. Looking at web/vitest.setup.ts, the global mock only covers react-i18next exports (useTranslation, Trans, etc.). The hooks useMixedTranslation (from @/app/components/plugins/marketplace/hooks) and useGetLanguage (from @/context/i18n) are not mocked globally and must be mocked locally in test files that use components which depend on these hooks.

Suggested change
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`
**Default behavior**: Most tests should use the global mock (no local override needed).
**Important**: App-specific hooks like:
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`)
are **not** part of the global mock and must be mocked **locally** in each test file that uses components depending on them, for example:
```typescript
vi.mock('@/context/i18n', () => ({
useGetLanguage: () => 'en-US',
}))
vi.mock('@/app/components/plugins/marketplace/hooks', () => ({
useMixedTranslation: () => ({
t: (key: string) => key,
}),
}))

Default behavior: Most tests should use the global mock (no local override needed) for react-i18next.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +62
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation incorrectly states that the global mock provides useMixedTranslation and useGetLanguage. Looking at web/vitest.setup.ts, the global mock only covers react-i18next exports (useTranslation, Trans, etc.). The hooks useMixedTranslation (from @/app/components/plugins/marketplace/hooks) and useGetLanguage (from @/context/i18n) are not mocked globally and must be mocked locally in test files that use components which depend on these hooks.

Suggested change
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`
**Note:** `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`) and `useGetLanguage` (from `@/context/i18n`) are **not** mocked globally. Mock them locally in test files that render components depending on these hooks.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants