test: unify i18next mocks into centralized helpers#7
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c622b17ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import * as React from 'react' | ||
| import { vi } from 'vitest' | ||
|
|
||
| interface TranslationMap extends Record<string, string | string[]> {} |
There was a problem hiding this comment.
The new helper declares interface TranslationMap ..., but the repo-level AGENTS.md rule "TypeScript Must Use Type Definitions Instead of Interfaces" (ts/consistent-type-definitions) treats this as a lint error. That means this change can fail CI/lint checks for the web tests. Switch this to a type TranslationMap = Record<...> to align with the enforced rule.
Useful? React with 👍 / 👎.
| // 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.
The new createTFunction always falls back to a string (and JSON-serialized options) even when options.returnObjects is passed. Components like SelfHostedPlanItem/List call t(..., { returnObjects: true }) and then do .map(...) on the result, so any test that relies on the global mock (without a local override) will now throw features.map is not a function. Consider special-casing returnObjects (e.g., return an array of keys or an empty array) to preserve prior behavior.
Useful? React with 👍 / 👎.
3 participants
Benchmark PR from agentic-review-benchmarks#7