Skip to content

Conversation

@tomerqodo
Copy link

Benchmark PR from qodo-benchmark#437

hyoban and others added 5 commits January 21, 2026 15:55
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 (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. TranslationMap uses interface 📘 Rule violation ✓ Correctness
Description
web/test/i18n-mock.ts introduces interface TranslationMap, which conflicts with the project
  requirement to use type aliases instead of interface declarations.
• Keeping type declarations consistent avoids style drift and prevents ESLint
  ts/consistent-type-definitions violations across the frontend codebase.
Code

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

+interface TranslationMap extends Record<string, string | string[]> {}
Evidence
The compliance checklist requires using type definitions rather than interface. The new helper
file declares interface TranslationMap, directly violating that requirement.

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
`web/test/i18n-mock.ts` uses an `interface` for `TranslationMap`, but the project standard requires `type` aliases.

## Issue Context
The repo enforces `ts/consistent-type-definitions`-style consistency: prefer `type Name = ...` over `interface Name { ... }`.

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



Remediation recommended

2. Marketplace spec exceeds max-lines 📘 Rule violation ⛯ Reliability
Description
web/app/components/plugins/marketplace/index.spec.tsx is well above the configured SonarJS
  max-lines limit (1000), as evidenced by code existing past line 2700.
• This increases maintenance cost and makes test navigation/refactoring harder; the PR also modifies
  this file, so it’s a good opportunity to split it into smaller spec files.
Code

web/app/components/plugins/marketplace/index.spec.tsx[R2759-2767]

+      // Note: The global mock returns the key with namespace prefix (plugin.)
+      expect(screen.getByText('plugin.category.all')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.models')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.tools')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.datasources')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.triggers')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.agents')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.extensions')).toBeInTheDocument()
+      expect(screen.getByText('plugin.category.bundles')).toBeInTheDocument()
Evidence
The SonarJS rules include a max-lines limit of 1000 lines for TypeScript. The modified test file
contains code at line ~2759+, proving it exceeds the configured limit.

AGENTS.md
web/app/components/plugins/marketplace/index.spec.tsx[2759-2767]

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

## Issue description
`web/app/components/plugins/marketplace/index.spec.tsx` exceeds the configured SonarJS `max-lines` threshold (1000), hurting maintainability.

## Issue Context
The file currently contains multiple large describe blocks (components + hooks) and has grown beyond the configured line limit.

## Fix Focus Areas
- web/app/components/plugins/marketplace/index.spec.tsx[2759-2767]

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


3. i18n docs vs setup 🐞 Bug ⛯ Reliability
Description
• Docs/templates state that the global Vitest i18n mock also provides useMixedTranslation and
  useGetLanguage, and that useGetLanguage returns 'en-US'.
• In reality, web/vitest.setup.ts only globally mocks react-i18next;
  useMixedTranslation/useGetLanguage come from other modules and are not globally mocked.
• This mismatch is likely to cause brittle tests (wrong expectations like 'en-US' vs actual
  'en_US') and confusion about when per-test mocks are required.
Code

web/testing/testing.md[R332-338]

+   The global mock provides:
+
+   - `useTranslation` - returns translation keys with namespace prefix
+   - `Trans` component - renders i18nKey and components
+   - `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
+   - `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`
+
Evidence
The documentation claims additional hooks and a specific locale value are provided by the global
mock, but the actual global setup only mocks react-i18next. Additionally, the real
useGetLanguage derives from getLanguage(locale), which normalizes to underscore locales (e.g.
en_US), contradicting the docs’ 'en-US' claim. The helper Trans mock also doesn’t render
components, despite docs claiming it does.

web/testing/testing.md[330-338]
.claude/skills/frontend-testing/references/mocking.md[52-62]
.claude/skills/frontend-testing/assets/component-test.template.tsx[29-36]
web/vitest.setup.ts[88-96]
web/context/i18n.ts[5-14]
web/i18n-config/language.ts[18-23]
web/test/i18n-mock.ts[54-62]

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

### Issue description
Documentation/templates state that the global Vitest setup provides mocks for `useMixedTranslation` and `useGetLanguage`, and that `useGetLanguage` returns `&#x27;en-US&#x27;`. However, `web/vitest.setup.ts` only mocks `react-i18next`. Also, docs claim `Trans` “renders i18nKey and components”, but the mock `Trans` implementation ignores `components`.

### Issue Context
This mismatch can cause future tests to be written with incorrect assumptions (wrong locale string expectations, or missing mocks for `@/context/i18n` / marketplace hooks).

### Fix Focus Areas
- web/testing/testing.md[330-338]
- .claude/skills/frontend-testing/references/mocking.md[52-66]
- .claude/skills/frontend-testing/assets/component-test.template.tsx[29-39]
- web/vitest.setup.ts[88-96]
- web/test/i18n-mock.ts[54-63]

### Suggested approach
Choose one:
1) **Docs-only fix (lowest risk):** Update docs/templates to say the global mock only covers `react-i18next` and remove/adjust claims about `useMixedTranslation`/`useGetLanguage` and the `&#x27;en-US&#x27;` return value. Clarify that `useGetLanguage` returns the normalized locale (e.g. `en_US`). Clarify `Trans` mock behavior (doesn’t interpolate `components` unless overridden).

2) **Behavior fix:** If you truly want these globally mocked, add explicit `vi.mock` entries in `web/vitest.setup.ts` for `@/app/components/plugins/marketplace/hooks` and `@/context/i18n` and update `createTransMock` to accept/render `components`/`ns` in a predictable test-friendly way.

ⓘ 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 uses interface 📘 Rule violation ✓ Correctness

web/test/i18n-mock.ts introduces interface TranslationMap, which conflicts with the project
  requirement to use type aliases instead of interface declarations.
• Keeping type declarations consistent avoids style drift and prevents ESLint
  ts/consistent-type-definitions violations across the frontend codebase.
Agent prompt
## Issue description
`web/test/i18n-mock.ts` uses an `interface` for `TranslationMap`, but the project standard requires `type` aliases.

## Issue Context
The repo enforces `ts/consistent-type-definitions`-style consistency: prefer `type Name = ...` over `interface Name { ... }`.

## 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 +2759 to +2767
// Note: The global mock returns the key with namespace prefix (plugin.)
expect(screen.getByText('plugin.category.all')).toBeInTheDocument()
expect(screen.getByText('plugin.category.models')).toBeInTheDocument()
expect(screen.getByText('plugin.category.tools')).toBeInTheDocument()
expect(screen.getByText('plugin.category.datasources')).toBeInTheDocument()
expect(screen.getByText('plugin.category.triggers')).toBeInTheDocument()
expect(screen.getByText('plugin.category.agents')).toBeInTheDocument()
expect(screen.getByText('plugin.category.extensions')).toBeInTheDocument()
expect(screen.getByText('plugin.category.bundles')).toBeInTheDocument()

Choose a reason for hiding this comment

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

Remediation recommended

2. Marketplace spec exceeds max-lines 📘 Rule violation ⛯ Reliability

web/app/components/plugins/marketplace/index.spec.tsx is well above the configured SonarJS
  max-lines limit (1000), as evidenced by code existing past line 2700.
• This increases maintenance cost and makes test navigation/refactoring harder; the PR also modifies
  this file, so it’s a good opportunity to split it into smaller spec files.
Agent prompt
## Issue description
`web/app/components/plugins/marketplace/index.spec.tsx` exceeds the configured SonarJS `max-lines` threshold (1000), hurting maintainability.

## Issue Context
The file currently contains multiple large describe blocks (components + hooks) and has grown beyond the configured line limit.

## Fix Focus Areas
- web/app/components/plugins/marketplace/index.spec.tsx[2759-2767]

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

Comment on lines +332 to +338
The global mock provides:

- `useTranslation` - returns translation keys with namespace prefix
- `Trans` component - renders i18nKey and components
- `useMixedTranslation` (from `@/app/components/plugins/marketplace/hooks`)
- `useGetLanguage` (from `@/context/i18n`) - returns `'en-US'`

Choose a reason for hiding this comment

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

Remediation recommended

3. I18n docs vs setup 🐞 Bug ⛯ Reliability

• Docs/templates state that the global Vitest i18n mock also provides useMixedTranslation and
  useGetLanguage, and that useGetLanguage returns 'en-US'.
• In reality, web/vitest.setup.ts only globally mocks react-i18next;
  useMixedTranslation/useGetLanguage come from other modules and are not globally mocked.
• This mismatch is likely to cause brittle tests (wrong expectations like 'en-US' vs actual
  'en_US') and confusion about when per-test mocks are required.
Agent prompt
### Issue description
Documentation/templates state that the global Vitest setup provides mocks for `useMixedTranslation` and `useGetLanguage`, and that `useGetLanguage` returns `'en-US'`. However, `web/vitest.setup.ts` only mocks `react-i18next`. Also, docs claim `Trans` “renders i18nKey and components”, but the mock `Trans` implementation ignores `components`.

### Issue Context
This mismatch can cause future tests to be written with incorrect assumptions (wrong locale string expectations, or missing mocks for `@/context/i18n` / marketplace hooks).

### Fix Focus Areas
- web/testing/testing.md[330-338]
- .claude/skills/frontend-testing/references/mocking.md[52-66]
- .claude/skills/frontend-testing/assets/component-test.template.tsx[29-39]
- web/vitest.setup.ts[88-96]
- web/test/i18n-mock.ts[54-63]

### Suggested approach
Choose one:
1) **Docs-only fix (lowest risk):** Update docs/templates to say the global mock only covers `react-i18next` and remove/adjust claims about `useMixedTranslation`/`useGetLanguage` and the `'en-US'` return value. Clarify that `useGetLanguage` returns the normalized locale (e.g. `en_US`). Clarify `Trans` mock behavior (doesn’t interpolate `components` unless overridden).

2) **Behavior fix:** If you truly want these globally mocked, add explicit `vi.mock` entries in `web/vitest.setup.ts` for `@/app/components/plugins/marketplace/hooks` and `@/context/i18n` and update `createTransMock` to accept/render `components`/`ns` in a predictable test-friendly way.

ⓘ 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