test: unify i18next mocks into centralized helpers#158
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.
Review Summary by QodoConsolidate i18next mocks into centralized helper functions
WalkthroughsDescription• Create centralized i18n mock helpers in web/test/i18n-mock.ts • Remove redundant mock implementations from 16 test files • Update global mock in vitest.setup.ts to use helper functions • Update documentation and templates with new mocking patterns Diagramflowchart LR
A["Scattered i18next mocks<br/>in 16 test files"] -->|"Consolidate"| B["web/test/i18n-mock.ts<br/>Factory functions"]
B -->|"Import & use"| C["vitest.setup.ts<br/>Global mock"]
B -->|"Import & use"| D["Individual test files<br/>Custom translations"]
E["Documentation<br/>& templates"] -->|"Update with<br/>new patterns"| F["Testing best practices"]
File Changes1. web/test/i18n-mock.ts
|
Code Review by Qodo
1. TranslationMap uses interface
|
| import * as React from 'react' | ||
| import { vi } from 'vitest' | ||
|
|
||
| interface TranslationMap extends Record<string, string | string[]> {} |
There was a problem hiding this comment.
1. translationmap uses interface 📘 Rule violation ✓ Correctness
The new web/test/i18n-mock.ts introduces an interface declaration, but the standard requires using type aliases instead of interface. This may cause ESLint ts/consistent-type-definitions violations and breaks the repository’s TypeScript type-style consistency.
Agent Prompt
## Issue description
The file `web/test/i18n-mock.ts` declares `TranslationMap` using an `interface`, but the repo standard requires `type` aliases instead of `interface`.
## Issue Context
This will violate the configured `ts/consistent-type-definitions` rule and breaks type declaration consistency.
## Fix Focus Areas
- web/test/i18n-mock.ts[4-4]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 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.
2. Returnobjects returns string 🐞 Bug ✓ Correctness
createTFunction does not honor options.returnObjects, so components that call
t(...,{returnObjects:true}) will receive a string and can crash when treated as a string[]. This
affects any test relying on the global react-i18next mock and rendering such components.
Agent Prompt
### Issue description
`web/test/i18n-mock.ts` implements the global `t()` mock used by Vitest. It currently ignores `options.returnObjects`, so calls like `t(key, { returnObjects: true })` return a string. Components that cast to `string[]` and call `.map()` will crash under the global mock.
### Issue Context
At least one production component (`SelfHostedPlanItem/List`) uses `returnObjects: true` to fetch an array of features from i18n.
### Fix Focus Areas
- web/test/i18n-mock.ts[10-28]
- web/vitest.setup.ts[88-96]
- web/app/components/billing/pricing/plans/self-hosted-plan-item/list/index.tsx[13-27]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export function createTransMock(translations: TranslationMap = {}) { | ||
| return { | ||
| Trans: ({ i18nKey, children }: { | ||
| i18nKey: string | ||
| children?: React.ReactNode | ||
| }) => { | ||
| const text = translations[i18nKey] ?? i18nKey | ||
| return React.createElement('span', { 'data-i18n-key': i18nKey }, children ?? text) | ||
| }, | ||
| } |
There was a problem hiding this comment.
3. Trans omits ns/components 🐞 Bug ✓ Correctness
createTransMock ignores Trans props like ns and components, so <Trans ns=... components=.../> loses namespace context and does not render injected elements. This reduces test fidelity and can break tests that assert on Trans-rendered structure.
Agent Prompt
### Issue description
The global `Trans` mock currently drops important props (`ns`, `components`, etc.). Components using `<Trans ns="..." components={{...}} />` will not render injected nodes under tests, which can hide UI regressions.
### Issue Context
Production code uses Trans with `ns` and `components` (e.g., highlight spans). The mock should preserve enough structure for assertions.
### Fix Focus Areas
- web/test/i18n-mock.ts[54-63]
- web/vitest.setup.ts[88-96]
- web/app/components/billing/pricing/plans/self-hosted-plan-item/list/index.tsx[20-24]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Benchmark PR from agentic-review-benchmarks#7