Skip to content

test: unify i18next mocks into centralized helpers#20

Open
tomerqodo wants to merge 5 commits intoqodo_action_req_base_test_unify_i18next_mocks_into_centralized_helpers_pr7from
qodo_action_req_head_test_unify_i18next_mocks_into_centralized_helpers_pr7
Open

test: unify i18next mocks into centralized helpers#20
tomerqodo wants to merge 5 commits intoqodo_action_req_base_test_unify_i18next_mocks_into_centralized_helpers_pr7from
qodo_action_req_head_test_unify_i18next_mocks_into_centralized_helpers_pr7

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#7

hyoban and others added 5 commits January 25, 2026 12:05
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.
@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. TranslationMap declared as interface 📘 Rule violation ✓ Correctness
Description
web/test/i18n-mock.ts introduces interface TranslationMap, which violates the requirement to
  use type aliases instead of interface declarations.
• This reduces consistency across the TypeScript codebase and can conflict with the repository’s
  preferred typing style.
• Replace the interface with an equivalent type alias to meet the rule.
Code

web/test/i18n-mock.ts[4]

+interface TranslationMap extends Record<string, string | string[]> {}
Evidence
PR Compliance ID 16 requires using type definitions instead of interface. The new helper file
explicitly defines TranslationMap as an interface.

AGENTS.md
web/test/i18n-mock.ts[4-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new TypeScript `interface` (`TranslationMap`) was introduced, but the codebase requires `type` aliases instead of `interface` declarations.

## Issue Context
This is in the new shared i18n mock helper.

## 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


2. returnObjects broken in mock 🐞 Bug ✓ Correctness
Description
• The new shared i18n mock’s t() always returns a string and ignores the i18next `returnObjects:
  true` option.
• Components using t(..., { returnObjects: true }) as string[] can crash at runtime (e.g., calling
  .map on a string) or produce incorrect assertions.
• This is a regression from the previous global mock which explicitly returned an array for
  returnObjects.
Code

web/test/i18n-mock.ts[R10-28]

+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}`
+  }
Evidence
The global Vitest setup now uses createReactI18nextMock(), whose createTFunction implementation
never branches on returnObjects and always returns a string. Meanwhile, the billing plan feature
list calls t(...,{returnObjects:true}) and then maps over the result as an array, which will fail if
the global mock is used (no local override).

web/vitest.setup.ts[88-96]
web/test/i18n-mock.ts[10-28]
web/app/components/billing/pricing/plans/self-hosted-plan-item/list/index.tsx[10-33]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new shared react-i18next mock (`web/test/i18n-mock.ts`) ignores the `returnObjects: true` option and always returns a string. Components that expect arrays (and do `as string[]` + `.map`) will crash or behave incorrectly when relying on the global mock.

## Issue Context
The global Vitest setup now uses `createReactI18nextMock()` for `react-i18next`.

## Fix Focus Areas
- web/test/i18n-mock.ts[10-28]

## Implementation notes
- In `createTFunction`, add a guard:
 - If `options?.returnObjects` is true:
   - If a custom translation exists for `fullKey`/`key` and it is an array, return it.
   - Otherwise return a deterministic `string[]` (e.g., [`${fullKey}-feature-1`, `${fullKey}-feature-2`] or []).
 - Ensure `returnObjects` does not end up serialized into the string suffix path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

Comment thread web/test/i18n-mock.ts
import * as React from 'react'
import { vi } from 'vitest'

interface TranslationMap extends Record<string, string | string[]> {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. translationmap declared as interface 📘 Rule violation ✓ Correctness

web/test/i18n-mock.ts introduces interface TranslationMap, which violates the requirement to
  use type aliases instead of interface declarations.
• This reduces consistency across the TypeScript codebase and can conflict with the repository’s
  preferred typing style.
• Replace the interface with an equivalent type alias to meet the rule.
Agent prompt
## Issue description
A new TypeScript `interface` (`TranslationMap`) was introduced, but the codebase requires `type` aliases instead of `interface` declarations.

## Issue Context
This is in the new shared i18n mock helper.

## 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

Comment thread web/test/i18n-mock.ts
Comment on lines +10 to +28
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}`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Returnobjects broken in mock 🐞 Bug ✓ Correctness

• The new shared i18n mock’s t() always returns a string and ignores the i18next `returnObjects:
  true` option.
• Components using t(..., { returnObjects: true }) as string[] can crash at runtime (e.g., calling
  .map on a string) or produce incorrect assertions.
• This is a regression from the previous global mock which explicitly returned an array for
  returnObjects.
Agent prompt
## Issue description
The new shared react-i18next mock (`web/test/i18n-mock.ts`) ignores the `returnObjects: true` option and always returns a string. Components that expect arrays (and do `as string[]` + `.map`) will crash or behave incorrectly when relying on the global mock.

## Issue Context
The global Vitest setup now uses `createReactI18nextMock()` for `react-i18next`.

## Fix Focus Areas
- web/test/i18n-mock.ts[10-28]

## Implementation notes
- In `createTFunction`, add a guard:
  - If `options?.returnObjects` is true:
    - If a custom translation exists for `fullKey`/`key` and it is an array, return it.
    - Otherwise return a deterministic `string[]` (e.g., [`${fullKey}-feature-1`, `${fullKey}-feature-2`] or []).
  - Ensure `returnObjects` does not end up serialized into the string suffix path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants