From 706726a3d4aaa034b4f6a46054b2a330bd4f8167 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Sat, 5 Sep 2026 10:10:14 +0400 Subject: [PATCH] fix(test): LaunchWiringTests was empirically vacuous for the makeLaunchModel path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewer patched main.swift back to FolderSettings.resolvedAppSupportPaths() (the AirDrop-only resolver) and `swift test --filter LaunchWiringTests` STILL PASSED, because the only thing the test asserted — model.watchFolderURL — is computed independently by AppModel.init() via TransportSettings.effectiveFolders(), not from the `paths` makeLaunchModel() built. bootstrap()'s own unconditional updateWatchFolder reconcile then papered over the reverted resolver, so the test only ever proved the bootstrap reconcile, never the launch resolver itself. The "verified this catches the blocker" claim in the previous commit was empirically false. Fix: added ReturnWatcher.currentWatchFolder (public var, actor-isolated — the folder a watcher is CURRENTLY seeded to scan, readable without calling scanNow()/updateWatchFolder first). The test now asserts, BEFORE bootstrap() runs: model.paths.watchFolder/outbox (already internal-visible via @testable import, no production API change needed there) equal the OneDrive folder, AND the watcher's currentWatchFolder equals it too — both of which genuinely depend on what makeLaunchModel() built. Verified properly this time (both outputs below are verbatim from `swift test --filter LaunchWiringTests`, main.swift's makeLaunchModel() temporarily reverted to FolderSettings.resolvedAppSupportPaths() then restored — the revert itself is not part of this commit): FAILURE (reverted resolver): Expectation failed: (model.paths.watchFolder.path -> "/Users/benjaminhippler/Downloads") == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-") Expectation failed: (model.paths.outbox.path -> "/Users/benjaminhippler/Desktop") == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-") Expectation failed: (seededWatchFolder.path -> "/Users/benjaminhippler/Downloads") == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-") Test ... failed after 0.324 seconds with 3 issues. PASS (resolver restored): Test "Real wiring: AppDelegate.makeLaunchModel() + AppModel.bootstrap() detect a marked OneDrive return" passed after 0.295 seconds. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp --- .../ShotdeckCore/Returns/ReturnWatcher.swift | 9 ++++++ Tests/ShotdeckTests/LaunchWiringTests.swift | 32 +++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Sources/ShotdeckCore/Returns/ReturnWatcher.swift b/Sources/ShotdeckCore/Returns/ReturnWatcher.swift index 9cadb4e..36e03cd 100644 --- a/Sources/ShotdeckCore/Returns/ReturnWatcher.swift +++ b/Sources/ShotdeckCore/Returns/ReturnWatcher.swift @@ -17,6 +17,15 @@ public actor ReturnWatcher { /// A document that IS commented is always recorded, regardless of this flag. public var recordUncommented: Bool = true + /// The folder this watcher is CURRENTLY seeded to scan/watch — whatever `init` + /// last set it to, or `updateWatchFolder` since. Exposed so tests can observe the + /// watcher's seeded folder directly (e.g. right after construction, before + /// `start()`/`updateWatchFolder()` ever run) rather than only inferring it + /// indirectly through `scanNow()`'s behavior. + public var currentWatchFolder: URL { + watchFolder + } + /// Watch folder is `paths.watchFolder`, which production constructs from /// `FolderSettings.resolve().watch`. This type never calls FolderSettings; /// `updateWatchFolder` is invoked by the UI layer only. diff --git a/Tests/ShotdeckTests/LaunchWiringTests.swift b/Tests/ShotdeckTests/LaunchWiringTests.swift index 68df70f..e45d498 100644 --- a/Tests/ShotdeckTests/LaunchWiringTests.swift +++ b/Tests/ShotdeckTests/LaunchWiringTests.swift @@ -5,14 +5,26 @@ import Testing import ShotdeckCore @testable import Shotdeck -/// Coverage gap closed (adversarial review, round 3): the Core-level regression tests -/// in ShotdeckCoreTests hand-replicate what `AppDelegate.makeLaunchModel()` and +/// Coverage gap closed (adversarial review, rounds 3 and 4): the Core-level regression +/// tests in ShotdeckCoreTests hand-replicate what `AppDelegate.makeLaunchModel()` and /// `AppModel.bootstrap()` do, rather than calling them — so a future revert of /// `makeLaunchModel()` back to the AirDrop-only resolver, or a dropped /// `updateWatchFolder` call inside `bootstrap()`, would NOT fail `swift test`. This /// test goes through the real, unmodified call sites in the `Shotdeck` executable /// target via `@testable import`, which `ShotdeckCoreTests` cannot reach (it only /// depends on `ShotdeckCore`) — hence this separate `ShotdeckTests` target. +/// +/// Round 4 correction: the first version of this test asserted only +/// `model.watchFolderURL`, which `AppModel.init` computes independently via +/// `TransportSettings.effectiveFolders()` — so it stayed correct (and the test kept +/// passing) even when `makeLaunchModel()` was reverted to the AirDrop-only resolver, +/// because `bootstrap()`'s own unconditional `updateWatchFolder` reconcile papered +/// over the reverted resolver. That made the "verified this catches the blocker" +/// claim in the previous round's commit message empirically false. This version +/// asserts `model.paths`/the watcher's `currentWatchFolder` BEFORE `bootstrap()` runs, +/// which actually depends on what `makeLaunchModel()` built — see this file's git +/// history (or the round-4 commit message) for the verbatim before/after +/// `swift test --filter` output proving it now discriminates correctly. @MainActor @Test("Real wiring: AppDelegate.makeLaunchModel() + AppModel.bootstrap() detect a marked OneDrive return") func realLaunchModelAndBootstrapDetectAMarkedOneDriveReturn() async throws { @@ -73,6 +85,22 @@ func realLaunchModelAndBootstrapDetectAMarkedOneDriveReturn() async throws { // combo already taken / the global hotkey table in an unexpected state. defer { model.hotkeys.unregisterAll() } + // PRE-bootstrap assertions — this is the actual proof of the launch RESOLVER + // (AppDelegate.makeLaunchModel() -> TransportSettings.resolvedAppSupportPaths()), + // independent of bootstrap()'s own reconcile. `model.watchFolderURL` alone does + // NOT prove this: AppModel.init computes it separately via + // TransportSettings.effectiveFolders(), so it would read as correct even if + // makeLaunchModel's `paths` were built by the AirDrop-only resolver — which is + // exactly how the first version of this test was empirically shown to be vacuous + // for the launch-resolver path (see this commit's message). `model.paths` is + // `internal` on AppModel, so @testable import already exposes it without any + // production API change; `currentWatchFolder` is the one new (internal-facing, + // `public` on the actor) seam added to ReturnWatcher for this purpose. + #expect(model.paths.watchFolder.path == oneDriveFolder.path) + #expect(model.paths.outbox.path == oneDriveFolder.path) + let seededWatchFolder = await model.watcher.currentWatchFolder + #expect(seededWatchFolder.path == oneDriveFolder.path) + #expect(model.transport == .oneDrive) #expect(model.watchFolderURL.path == oneDriveFolder.path)