Host every image suite in one snapshot bundle, and move two suites to their modules#143
Merged
Conversation
Groundwork for hosting every module's image suite in one bundle, so the Periscope and SwiftDataInspector suites can move out of WhereUI. No suites move yet and no reference image changes; this is the rename plus the reason it stays a single bundle. The single-bundle shape is load-bearing, not tidiness. Each .xctest statically embeds what it links, so a second snapshot bundle would carry a second copy of SnapshotKitTesting -- whose "process-global" capture state is module-global, i.e. per copy. Verified with nm against the two bundles Xcode already builds side by side into StuffTestHost.app/PlugIns: each defines its own private _swizzleDepth, and both install the UIView.snapshotKitOverriddenSafeAreaInsets category. Co-loading two such copies into one host process would count separate swizzle depths against the one shared method exchange -- flipping parity so captures silently render with the simulator's real safe-area insets -- and neither SnapshotCaptureLock would see the other's captures. They never co-load today only because they sit in different schemes. Per-module ownership does not need per-module bundles: swift-snapshot-testing derives the __Snapshots__ directory from the calling file's #filePath, which assertSnapshots threads through, so a suite records its references next to itself wherever it lives. Ownership follows the source directory, not the target. Renames the target, the scheme (keeping its SNAPSHOT_EXPECTED_* / TZ pins), the CI job's tuist invocation, the profile script's SNAPSHOT_SCHEME, and the doc/comment references. Records the one-bundle rule in the root AGENTS.md "Targets" section and in SnapshotKitTesting's AGENTS.md, next to the state it protects. Verified: tuist build StuffSnapshotTests succeeds; swiftformat --lint clean.
The suite captured a PeriscopeTools view from Where/WhereUI/SnapshotTests, where its own doc comment conceded it lived only because that is where the app reaches the viewer. It now lives in Shared/Periscope/PeriscopeTools/SnapshotTests, and its reference images with it, so the module owns the pixels it is responsible for. The references moved unmodified -- git records both as R100 and the suite passes against them untouched. That is worth recording, because it disproves the old comment: the suite wrapped the viewer in whereBroadwayRoot() "so it renders with app styling", but PeriscopeViewer seeds its own periscopeBroadwayRoot() internally (PeriscopeViewer.swift:89), which re-seeds from the same system traits. The Where root was contributing nothing to the capture, so dropping it changed no pixels. The fixture was already self-contained apart from PreviewSupport.referenceNow, which is WhereUI's; the moved suite pins the same instant locally so nothing here reaches back into the app. Adds the module's SnapshotTests directory to the StuffSnapshotTests sources -- it joins the one snapshot bundle rather than getting its own, and records references beside itself via #filePath. Closes the PeriscopeViewer half of the root TODOs.md broken-snapshots item. Documents the arrangement in the module's README and AGENTS. Verified: StuffSnapshotTests/PeriscopeViewerSnapshotTests green against the moved references with no recording; swiftformat --lint clean.
The suite lived in Where/WhereUI/SnapshotTests and reached through PreviewSupport.previewServices(), a seeded manual day, and a real WhereSession just to obtain a configuration -- so an app-agnostic module was snapshotted through Where's store, and its reference images showed Where's schema (SDManualDay and friends). It now lives in Shared/SwiftDataInspector/SnapshotTests over a local in-memory fixture schema, matching how the module's unit tests already work and how a consumer other than Where would actually see it. Unlike the PeriscopeViewer move, this one genuinely re-records: the content changed. The references now show three fixture entities with their seeded row counts (SnapshotAlbum 3, SnapshotPlaylist 1, SnapshotTrack 12), which is both deterministic and legible as a fixture. The capture also drops whereBroadwayRoot() -- the module has no design system of its own, so the image should show what a bare consumer gets, not Where's styling. Fixture models are named Snapshot* rather than the Tests/ directory's Test*: they are a separate bundle, and duplicate @model class names would collide in the runtime if both ever loaded into one host process. Also corrects the now-stale pointer in WhereUI's SwiftDataInspectorWiringTests, which credited the moved snapshot with covering rendering against Where's store. It no longer does, which makes that wiring test the only thing tying the inspector to the real schema -- worth saying plainly. Closes the SwiftDataInspector half of the root TODOs.md broken-snapshots item. Verified: both moved suites green in a comparison run (no recording, no reference modifications); swiftformat --lint clean.
Moves the root broken-snapshots item to Completed, recording that the prerequisite it assumed -- a snapshot bundle per module, each with its own scheme and CI job -- was deliberately not built, and why: per-module bundles duplicate SnapshotKitTesting's module-global capture state and would corrupt each other in a shared host process, and they were never needed for ownership because references follow the calling file's #filePath. A future reader following that item would otherwise build exactly the thing this change established is unsafe. Rewrites the two dependent items, whose stated blocker is gone. Periscope's hosting-test conversion no longer needs a bundle built -- the directory and a worked example exist -- so it now names the actual remaining work: per-view fixtures, a SnapshotKit dependency for SnapshotProviding conformances (the module has no previews at all today), and OpenSpansView's ticking TimelineView as the one real determinism problem. SwiftDataInspector's note drops from needs-design to quick-win for the same reason and names the two uncovered surfaces. Verified: full StuffSnapshotTests suite green (27 suites, no recording, zero reference modifications), including SnapshotCaptureFlagProbeTests -- the guard that the pipeline's capture flag still resolves across the WhereUI boundary after the bundle rename. swiftformat --lint clean.
kyleve
enabled auto-merge
July 27, 2026 03:02
kyleve
disabled auto-merge
July 27, 2026 03:02
kyleve
enabled auto-merge (squash)
July 27, 2026 03:03
kyleve
added a commit
that referenced
this pull request
Jul 27, 2026
Brings in #143, which consolidates every image suite into one StuffSnapshotTests bundle (renamed from WhereUISnapshotTests) and moves the PeriscopeViewer and SwiftDataInspector suites to the modules that own those views. The rename doesn't reach this branch's two new suites: the consolidated bundle still lists Where/WhereUI/SnapshotTests/** among its sources, so AboutSettingsViewSnapshotTests and LicenseViewSnapshotTests are picked up unchanged. Main also touched the snapshot rendering helpers (MotionIsStatic, SnapshotCaptureFlagProbe), which could have shifted the 28 references recorded here, so the whole bundle was re-run rather than assumed: 30 suites green with no re-recording needed. One conflict, in the root TODOs.md, structural rather than substantive: main moved its broken-snapshots item into "Completed issues" and bucketed that section by original priority, while this branch had appended the closed CreditKit item there flat. Kept both, with the CreditKit item under a P1s heading to match main's new bucketing. Verified on the merged tree: swiftformat --lint, xcstrings --lint, ./attribution --check, bumper lint, Stuff-iOS-Tests, and the renamed StuffSnapshotTests scheme all green.
kyleve
added a commit
that referenced
this pull request
Jul 27, 2026
Gives each module its own image-snapshot bundle — `WhereUISnapshotTests`, `PeriscopeToolsSnapshotTests`, `SwiftDataInspectorSnapshotTests` — all listed in the single `StuffSnapshotTests` scheme, so CI still runs them in one invocation. **Stacked on #145.** Review/merge that first. ## This corrects my mistake in #143 I argued there for a single shared bundle, on the grounds that a second one would be unsafe: several `.xctest` bundles in one scheme load into one `StuffTestHost` process, where each bundle's own copy of `SnapshotKitTesting`'s capture state (the safe-area swizzle's depth counter, `SnapshotCaptureLock`, the `UIView` category) would fight over the single shared `UIView` method exchange. **The premise is false.** I inherited "one scheme means one process" from the root `AGENTS.md` double-linking note instead of measuring it. Probing `ProcessInfo.processIdentifier` from two bundles in one scheme: ``` full unfiltered Stuff-iOS-Tests run: SnapshotKitTestingTests pid=12076 WhereUITests pid=12146 ``` Different PIDs, on both a filtered and a full run. xcodebuild gives each test bundle its own host process, so each copy of that state is genuinely process-wide — exactly as its documentation claims. The hazard is real, but it needs co-loading, which doesn't happen. ## Splitting is better than merely permissible Each module's bundle now links only what that module needs. `PeriscopeToolsSnapshotTests` depends on `PeriscopeTools` and `SwiftDataInspectorSnapshotTests` on `SwiftDataInspector` — neither builds against WhereUI any more. That's the layering wart I explicitly accepted when arguing for the shared bundle, now gone. References were already per-module (swift-snapshot-testing derives `__Snapshots__` from the caller's `#filePath`), so **no image moved and none were re-recorded**. ## Docs The "never add a second image-snapshot bundle" rule is replaced with the measurement and the assumption it rests on: if a toolchain ever starts sharing one host process across bundles, re-measure before adding another. Updated across the root `AGENTS.md`, `SnapshotKitTesting`, and the three modules' `AGENTS.md`/`README.md`. The #143 completed TODO keeps its wrong reasoning with a correction appended rather than being edited away. Two load-bearing claims in these docs have now turned out to be inherited rather than verified — the "WhereUI is a dynamic framework" one in #145, and this — and that pattern seems worth leaving visible. ## Note on #145 Its filed spike item existed to make a second snapshot bundle safe. That motivation is now moot, so this PR reframes the entry: the findings stay recorded (they're still accurate about *how* dynamic linking behaves), but it's explicit that the reason for wanting it evaporated. ## Verification - All three bundles green from the one scheme, with three separate `All tests` boundaries in the output — the process separation showing up directly. - **Zero reference images modified.** - `Stuff-iOS-Tests` green. - `./swiftformat --lint` clean.
kyleve
added a commit
that referenced
this pull request
Jul 27, 2026
`StuffTestHost` force-linked `WhereCore` so that `Stuff_WhereCore.bundle` and — transitively — the GeoJSON `Stuff_RegionKit.bundle` were embedded in the host, on the theory that a hosted test bundle's `Bundle.module` resolves against the host's main bundle. **On Xcode 27 that's no longer necessary.** Each `.xctest` now carries its own copies of the resource bundles for the code it links — `WhereUITests.xctest` ships `Stuff_WhereCore.bundle`, `Stuff_RegionKit.bundle`, `Stuff_WhereUI.bundle`, and `Stuff_LifecycleKitUI.bundle` — so SwiftPM's `Bundle.module` resolves via `Bundle(for: BundleFinder.self)` (the test bundle) and never falls back to `Bundle.main`. So every bundle in the scheme was paying for a host dependency nothing needed, and the full GeoJSON set was being embedded twice. ## On the old comment It cited two canaries as proof the dependency was load-bearing: | Canary | Status | |---|---| | `WhereUITests.StringsTests` | **No longer exists** — went with the String Catalog symbol migration that deleted the hand-maintained `Strings` facades | | `SwiftDataInspectorWiringTests` | Still exists, passes without the dependency | The claim quietly stopped being true rather than being wrong when written. That's why the replacement comment records *what makes it unnecessary* and what to do instead of re-adding a product here, rather than just deleting the note — the next person hitting a missing-resource failure should give the bundle that needs the resource a dependency on the product that owns it, not re-embed it in the host for everyone. ## Verification - `Stuff-iOS-Tests` green. - `WhereUISnapshotTests` green — the resource-sensitive one (`Assets.car`, GeoJSON, string catalogs), with **zero reference images modified**. This is the scheme that would fail loudly if a resource lookup silently degraded. - Built host confirmed to contain no WhereCore symbols and no resource bundles at all (`StuffTestHost.app` is now just the executable, dylib, and `PlugIns/`). - `./swiftformat --lint` clean. ## Notes Closes the WhereCore half of the `StuffTestHost` item in `TODOs.md`; the scene-configuration-name duplication half stays open and is re-filed as its own `quick-win`. Independent of #142 / #143 and mergeable in any order, though all three touch `TODOs.md`, so whichever lands last may need a trivial conflict resolution.
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
1 participant
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.
Resolves the two relocation items filed in #142: the
PeriscopeViewerandSwiftDataInspectorsnapshot suites now live in the modules that own those views, and every module's image suite runs from one scheme and one CI job.Stacked on #142 (the TODO-filing PR) — it edits the same
TODOs.mdfiles. Review/merge that first.The design changed during the work, deliberately
The filed TODO assumed each module would get its own
*SnapshotTestsbundle, scheme, and CI job. That turned out to be actively unsafe, so it wasn't built.Each
.xcteststatically embeds what it links, so a second snapshot bundle carries a second copy ofSnapshotKitTesting— whose "process-global" capture state is module-global, and therefore per copy. Verified withnmagainst the two bundles Xcode already builds side by side intoStuffTestHost.app/PlugIns/:Two such copies co-loaded into one host process would count separate swizzle depths against the one shared
UIViewmethod exchange — flipping parity so captures silently render with the simulator's real safe-area insets — and neitherSnapshotCaptureLockcould see the other's captures. They never co-load today only because they sit in different schemes.And per-module bundles were never needed for the actual goal. swift-snapshot-testing derives the
__Snapshots__directory from the calling file's#filePath, whichassertSnapshotsthreads through, so a suite records its references next to itself wherever it lives. Ownership follows the source directory, not the target.So the aggregation happens at the bundle level:
WhereUISnapshotTests→StuffSnapshotTests, onesourcesdirectory per module, one scheme, one CI job. This keeps one copy of the pipeline, which is what makes those types' "process wide" docs true rather than aspirational. The rule is now recorded in the rootAGENTS.mdand next to the state it protects inSnapshotKitTesting/AGENTS.md.The two moves
PeriscopeViewer— zero pixels changed. Git records both references asR100and the suite passes against them untouched. That disproves the old suite's own comment: it wrapped the viewer inwhereBroadwayRoot()"so it renders with app styling", butPeriscopeViewerseeds its ownperiscopeBroadwayRoot()internally (PeriscopeViewer.swift:89), which re-seeds from the same system traits. The Where root was contributing nothing.SwiftDataInspector— genuinely re-recorded. The old suite reached throughPreviewSupport.previewServices(), a seeded manual day, and a realWhereSessionjust to obtain a configuration, so an app-agnostic module was snapshotted through Where's store and its references showed Where's schema. It now uses a local in-memory fixture schema, matching the module's unit tests and what a non-Where consumer would see. The new references show three fixture entities with their seeded row counts (3 / 1 / 12) and no design-system root.Also
SwiftDataInspectorWiringTests, which credited the moved snapshot with covering rendering against Where's store. It no longer does, making that wiring test the only thing tying the inspector to the real schema.SnapshotKitdependency forSnapshotProvidingconformances since the module has no#Previews today, andOpenSpansView's tickingTimelineViewas the one genuine determinism problem). SwiftDataInspector's drops toquick-win.Verification
StuffSnapshotTestssuite green: 27 suites, no recording, zero reference modifications — includingSnapshotCaptureFlagProbeTests, the guard that the capture flag still resolves across the WhereUI boundary after the rename.Stuff-iOS-Testsgreen../swiftformat --lintclean.