Skip to content

Conversation

@tomerqodo
Copy link

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 <[email protected]>
…eateReactI18nextMock` and detail global mock provisions.
@qodo-code-review
Copy link

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.
• This will trigger the repo’s ts/consistent-type-definitions rule and can block CI/linting for
  TypeScript code.
Code

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

+interface TranslationMap extends Record<string, string | string[]> {}
Evidence
PR Compliance ID 16 requires type definitions instead of interface. The new helper file defines
TranslationMap using interface, directly conflicting with the rule.

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
TypeScript lint rule requires `type` aliases instead of `interface`, but the new `web/test/i18n-mock.ts` defines `TranslationMap` as an `interface`.

## Issue Context
Repo enforces `ts/consistent-type-definitions` (per compliance ID 16).

## 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 not mocked 🐞 Bug ✓ Correctness
Description
• The new global react-i18next mock delegates to createReactI18nextMock(), whose t()
  implementation never handles options.returnObjects.
• Components using t(..., { returnObjects: true }) as string[] and then iterating (e.g.
  features.map(...)) will receive a string from the global mock, causing runtime errors in tests.
• This is a regression vs. prior global behavior and can surface as `TypeError: features.map is not
  a function`.
Code

web/vitest.setup.ts[R88-96]

// mock react-i18next
vi.mock('react-i18next', async () => {
  const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next')
+  const { createReactI18nextMock } = await import('./test/i18n-mock')
  return {
    ...actual,
-    useTranslation: (defaultNs?: string) => ({
-      t: (key: string, options?: Record<string, unknown>) => {
-        if (options?.returnObjects)
-          return [`${key}-feature-1`, `${key}-feature-2`]
-        const ns = options?.ns ?? defaultNs
-        if (options || ns) {
-          const { ns: _ns, ...rest } = options ?? {}
-          const prefix = ns ? `${ns}.` : ''
-          const suffix = Object.keys(rest).length > 0 ? `:${JSON.stringify(rest)}` : ''
-          return `${prefix}${key}${suffix}`
-        }
-        return key
-      },
-      i18n: {
-        language: 'en',
-        changeLanguage: vi.fn(),
-      },
-    }),
+    ...createReactI18nextMock(),
  }
})
Evidence
web/vitest.setup.ts now uses createReactI18nextMock() for the global mock. That helper’s
createTFunction() always returns a string fallback and does not special-case returnObjects.
Meanwhile, production code exists that calls t(..., { returnObjects: true }) and then calls
.map() on the result, which will crash if t() returns a string.

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 global i18n mock’s `t()` implementation does not handle `options.returnObjects`. Components that expect arrays from `t(..., { returnObjects: true })` can crash in tests when the global mock returns a string.

### Issue Context
Previously, the global mock explicitly handled `returnObjects`. After refactor, `web/vitest.setup.ts` uses `createReactI18nextMock()` which currently always returns a string fallback.

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

ⓘ 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

import * as React from 'react'
import { vi } from 'vitest'

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

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.
• This will trigger the repo’s ts/consistent-type-definitions rule and can block CI/linting for
  TypeScript code.
Agent prompt
## Issue description
TypeScript lint rule requires `type` aliases instead of `interface`, but the new `web/test/i18n-mock.ts` defines `TranslationMap` as an `interface`.

## Issue Context
Repo enforces `ts/consistent-type-definitions` (per compliance ID 16).

## 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 on lines 88 to 96
// mock react-i18next
vi.mock('react-i18next', async () => {
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next')
const { createReactI18nextMock } = await import('./test/i18n-mock')
return {
...actual,
useTranslation: (defaultNs?: string) => ({
t: (key: string, options?: Record<string, unknown>) => {
if (options?.returnObjects)
return [`${key}-feature-1`, `${key}-feature-2`]
const ns = options?.ns ?? defaultNs
if (options || ns) {
const { ns: _ns, ...rest } = options ?? {}
const prefix = ns ? `${ns}.` : ''
const suffix = Object.keys(rest).length > 0 ? `:${JSON.stringify(rest)}` : ''
return `${prefix}${key}${suffix}`
}
return key
},
i18n: {
language: 'en',
changeLanguage: vi.fn(),
},
}),
...createReactI18nextMock(),
}
})

Choose a reason for hiding this comment

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

Action required

2. Returnobjects not mocked 🐞 Bug ✓ Correctness

• The new global react-i18next mock delegates to createReactI18nextMock(), whose t()
  implementation never handles options.returnObjects.
• Components using t(..., { returnObjects: true }) as string[] and then iterating (e.g.
  features.map(...)) will receive a string from the global mock, causing runtime errors in tests.
• This is a regression vs. prior global behavior and can surface as `TypeError: features.map is not
  a function`.
Agent prompt
### Issue description
The new global i18n mock’s `t()` implementation does not handle `options.returnObjects`. Components that expect arrays from `t(..., { returnObjects: true })` can crash in tests when the global mock returns a string.

### Issue Context
Previously, the global mock explicitly handled `returnObjects`. After refactor, `web/vitest.setup.ts` uses `createReactI18nextMock()` which currently always returns a string fallback.

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

ⓘ 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.

4 participants