-
Notifications
You must be signed in to change notification settings - Fork 0
test: unify i18next mocks into centralized helpers #20
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: devin_pr_code_review_bench_100_devin2_base_test_unify_i18next_mocks_into_centralized_helpers_pr437
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 <[email protected]>
…eateReactI18nextMock` and detail global mock provisions.
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.
| export function createUseTranslationMock(translations: TranslationMap = {}) { | ||
| return { | ||
| useTranslation: () => ({ | ||
| t: createTFunction(translations), | ||
| i18n: { | ||
| language: 'en', | ||
| changeLanguage: vi.fn(), | ||
| }, | ||
| }), | ||
| } |
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.
🟡 New i18n mock loses useTranslation(defaultNs) parameter support causing namespace prefix regression
The new createUseTranslationMock function does not pass the defaultNs parameter from useTranslation() to createTFunction(), causing a regression in behavior.
Click to expand
Issue
The original mock in vitest.setup.ts accepted a defaultNs parameter:
useTranslation: (defaultNs?: string) => ({
t: (key: string, options?: Record<string, unknown>) => {
const ns = options?.ns ?? defaultNs
// ... uses ns for prefix
},
}),The new implementation at web/test/i18n-mock.ts:39-48 ignores this parameter:
useTranslation: () => ({
t: createTFunction(translations),
// defaultNs is never captured or passed
}),Impact
Any code that calls useTranslation('someNamespace') expecting the returned t function to automatically prefix keys with that namespace will now receive unprefixed keys. This could cause test failures or false positives in tests that rely on namespace-prefixed translation keys.
For example, const { t } = useTranslation('plugin') followed by t('title') should return 'plugin.title' but now returns just 'title'.
Recommendation: Update createUseTranslationMock to accept and pass through the defaultNs parameter:
export function createUseTranslationMock(translations: TranslationMap = {}) {
return {
useTranslation: (defaultNs?: string) => ({
t: createTFunction(translations, defaultNs),
i18n: {
language: 'en',
changeLanguage: vi.fn(),
},
}),
}
}Was this helpful? React with 👍 or 👎 to provide feedback.
Benchmark PR from qodo-benchmark#437