🧪 [testing] add unit tests for i18n module#567
Conversation
🎯 What: Added a missing test file `src/__tests__/i18n.test.ts` for `src/i18n.ts` to ensure proper functioning of translation features. 📊 Coverage: Tested `setLocale`, `getLocale`, `t` (including fallback and string interpolation logic), `addTranslations`, and the singleton export. ✨ Result: Improved test coverage and ensured reliability of translation utility and interpolation functions. Co-authored-by: sunnylqm <615282+sunnylqm@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughAdds a Bun test file for the i18n module that verifies locale management, translation lookups (including cross-locale fallback and missing-key behavior), string interpolation and placeholder rules, dynamic addition of translations, and the default-export singleton methods. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 4/8 reviews remaining, refill in 27 minutes and 19 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/i18n.test.ts (1)
83-89: ⚡ Quick winAssert singleton identity, not only method presence.
The current check proves shape, but not singleton semantics. Add identity assertions between named/default exports (and optionally repeated imports).
♻️ Suggested test strengthening
@@ - const getRealI18n = async () => { + const getRealI18n = async () => { @@ - return { I18n: module.I18n, i18nSingleton: module.default }; + return { I18n: module.I18n, i18nSingleton: module.default, i18nNamed: module.i18n }; }; @@ test('singleton export', async () => { - const { i18nSingleton } = await getRealI18n(); + const first = await getRealI18n(); + const second = await getRealI18n(); - // Verify it works - expect(i18nSingleton.getLocale()).toBeDefined(); - expect(i18nSingleton.t('checking_update')).toBeDefined(); + expect(first.i18nSingleton).toBe(first.i18nNamed); + expect(first.i18nSingleton).toBe(second.i18nSingleton); + expect(first.i18nSingleton.getLocale()).toBeDefined(); + expect(first.i18nSingleton.t('checking_update')).toBeDefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/i18n.test.ts` around lines 83 - 89, The test currently only checks that i18nSingleton has working methods; update it to assert identity semantics by comparing the singleton instance from getRealI18n() to the module's default/named export and to a second retrieval: after const { i18nSingleton } = await getRealI18n(); also obtain the default export (e.g., const { default: i18nDefault } = await getRealI18n() or import the module directly) and add expect(i18nSingleton).toBe(i18nDefault); then call getRealI18n() a second time (or re-import the module) and assert expect(i18nSingleton).toBe(i18nSingleton2) to prove repeated retrievals return the same object; use the symbols getRealI18n and i18nSingleton to locate where to add these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/i18n.test.ts`:
- Around line 83-89: The test currently only checks that i18nSingleton has
working methods; update it to assert identity semantics by comparing the
singleton instance from getRealI18n() to the module's default/named export and
to a second retrieval: after const { i18nSingleton } = await getRealI18n(); also
obtain the default export (e.g., const { default: i18nDefault } = await
getRealI18n() or import the module directly) and add
expect(i18nSingleton).toBe(i18nDefault); then call getRealI18n() a second time
(or re-import the module) and assert expect(i18nSingleton).toBe(i18nSingleton2)
to prove repeated retrievals return the same object; use the symbols getRealI18n
and i18nSingleton to locate where to add these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43167c58-7f0e-42e2-89db-f18f6b19832a
📒 Files selected for processing (1)
src/__tests__/i18n.test.ts
🎯 What: Modified the dynamic import in `src/__tests__/i18n.test.ts` to use a string interpolation variable instead of a raw query parameter string. 📊 Coverage: Ensures the codebase passes `tsc --noEmit` while retaining the ability to bypass bun:test's global module cache to test the genuine implementation. ✨ Result: CI lint/TS validation check now passes properly. Co-authored-by: sunnylqm <615282+sunnylqm@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/i18n.test.ts (1)
84-90: ⚡ Quick winStrengthen singleton validation beyond method existence.
On Line 88 and Line 89,
toBeDefined()only proves shape, not singleton semantics. Please assert same exported instance across imports and shared state visibility.Proposed test tightening
test('singleton export', async () => { - const { i18nSingleton } = await getRealI18n(); - - // Verify it works - expect(i18nSingleton.getLocale()).toBeDefined(); - expect(i18nSingleton.t('checking_update')).toBeDefined(); + const first = await getRealI18n(); + const second = await getRealI18n(); + + // Verify same exported instance + expect(first.i18nSingleton).toBe(second.i18nSingleton); + + // Verify shared state + first.i18nSingleton.setLocale('zh'); + expect(second.i18nSingleton.getLocale()).toBe('zh'); + expect(second.i18nSingleton.t('checking_update')).toBe('正在检查更新...'); + + // Reset for test isolation + first.i18nSingleton.setLocale('en'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/i18n.test.ts` around lines 84 - 90, The test currently only checks methods exist on i18nSingleton; call getRealI18n() twice (e.g., const a = (await getRealI18n()).i18nSingleton; const b = (await getRealI18n()).i18nSingleton) and assert expect(a).toBe(b) to verify true singleton identity, then mutate shared state (use i18nSingleton.setLocale(...) or changeLocale(...) depending on the API) and assert the other reference sees the change via getLocale() and a localized t(...) string to confirm shared state visibility; update assertions to use toBe for identity and explicit expectations on getLocale()/t to validate the mutation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/i18n.test.ts`:
- Around line 84-90: The test currently only checks methods exist on
i18nSingleton; call getRealI18n() twice (e.g., const a = (await
getRealI18n()).i18nSingleton; const b = (await getRealI18n()).i18nSingleton) and
assert expect(a).toBe(b) to verify true singleton identity, then mutate shared
state (use i18nSingleton.setLocale(...) or changeLocale(...) depending on the
API) and assert the other reference sees the change via getLocale() and a
localized t(...) string to confirm shared state visibility; update assertions to
use toBe for identity and explicit expectations on getLocale()/t to validate the
mutation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8da944c6-32e5-4ed6-a9f4-355c4e75dc2d
📒 Files selected for processing (1)
src/__tests__/i18n.test.ts
1 participant
🎯 What: The
src/i18n.tsfile lacked a test file despite containing foundational, pure-logic functions for translations and string interpolation.📊 Coverage: Covered multiple critical paths:
setLocaleandgetLocalet(key)) with bothenandzhzh->en,en->zh, and key fallback when both missing){{key}}){{ space }})addTranslations)✨ Result: Enhanced the unit test suite and fortified safety against unintended regressions in the internationalization utility module. The tests successfully execute alongside the full core suite using
bun test src/.PR created automatically by Jules for task 9075205663058447537 started by @sunnylqm
Summary by CodeRabbit