test: unify i18next mocks into centralized helpers#20
Conversation
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.
WalkthroughA new i18n test mock utility is introduced in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/test/i18n-mock.ts`:
- Around line 6-28: The comment above createTFunction is incorrect about lookup
order: the implementation builds fullKey (ns ? `${ns}.${key}` : key) and checks
translations[fullKey] first then translations[key]; update the comment to state
that the function checks the namespaced key (fullKey) first, then the raw key,
and then returns `${fullKey}` with serialized params as fallback so it matches
the logic around createTFunction, fullKey, translations and params.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.claude/skills/frontend-testing/assets/component-test.template.tsx.claude/skills/frontend-testing/references/mocking.mdweb/app/components/app/configuration/config/agent-setting-button.spec.tsxweb/app/components/app/configuration/config/config-audio.spec.tsxweb/app/components/base/inline-delete-confirm/index.spec.tsxweb/app/components/base/input-with-copy/index.spec.tsxweb/app/components/base/input/index.spec.tsxweb/app/components/billing/pricing/footer.spec.tsxweb/app/components/datasets/create/index.spec.tsxweb/app/components/datasets/documents/create-from-pipeline/processing/index.spec.tsxweb/app/components/plugins/card/index.spec.tsxweb/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsxweb/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.spec.tsxweb/app/components/plugins/marketplace/index.spec.tsxweb/app/components/plugins/plugin-detail-panel/subscription-list/create/common-modal.spec.tsxweb/app/components/plugins/plugin-detail-panel/subscription-list/create/oauth-client.spec.tsxweb/app/components/plugins/plugin-detail-panel/subscription-list/edit/index.spec.tsxweb/app/components/plugins/plugin-mutation-model/index.spec.tsxweb/test/i18n-mock.tsweb/testing/testing.mdweb/vitest.setup.ts
💤 Files with no reviewable changes (11)
- web/app/components/plugins/plugin-detail-panel/subscription-list/create/oauth-client.spec.tsx
- web/app/components/datasets/documents/create-from-pipeline/processing/index.spec.tsx
- web/app/components/app/configuration/config/config-audio.spec.tsx
- web/app/components/billing/pricing/footer.spec.tsx
- web/app/components/plugins/card/index.spec.tsx
- web/app/components/plugins/plugin-detail-panel/subscription-list/create/common-modal.spec.tsx
- web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.spec.tsx
- web/app/components/datasets/create/index.spec.tsx
- web/app/components/plugins/plugin-detail-panel/subscription-list/edit/index.spec.tsx
- web/app/components/plugins/plugin-mutation-model/index.spec.tsx
- web/app/components/app/configuration/config/agent-setting-button.spec.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
web/app/components/base/inline-delete-confirm/index.spec.tsx (1)
web/test/i18n-mock.ts (1)
createReactI18nextMock(74-79)
web/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsx (1)
web/test/i18n-mock.ts (1)
createReactI18nextMock(74-79)
web/app/components/base/input-with-copy/index.spec.tsx (1)
web/test/i18n-mock.ts (1)
createReactI18nextMock(74-79)
web/app/components/base/input/index.spec.tsx (1)
web/test/i18n-mock.ts (1)
createReactI18nextMock(74-79)
web/vitest.setup.ts (1)
web/test/i18n-mock.ts (1)
createReactI18nextMock(74-79)
🔇 Additional comments (16)
web/app/components/base/inline-delete-confirm/index.spec.tsx (1)
3-12: LGTM - Clean migration to centralized i18n mock.The synchronous mock factory pattern is correctly used here. Translation keys align with test assertions (
Delete?,Yes,No).Minor observation:
'operation.confirmAction'(Line 11) is defined but doesn't appear to be asserted in any test. If this key isn't used by the component under test, consider removing it to keep the mock minimal.web/app/components/base/input/index.spec.tsx (1)
3-10: LGTM - Consistent migration pattern.The mock setup correctly provides the two translation keys used by the Input component's default placeholder and search functionality. Tests properly assert against the translated values.
web/vitest.setup.ts (1)
89-96: LGTM - Proper global mock setup with dynamic import.The async factory pattern correctly:
- Preserves actual module exports via
importActual- Dynamically imports the centralized mock helper
- Spreads in order so mock exports override actual ones
This allows individual tests to either rely on the global mock or override with custom translations using
createReactI18nextMock({...}).web/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsx (1)
67-80: LGTM - Justified custom Trans override.This test correctly uses the centralized mock while preserving a custom
Transcomponent. The custom implementation is necessary because:
- The test at Line 151 verifies the trust source message renders
- The
trustSourcecomponent prop (Line 76) is part of the test's assertion requirementsThe pattern of spreading
createReactI18nextMock()then overriding specific exports is the right approach when partial customization is needed.web/app/components/base/input-with-copy/index.spec.tsx (1)
3-20: LGTM - Migration looks good.The mock correctly provides translation keys for copy/copied states. The test at Line 94 properly asserts the
'Copied'translation appears.Note: Both
'operation.*'and'overview.appInfo.embedded.*'namespaces are provided. If the component only uses one namespace, consider removing the unused keys to keep the mock minimal. This is a minor optimization..claude/skills/frontend-testing/references/mocking.md (1)
56-77: LGTM - Clear and helpful documentation update.The documentation effectively explains:
- What the global mock provides out-of-the-box (Lines 58-61)
- When to use the default vs. custom translations (Lines 63-65)
- Correct usage pattern with code example (Lines 67-74)
- Anti-pattern to avoid (Lines 76-77)
This will help developers understand when and how to use the centralized mock.
.claude/skills/frontend-testing/assets/component-test.template.tsx (1)
31-38: i18n guidance aligns with centralized mock usage.
Clear and consistent with the new helper-based approach.web/testing/testing.md (1)
330-352: Documentation updates are consistent and actionable.web/test/i18n-mock.ts (3)
39-48: createUseTranslationMock looks good.
54-63: createTransMock looks good.
74-78: createReactI18nextMock composition is clean.web/app/components/plugins/marketplace/index.spec.tsx (5)
33-40: Mock update is consistent with namespaced key expectations.
617-643: Namespace-prefixed assertions look correct.
2759-2767: Updated labels align with the new mock output.
2797-2799: Click target updated consistently with new labels.
2818-2820: Active-state assertion aligns with updated text output.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Create a t function with optional custom translations | ||
| * Checks translations[key] first, then translations[ns.key], then returns ns.key as fallback | ||
| */ | ||
| 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}` | ||
| } |
There was a problem hiding this comment.
Fix comment to match lookup order.
The comment says raw key is checked first, but the code checks namespaced key first. Align the comment (or logic) to avoid confusion.
💡 Suggested fix
- * 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📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Create a t function with optional custom translations | |
| * Checks translations[key] first, then translations[ns.key], then returns ns.key as fallback | |
| */ | |
| 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}` | |
| } | |
| /** | |
| * Create a t function with optional custom translations | |
| * Checks translations[ns.key] first, then translations[key], then returns ns.key as fallback | |
| */ | |
| 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}` | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@web/test/i18n-mock.ts` around lines 6 - 28, The comment above createTFunction
is incorrect about lookup order: the implementation builds fullKey (ns ?
`${ns}.${key}` : key) and checks translations[fullKey] first then
translations[key]; update the comment to state that the function checks the
namespaced key (fullKey) first, then the raw key, and then returns `${fullKey}`
with serialized params as fallback so it matches the logic around
createTFunction, fullKey, translations and params.
Benchmark PR from qodo-benchmark#437
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.