-
Notifications
You must be signed in to change notification settings - Fork 0
test: unify i18next mocks into centralized helpers #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: coderabbit_full_base_test_unify_i18next_mocks_into_centralized_helpers_pr7
Are you sure you want to change the base?
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.
WalkthroughThis PR centralizes i18n (react-i18next) mocking across the test suite. A new test utility module Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/test/i18n-mock.ts`:
- Around line 6-9: Update the docstring in web/test/i18n-mock.ts to match the
actual behavior of the t function: state that the implementation checks the
namespaced key (fullKey / `${ns}.${key}`) first, then falls back to the simple
key (`key`), and finally returns the namespaced key as the default fallback;
reference the t function / createT helper and the variables fullKey and key when
editing the comment to accurately reflect the precedence order used in the code.
🧹 Nitpick comments (1)
web/test/i18n-mock.ts (1)
4-4: Consider simplifying the interface declaration.The empty interface body
{}is redundant since it only extendsRecord<string, string | string[]>. Consider using a type alias instead.♻️ Suggested simplification
-interface TranslationMap extends Record<string, string | string[]> {} +type TranslationMap = Record<string, string | string[]>
📜 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/card/index.spec.tsx
- web/app/components/datasets/create/index.spec.tsx
- 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/plugins/plugin-detail-panel/subscription-list/edit/index.spec.tsx
- web/app/components/app/configuration/config/config-audio.spec.tsx
- web/app/components/plugins/plugin-mutation-model/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/app/configuration/config/agent-setting-button.spec.tsx
- web/app/components/billing/pricing/footer.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/base/input/index.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/vitest.setup.ts (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)
🔇 Additional comments (16)
.claude/skills/frontend-testing/references/mocking.md (1)
56-77: Documentation updates look good.The documentation clearly explains the global mock behavior and when to override with custom translations. The example using
createReactI18nextMockis consistent with the actual implementation inweb/test/i18n-mock.ts.web/test/i18n-mock.ts (2)
39-49: LGTM - useTranslation mock is appropriately simple for testing.The mock provides the essential
tfunction andi18nobject. Tests can use fully qualified translation keys (e.g.,'namespace.key') in their translation maps to handle namespacing.
74-79: Clean composition pattern for the combined mock.The factory function correctly combines both
useTranslationandTransmocks, providing a convenient single-function API for most test scenarios.web/vitest.setup.ts (1)
89-96: Good use of centralized mock factory in global setup.The implementation correctly:
- Imports the actual
react-i18nextmodule to preserve other exports- Uses dynamic import for the mock helper (required since mocks are hoisted)
- Spreads the centralized mock to override
useTranslationandTransThis ensures all tests get consistent i18n mock behavior by default.
.claude/skills/frontend-testing/assets/component-test.template.tsx (1)
31-39: Template comments updated correctly.The updated comments clearly document:
- What the global mock provides (useTranslation, Trans, useMixedTranslation, useGetLanguage)
- When no local override is needed
- How to override with custom translations using the factory function
This provides good guidance for developers writing new tests.
web/app/components/plugins/install-plugin/install-from-local-package/steps/install.spec.tsx (1)
67-80: Good use of centralized mock with custom Trans override.This test correctly:
- Imports the actual module with
importOriginal- Uses the centralized
createReactI18nextMock()foruseTranslation- Provides a custom
Transimplementation that handles thecomponents.trustSourceprop needed by this specific componentThis is the appropriate pattern when tests need specialized mock behavior beyond what the centralized helper provides.
web/app/components/base/input/index.spec.tsx (1)
3-10: Clean migration to centralized i18n mock.The test correctly uses
createReactI18nextMockwith the specific translations needed for assertions (e.g., testing that the placeholder shows "Please input" and the search input shows "Search"). This replaces what was likely a more verbose inline mock.web/app/components/base/inline-delete-confirm/index.spec.tsx (1)
3-12: Centralized i18n mock wiring looks good. The helper-based mock with explicit translations keeps assertions deterministic and consistent across tests.web/app/components/plugins/marketplace/index.spec.tsx (6)
33-40: Namespace-aware getFixedT mock matches intended behavior. The prefixing logic aligns downstream expectations with the updated i18n strategy.
620-622: Expectation updated for namespace prefix. Matches the new mock behavior.
641-642: Namespaced key assertion is aligned with getFixedT. Looks consistent.
2759-2767: Namespaced label expectations are consistent with the mock. Good update.
2797-2797: Click target now uses namespaced label. This keeps the interaction test in sync with mock output.
2819-2819: Active state lookup uses namespaced key. Consistent with updated render text.web/app/components/base/input-with-copy/index.spec.tsx (1)
3-19: Centralized i18n mock adoption looks good. The explicit translations keep copy-state assertions deterministic.web/testing/testing.md (1)
332-352: Docs now clearly reflect the centralized i18n mock flow. The guidance and example should keep test mocks consistent.
✏️ 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 | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring is inconsistent with implementation.
The docstring says "Checks translations[key] first, then translations[ns.key]" but the actual implementation (lines 16-21) checks fullKey (ns.key) first, then falls back to key. The implementation is correct (more specific key should take precedence), but the docstring should be updated.
📝 Suggested fix
/**
* Create a t function with optional custom translations
- * 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 | |
| */ | |
| /** | |
| * Create a t function with optional custom translations | |
| * Checks translations[ns.key] first, then translations[key], then returns ns.key as fallback | |
| */ |
🤖 Prompt for AI Agents
In `@web/test/i18n-mock.ts` around lines 6 - 9, Update the docstring in
web/test/i18n-mock.ts to match the actual behavior of the t function: state that
the implementation checks the namespaced key (fullKey / `${ns}.${key}`) first,
then falls back to the simple key (`key`), and finally returns the namespaced
key as the default fallback; reference the t function / createT helper and the
variables fullKey and key when editing the comment to accurately reflect the
precedence order used in the code.
Benchmark PR from agentic-review-benchmarks#7
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.