Repository navigation
test: pin the locale when asserting grouped numbers - #166
Merged
Merged
Conversation
TokenFormatter.grouped builds a NumberFormatter with no locale, so it follows the system region setting — the macOS convention, and unchanged here. The assertion, however, hardcoded comma grouping, so the suite went red for any contributor whose region groups differently (`253 412 890`, reported while reviewing #160). CI runs en_US and my machine is ko_KR, both comma, so neither ever sees it. grouped now takes an explicit locale defaulting to .current: production behaviour is identical, and the test pins en_US instead of inheriting the runner's region. The added es_ES case fails if the argument is ever ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7 of 10 tasks
donky-ey
added a commit
to donky-ey/PokeDexBar
that referenced
this pull request
Oct 16, 2026
…c and plan (SP4 batch A) (#20) * docs: add SP4 upstream port triage spec Classify all 191 non-merge commits in main..upstream/main (v2.5.6, 77f789d): 56 already in the fork, 87 not applicable, 26 port candidates grouped into batches A-H, 22 needing a user decision (new providers, small UX features, save backups). Also checks open upstream issues chattymin#337, chattymin#443 and chattymin#439 against the fork's code; all three apply. * docs: record the confirmed SP4 scope in the spec The user confirmed the triage recommendations: new providers, the pet size ceiling, variable menu-bar width, the Low Power GIF policy and save backups stay out; the right-click menu (chattymin#373) joins batch E; chattymin#337 and chattymin#443 are in, with chattymin#443 skipping claude-code mirror rows unconditionally. chattymin#216 is deferred. * docs: add the SP4 upstream port plan Eight batches (A-H), one PR each, executed A -> D -> G -> B -> C -> E -> F -> H. * fix: isolate every app-support store from test runs (upstream chattymin#359) `swift test` uses the same storage name as the installed app, so any code that reached a store's default path read or wrote the user's real Application Support folder. Upstream chattymin#359 measured usage-cache.json being rewritten by one suite run. A sweep found the same class in more places: the sprite cache (a test wrote fake GIFs into it), and the save, base-index.json and weather.json, which were safe only because every test happened to inject a path. Instead of gating each store, isolate at the root: for non-bundled runs AppEnv.supportDirectory() returns a per-process temporary folder, and every Application Support path now goes through it. The session-key store already did this on its own and now shares the root. AppEnv.userSupportDirectory names the real location for comparisons without creating it. Guards: AppStorageIsolationTests pins the root branch, each store's default path, and a source scan that forbids composing Application Support paths outside AppEnv. All three fail when the isolation branch is reverted. The upstream regression test is ported as testDefaultPathCacheNeverRewritesTheUserFile. The screenshot generator clones the installed app's sprite cache read-only into the isolated cache so its renders still have sprites. CLAUDE.md: the test-storage rule now covers all app storage, not only credentials. * test: pin the locale when asserting grouped numbers (upstream chattymin#166) TokenFormatter.grouped follows the system region, as macOS does, but the test hardcoded comma grouping, so it failed on machines whose region groups digits differently. grouped now takes a locale defaulting to .current (production unchanged); the test pins en_US and adds an es_ES case that fails if the argument is ignored. * test: guard UI sources against Hangul string literals (upstream chattymin#222) Interpolation tests only see strings that already go through L, so a literal that never reaches the table is invisible to them. Scan the UI sources (plus the app entry file, which holds UI copy in this fork) and fail on any Hangul string literal; Korean comments stay allowed. Core and Player are left out because their per-language data tables hold Hangul legitimately. Only the test is ported; the Antigravity notice it shipped with does not exist here. * test: check the bundled storage namespace now that test runs are isolated testEveryStoredPathDerivesFromTheNamespace asserted that the support folder ends in the storage name, which held only because swift test used the real folder. Check the bundled path with the pure supportDirectoryURL(isBundledApp:) and keep the save-under-the-root check for the running process. * test: stop the seal test from failing on random ciphertext testPayloadIsNotReadableInTheFile searched the envelope for "42". The seal uses a fresh AES-GCM nonce each time, so those two characters show up in the base64 by chance in roughly 1.5% of runs, which failed the gate intermittently. Assert that the plaintext payload bytes are absent instead. * fix: tighten the storage-isolation guards and correct their description (review) - Put @discardableResult back on writeFile. The new tests had been inserted between the attribute and the helper, which added 41 unused-result warnings. - The source scan now also forbids AppEnv.userSupportDirectory outside AppEnv.swift. A future store could otherwise reach the real folder through the new accessor and pass every guard. - Comment stripping in the scan walks characters (shared SourceScan, also used by LocalizedUILiteralTests), so a "//" inside a string no longer hides the rest of the line. - The isolated folder is named with a UUID instead of the PID. PIDs are reused, so a later run could inherit an earlier run's cache. - Correct the claim in 6b52c9e and CLAUDE.md that all three isolation tests fail when the root branch is reverted. Mutation checks show: reverting the root fails the root-branch and per-store tests (plus the session-key test); the source scan does not react to that, and instead fails when a store composes the path itself (the old LocalUsageCache.defaultFileURL, restored) or uses userSupportDirectory. --------- Co-authored-by: 이동기 <ldg0831@midasin.com>
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.
Summary
TokenFormatter.groupedbuilds aNumberFormatterwithout setting a locale, so grouping follows thesystem region setting. That is the macOS convention and this PR does not change it — the popover
and the floating-pet tooltip keep formatting exactly as before.
The problem was the assertion.
testFormatterEdgeshardcoded"253,412,890", so the suite turned redfor any contributor whose region groups differently —
253 412 890in fr/ru,253.412.890in es/de.It surfaced while reviewing #160, where the contributor reported that failure and correctly identified
it as unrelated to their change. CI runs on
en_USand my machine isko_KR, both comma-grouping, soneither ever sees it; the red suite looks like the contributor's fault.
grouped(_:locale:)now takes an explicit locale, defaulting to.current. Production call sitesare untouched, so behaviour is identical.
en_USinstead of inheriting the runner's region, and adds anes_EScase so theargument cannot be silently ignored.
Verification
./scripts/test-gate.sh— 625 tests, 0 failures, logic-core line coverage 89.70%.f.locale = localemakes the new
es_ESassertion fail ("253,412,890"vs"253.412.890"); restoring it passes.groupedwas the only locale-sensitive assertion.cost,costCompact,percentandcompactall useString(format:), which is locale-independent,and date formatting in
Sourcesalready pinsen_US_POSIX.Type of change
UI changes
None — grouping still follows the system region.
Checklist
swift buildandswift testpass locally