Skip to content

chore: new tests for localization#217

Merged
camrun91 merged 5 commits intomainfrom
YPE-1959-react-sdk-set-up-localization-tests
Apr 27, 2026
Merged

chore: new tests for localization#217
camrun91 merged 5 commits intomainfrom
YPE-1959-react-sdk-set-up-localization-tests

Conversation

@camrun91
Copy link
Copy Markdown
Collaborator

@camrun91 camrun91 commented Apr 22, 2026

This PR adds the tests for localization

Greptile Summary

This PR adds two new test files covering the i18n module and the VerseOfTheDay component's localization integration. Both previous review concerns (orphaned event listener, hardcoded locale cleanup) have been addressed in this iteration.

Confidence Score: 5/5

Safe to merge — both prior concerns are resolved and no new issues are present.

All findings are P2 or lower; prior P1 concerns (orphaned listener, fragile afterEach cleanup) have been addressed. The tests are correct and serve their stated purpose.

No files require special attention.

Important Files Changed

Filename Overview
packages/ui/src/i18n/index.test.ts New test verifying English bundle key resolution; handler cleanup (i18n.off) is now present, addressing the prior orphaned-listener concern.
packages/ui/src/components/verse-of-the-day.test.tsx New i18n integration smoke test for VerseOfTheDay; all hooks stubbed correctly, hardcoded-locale afterEach from the prior version has been removed.

Sequence Diagram

sequenceDiagram
    participant T as Test Runner
    participant I as i18n index.test.ts
    participant V as verse-of-the-day.test.tsx
    participant i18n as i18n Instance
    participant VOTD as VerseOfTheDay Component

    T->>I: run i18n instance tests
    I->>i18n: check isInitialized
    i18n-->>I: true (synchronous resources)
    I->>i18n: t('verseOfTheDay')
    i18n-->>I: "Verse of The Day"
    I-->>T: ✓ key resolves correctly

    T->>V: run VerseOfTheDay i18n tests
    V->>V: stubHooks() (mock all platform hooks)
    V->>VOTD: render(<VerseOfTheDay />)
    VOTD->>i18n: useTranslation({ i18n }) → t('verseOfTheDay')
    i18n-->>VOTD: "Verse of The Day"
    VOTD-->>V: DOM with translated label
    V->>V: screen.getByText(en.verseOfTheDay)
    V-->>T: ✓ translated label in document
Loading

Reviews (3): Last reviewed commit: "fix greptile request" | Re-trigger Greptile

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 22, 2026

⚠️ No Changeset found

Latest commit: c9072ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@camrun91 camrun91 requested a review from cameronapak April 22, 2026 21:37
Copy link
Copy Markdown
Collaborator

@cameronapak cameronapak left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@cameronapak
Copy link
Copy Markdown
Collaborator

Hey Cam L., been looking at this PR the past 20 minutes. Thank you for the initiative.

I imagine the heart of these tests are to ensure things are working well with the localization work you did in #210. It seems good as-is

thought (non-blocking): If you want to make this PR even better, it seems these tests could be simplified. In i18n/index.ts, most of it validates that i18next does what it says it will do, but we are more concerned with how it works practically within our codebase. (This will flesh out more as we get translations and do Storybook testing in the future) I have a fun snippet that I ask in Claude after it creates a plan or writes code: "Please step back and think again. How can we make this SIMPLER and DUMBER while still achieving our goals?" This is completely optional, but I'm often a fan of less code (in general) leads to less problems.

@camrun91
Copy link
Copy Markdown
Collaborator Author

thought (non-blocking): If you want to make this PR even better, it seems these tests could be simplified. In i18n/index.ts, most of it validates that i18next does what it says it will do, but we are more concerned with how it works practically within our codebase. (This will flesh out more as we get translations and do Storybook testing in the future) I have a fun snippet that I ask in Claude after it creates a plan or writes code: "Please step back and think again. How can we make this SIMPLER and DUMBER while still achieving our goals?" This is completely optional, but I'm often a fan of less code (in general) leads to less problems.

This is a good idea. I simplified it

@camrun91 camrun91 marked this pull request as ready for review April 23, 2026 20:42
Comment thread packages/ui/src/i18n/index.test.ts
Comment thread packages/ui/src/components/verse-of-the-day.test.tsx Outdated
@camrun91 camrun91 requested a review from cameronapak April 24, 2026 17:46
Copy link
Copy Markdown
Collaborator

@cameronapak cameronapak left a comment

Choose a reason for hiding this comment

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

LGTM!

@camrun91 camrun91 merged commit 50e813f into main Apr 27, 2026
6 checks passed
@camrun91 camrun91 deleted the YPE-1959-react-sdk-set-up-localization-tests branch April 27, 2026 13:28
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.

2 participants