Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
706726a3d4 | ||
|
|
e1f569cc3e |
@@ -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.
|
||||
|
||||
@@ -189,16 +189,33 @@ public enum OneDriveLocator {
|
||||
fileManager: FileManager = .default
|
||||
) -> Bool {
|
||||
let probeURL = folder.appendingPathComponent(".redline-probe-\(UUID().uuidString)")
|
||||
// Belt-and-suspenders cleanup, unconditional: AtomicFile.write renames the temp
|
||||
// file onto probeURL and THEN fsyncs the containing directory — if that last
|
||||
// fsync throws, the probe file already exists on disk but the catch below
|
||||
// returns false before ever reaching the explicit removeItem call. And if the
|
||||
// explicit removeItem itself throws, this is the only retry it gets. Either
|
||||
// way, never leave the probe file behind just because we're about to return.
|
||||
defer {
|
||||
if fileManager.fileExists(atPath: probeURL.path) {
|
||||
try? fileManager.removeItem(at: probeURL)
|
||||
}
|
||||
}
|
||||
|
||||
do {
|
||||
try AtomicFile.write(Data(), to: probeURL)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
|
||||
do {
|
||||
try fileManager.removeItem(at: probeURL)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
|
||||
// Only true when the explicit removal above actually succeeded AND the file is
|
||||
// confirmed gone — never trust a removeItem call that returned without throwing
|
||||
// as proof of anything on a File Provider domain.
|
||||
return !fileManager.fileExists(atPath: probeURL.path)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -160,6 +160,49 @@ func probeWritableFalseForAPlainFilePath() throws {
|
||||
#expect(!OneDriveLocator.probeWritable(at: filePath))
|
||||
}
|
||||
|
||||
/// The write itself succeeds (a real probe file lands on disk via AtomicFile.write,
|
||||
/// which never touches this injected FileManager — it uses raw POSIX calls), but the
|
||||
/// FIRST call to `removeItem(at:)` throws, simulating a transient File Provider
|
||||
/// removal failure. probeWritable's own defer-based cleanup must retry and succeed
|
||||
/// (the second call through this same override falls through to `super`), so no
|
||||
/// probe file is left behind even though the function correctly still reports false
|
||||
/// (the removal it explicitly attempted did fail).
|
||||
private final class ThrowOnceOnRemoveFileManager: FileManager, @unchecked Sendable {
|
||||
private let lock = NSLock()
|
||||
private var hasThrown = false
|
||||
|
||||
override func removeItem(at URL: URL) throws {
|
||||
lock.lock()
|
||||
let shouldThrow = !hasThrown
|
||||
hasThrown = true
|
||||
lock.unlock()
|
||||
if shouldThrow {
|
||||
throw NSError(domain: "ShotdeckCoreTests.ThrowOnceOnRemove", code: 1)
|
||||
}
|
||||
try super.removeItem(at: URL)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
func probeWritableFalseAndLeavesNoProbeFileWhenRemoveItemThrowsOnce() throws {
|
||||
// Directory fsync failure (the OTHER way probeWritable's cleanup can be needed) has
|
||||
// no injectable seam: AtomicFile.write's directory fsync is a raw Darwin fsync(2)
|
||||
// call on an already-open file descriptor, not parameterized by any FileManager or
|
||||
// other dependency this test can substitute, and there is no portable way to make
|
||||
// fsync(2) itself fail via chmod or other standard test techniques (fsync failures
|
||||
// are OS/filesystem/hardware-level events). Covering the removeItem-throws path
|
||||
// (below) is what this test does; the fsync-throws path is covered by code
|
||||
// inspection only — the same `defer` block guards both.
|
||||
let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-remove-throws")
|
||||
defer { try? FileManager.default.removeItem(at: dir) }
|
||||
let injectedFileManager = ThrowOnceOnRemoveFileManager()
|
||||
|
||||
#expect(!OneDriveLocator.probeWritable(at: dir, fileManager: injectedFileManager))
|
||||
|
||||
let leftovers = try FileManager.default.contentsOfDirectory(atPath: dir.path)
|
||||
#expect(leftovers.isEmpty)
|
||||
}
|
||||
|
||||
struct TransportDefaultsSuite {
|
||||
let name: String
|
||||
let defaults: UserDefaults
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user