You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(journal): cover production typed catalog resolution - #2768
Four journal catalog regressions on each platform call unused string-only mergers, so they can stay green when the typed resolver used by the journal UI regresses. Remove both mergeCatalog / mergeJournalCatalog copies and run those assertions through the unchanged production resolvedItems / resolveJournalItems paths and real legacy migration.
Add matching checks for saved canonical/display/type/group/order metadata surviving normalized imported aliases and persistence reload, and for hiding/restoring an imported alias without duplicating its saved item. Inject an optional UserDefaults suite into the Apple catalog store (defaulting to standard) so this test class's catalog mutations use isolated suites with teardown cleanup. Update obsolete helper-name comments in both Insights screens. No scoring, schema, persisted representation or UI styling changes.
Type of change
Refactor / cleanup
CI / tooling (production regression coverage)
How it was tested
Linux Swift harness: 11 committed catalog methods passed against the entire production catalog implementation, using Foundation UserDefaults/JSON persistence. Linux scaffolding supplies ObservableObject/Published and the localized-label constructor; this does not validate Combine publication, Apple app compilation or the unrelated journal-entry/Insights methods in the class.
Kotlin harness: 12 committed methods passed against the actual pure production catalog, normalizer and JSON persistence with pinned Kotlin 1.9.24/JDK 17/JUnit 4.13.2 and the real org.json library. This is not a full Android app build.
Private mutations discarding saved metadata in Swift and removing hidden filtering in Kotlin were rejected (nine assertions and three test methods respectively).
Verified unchanged head 83a7e4701ff1454952c29c5d8ce4f44d9530608a: the complete expected six-check roster passed and was confirmed stable twice. Native Android app Kotlin/KSP compilation, APK build and full testFullDebugUnitTest passed; universal macOS Strand and iOS NOOPiOS builds passed.
Native macOS StrandTests ran 2,281 tests, with two skipped and zero failures. All 14 JournalLogicTests methods passed explicitly, including both new metadata/reload and hidden-alias cases. iOS evidence is compile-only; its test step is skipped by design. No warning names the changed source/test files in the inspected native logs.
Tools Python CI passed capture, parity repository acceptance, legacy R-R preservation and core suites; hygiene and i18n also passed. No package source changed, and app/UI helper removals lie outside scanned parity roots, so no derived parity metadata change is required.
CI's merge-current-main steps were skipped; validation covers the actual PR head rather than a hypothetical merge with later main changes.
The shipped resolvers and serialization remain unchanged on both platforms; only unused twins were removed. No BLE changes or hardware testing applies.
Checklist
Native Android full unit suite passes
Both Apple app builds and macOS StrandTests pass
No warnings naming changed source/test files in inspected native logs
Reviewed and verified locally. Green at 83a7e47, 6/6.
mergeCatalog and mergeJournalCatalog had no production callers on either platform. The only non-test references were three doc comments, and this PR updates all three, so the removal is complete.
Three parts stand out.
The defaults: injection on JournalCatalogStore is the right call and probably the most valuable line in the PR. The previous tests ran JournalCatalogStore() against UserDefaults.standard, so a test run wrote into the real app namespace and whatever a developer had saved locally leaked into the assertions. An isolated suite per test, with a teardown that removes the persistent domain, makes the catalog tests deterministic for the first time.
Adding "Did you take magnesium?\u{00a0}" is real new coverage rather than a reshuffle. The comment above that test has named the non-breaking space as the export's doing since #224, and no test input had ever actually contained one: the old pair only exercised the trailing newline and the doubled interior space. The same string lands on the Kotlin side, so the two stay value-for-value.
testSavedMetadataSurvivesNormalizedImportAndReload is the strongest addition. It pins display name, kind, group and sortIndex across a reload and against an imported alias with different casing and spacing at the same time, which is the combination that would silently fork a renamed item into two rows.
One small thing, take it or leave it. Both rewritten comments drop the "Triage fix (a)/(b)" marker that pointed back at why experimentCandidates deliberately avoids the full resolver. The behavioural reason survives in the new wording, but the pointer does not, and that marker is the only thing in the tree naming that triage. Keeping those four words would preserve the trail.
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
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.
What this PR does
Four journal catalog regressions on each platform call unused string-only mergers, so they can stay green when the typed resolver used by the journal UI regresses. Remove both
mergeCatalog/mergeJournalCatalogcopies and run those assertions through the unchanged productionresolvedItems/resolveJournalItemspaths and real legacy migration.Add matching checks for saved canonical/display/type/group/order metadata surviving normalized imported aliases and persistence reload, and for hiding/restoring an imported alias without duplicating its saved item. Inject an optional UserDefaults suite into the Apple catalog store (defaulting to standard) so this test class's catalog mutations use isolated suites with teardown cleanup. Update obsolete helper-name comments in both Insights screens. No scoring, schema, persisted representation or UI styling changes.
Type of change
How it was tested
83a7e4701ff1454952c29c5d8ce4f44d9530608a: the complete expected six-check roster passed and was confirmed stable twice. Native Android app Kotlin/KSP compilation, APK build and fulltestFullDebugUnitTestpassed; universal macOS Strand and iOS NOOPiOS builds passed.Checklist
Related issues
Refs #2708