Repository navigation
test(i18n): locale fallback tests use an injected catalog, so new translations can't break them - #25454
Closed
brennanb2025 wants to merge 1 commit into
Closed
test(i18n): locale fallback tests use an injected catalog, so new translations can't break them#25454brennanb2025 wants to merge 1 commit into
brennanb2025 wants to merge 1 commit into
Conversation
… a new translation cannot break them #25418 translated the strings both tests used as their untranslated examples.
7 of 8 tasks
Contributor
Author
|
Superseded by e46fa0d (#10241) on main, which fixed these two tests by asserting the current translations. This PR took a different route: a made-up language pack for the English-fallback test, and an old-wording guard across every shipped language for the discard test. That would survive the next translation change, but it isn't worth a competing PR, so I'm closing this one. If the tests break again on a future translation update, this branch's approach is the one to reuse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Two tests on main went red after #25418 added Spanish, French, Japanese, Korean and Chinese translations. Both tests had borrowed a real string that wasn't translated yet, to stand in for "a string with no translation". Once the translations landed, the stand-in strings were translated and the tests failed, even though the app was working correctly. This PR gives the tests a made-up language catalog that never has those strings, so the next translation can't break them again.
What Changed
What broke on main. Since
e02adc5320b(#25418, "Add translations for terminal shell settings"), six tests fail on main and on every PR that merges main:NativeChatSupportedAgents.test.tsx, "renders the English fallback when the active locale lacks the label key". The test switched to Spanish and first checked that Spanish has no "Supported agents:" label. Add translations for terminal shell settings #25418 added that label to the Spanish catalog, so the check fails.source-control-discard-localization.test.ts, "uses current index-preserving descriptions in es/fr/ja/ko/zh". fix(git): reduce queries and preserve data across execution hosts #24602 changed the wording of the discard confirmation, because discarding now keeps your staged changes. It moved the text to new keys that existed only in English. This test then expected the English text in every language, so no language could still show the old, wrong wording ("revert all changes", "restore the file from HEAD"). Add translations for terminal shell settings #25418 translated the new keys correctly, so the test now gets real translations instead of English.I checked that
e02adc5320bis the cause: its parentd279adc6efdpasses all 12 tests in these two files, ande02adc5320bfails the same 6.The fix. Both tests now use the plugin language pack mechanism to load a small catalog they write themselves, then switch the app to that language. Other tests already load catalogs this way, for example
lazy-locale.test.ts.What users see. Nothing changes. This PR changes tests only.
Why
These tests are about what happens when a string has no translation. A real string that happens to be untranslated today becomes translated as soon as someone translates it, which is exactly what #25418 did. A catalog the test writes itself stays without the string for good. Pointing the tests at another real string that is untranslated today would break again with the next translation PR.
The discard test was really guarding against the old wording coming back, not testing the English fallback. So it keeps that purpose: against the shipped catalogs it now checks "not the old wording" instead of "exactly English".
Linked Issue
None. This fixes main's red tests from #25418.
Visual Proof
N/A. Test-only change; nothing a user sees changes.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
The two files pass on current main (
2b0ce175145) with this change: 13 passed.Without this change, the same two files fail on current main: 6 failed, 6 passed.
Ablations:
Web typecheck,
oxlintandoxfmton both files, and the changed-code quality gate pass.AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Test-only. No product code, catalogs or wire changes, so nothing changes for security, platforms, SSH, mobile, compatibility or performance.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)