diff --git a/Scripts/generate_starter_splits.swift b/Scripts/generate_starter_splits.swift index 8415690..27198de 100644 --- a/Scripts/generate_starter_splits.swift +++ b/Scripts/generate_starter_splits.swift @@ -105,7 +105,7 @@ let seeds: [Sp] = [ Ex(name: "Hollow Body Hold", sets: 3, reps: 0, load: 2, dur: 30), Ex(name: "Side Plank", sets: 3, reps: 0, load: 2, dur: 30), Ex(name: "Leg Raises", sets: 3, reps: 12, load: 0), - Ex(name: "Bird Dog", sets: 3, reps: 8, load: 0), + Ex(name: "Crunch", sets: 3, reps: 12, load: 0), Ex(name: "Reverse Crunch", sets: 3, reps: 12, load: 0), ]), ] diff --git a/Shared/Model/Documents.swift b/Shared/Model/Documents.swift index 9a72895..240ce2e 100644 --- a/Shared/Model/Documents.swift +++ b/Shared/Model/Documents.swift @@ -118,8 +118,13 @@ extension WorkoutDocument { let anyStarted = statuses.contains { $0 != .notStarted } if allResolved { + // Only stamp the finish time on the *transition* into completed. A workout + // that was already completed (with an `end`) keeps that original end, so + // editing a finished workout — which re-runs this recompute with all logs + // still resolved — doesn't jump its end forward to now. + let wasCompletedWithEnd = status == WorkoutStatus.completed.rawValue && end != nil status = WorkoutStatus.completed.rawValue - end = Date() + if !wasCompletedWithEnd { end = Date() } } else if anyStarted { status = WorkoutStatus.inProgress.rawValue end = nil diff --git a/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift b/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift index 978db92..93e93de 100644 --- a/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift +++ b/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift @@ -212,7 +212,6 @@ final class WatchConnectivityBridge: NSObject { private func applyState(_ splits: [SplitDocument], workouts: [WorkoutDocument]) { Self.log.info("applyState: \(splits.count) splits, \(workouts.count) workouts") - guard !splits.isEmpty || !workouts.isEmpty else { return } var liveSplitIDs = Set() for s in splits { CacheMapper.upsertSplit(s, relativePath: s.relativePath, into: context) diff --git a/Workouts/Resources/StarterSplits/Bodyweight Core.split.json b/Workouts/Resources/StarterSplits/Bodyweight Core.split.json index 760b6ec..c878837 100644 --- a/Workouts/Resources/StarterSplits/Bodyweight Core.split.json +++ b/Workouts/Resources/StarterSplits/Bodyweight Core.split.json @@ -77,9 +77,9 @@ "durationSeconds" : 0, "id" : "01DXF6DT00MJH9N622QTMM7XB3", "loadType" : 0, - "name" : "Bird Dog", + "name" : "Crunch", "order" : 7, - "reps" : 8, + "reps" : 12, "sets" : 3, "weight" : 0 }, diff --git a/Workouts/Sync/SyncEngine.swift b/Workouts/Sync/SyncEngine.swift index fa54d6e..56a079f 100644 --- a/Workouts/Sync/SyncEngine.swift +++ b/Workouts/Sync/SyncEngine.swift @@ -331,9 +331,21 @@ final class SyncEngine { func save(workout doc: WorkoutDocument) async { guard let store else { return } + // The month bucket in a workout's path derives from `start`, so editing the + // start date (or a device in a different time zone) can move the file to a new + // path. Capture the previously-written path before the upsert overwrites it, so + // we can remove the stale file below — otherwise the same id would live at two + // paths and the old copy would re-import on the next reconcile. + let previousPath = CacheMapper.fetchWorkout(id: doc.id, in: context)?.jsonRelativePath do { try await store.write(doc, to: doc.relativePath) CacheMapper.upsertWorkout(doc, relativePath: doc.relativePath, into: context) + // Same id, new path: drop the orphaned file at the old bucket. The id lives + // on at the new path, so this is a plain removal — no tombstone (a tombstone + // would veto the record that just moved). Phone stays the sole writer. + if let previousPath, previousPath != doc.relativePath { + try? await store.remove(at: previousPath) + } saveCacheAndNotify() lastSyncError = nil } catch { @@ -619,8 +631,17 @@ final class SyncEngine { case .skip: continue case .upgrade: + // Re-check the veto against fresh state before overwriting — symmetric + // with `.write`. The batch above was gathered before a settle-window + // race: if the user forked/soft-deleted this seed (clone-on-edit writes + // a stub and removes the live file) after we listed the live paths, + // rewriting the seed bytes here would transiently resurrect the deleted + // seed alongside the user's clone. The stub now present vetoes that. + // (No name-collision guard here: on a legitimate upgrade the seed itself + // is a live split by that name, so a name check would block every upgrade.) + if await tombstones.stubExists(id: seed.id) { continue } // Overwriting an existing seed-path file with canonical bundle bytes is - // always safe (the file can only ever be an older seed revision). + // otherwise always safe (the file can only ever be an older seed revision). if await writeSeedBytes(seed) { didChange = true } case .write: // Re-check the veto and name guards against fresh state — the metadata @@ -827,7 +848,7 @@ final class SyncEngine { /// for the UI to render. Replaces silent `try?` / print-only handling. private func report(_ message: String, _ error: Error? = nil) { let full = error.map { "\(message): \($0.localizedDescription)" } ?? message - print("[Sync] \(full)") + log.error("\(full, privacy: .public)") lastSyncError = full } } diff --git a/Workouts/Views/Settings/SettingsView.swift b/Workouts/Views/Settings/SettingsView.swift index 4a9cc64..6f4fe71 100644 --- a/Workouts/Views/Settings/SettingsView.swift +++ b/Workouts/Views/Settings/SettingsView.swift @@ -132,7 +132,8 @@ struct SettingsView: View { Text("iCloud Sync") } - // MARK: - Developer Section + // MARK: - Developer Section (debug builds only) + #if DEBUG Section { NavigationLink { DuplicateCleanupView() @@ -142,6 +143,7 @@ struct SettingsView: View { } header: { Text("Developer") } + #endif // MARK: - About Section Section { diff --git a/WorkoutsTests/SyncEngineTests.swift b/WorkoutsTests/SyncEngineTests.swift new file mode 100644 index 0000000..26b40fc --- /dev/null +++ b/WorkoutsTests/SyncEngineTests.swift @@ -0,0 +1,47 @@ +import Foundation +import SwiftData +import Testing +@testable import Workouts + +/// Coverage for the parts of `SyncEngine` reachable without a live iCloud container. +/// +/// SCOPE NOTE: `SyncEngine`'s orchestration (`save`/`delete`/`reconcile`/`reconcileSeeds`) +/// resolves its `DocumentFileStore` *inside* `connect()` from the real ubiquity +/// container — there is no seam to inject a fake store, and the task deliberately rules +/// out building a heavyweight `NSMetadataQuery` mock. So the file-touching paths (incl. +/// FIX-1's rebucketing removal and FIX-2's `.upgrade` stub re-check) are not unit-testable +/// here without refactoring `SyncEngine` to accept an injected store — out of scope. +/// Their *pure* decision logic is already locked elsewhere: `SeedReconcilePlannerTests` +/// (the seed skip/upgrade/write table, incl. the veto that FIX-2 re-checks), +/// `DuplicateCleanupPlannerTests`, `WorkoutPathBucketingTests` (FIX-1's path divergence), +/// and `WorkoutStatusMachineTests`. What remains directly testable is the pure +/// in-memory redirect helper below. +struct SyncEngineTests { + + @MainActor + private func makeEngine() throws -> SyncEngine { + let schema = Schema([Split.self, Exercise.self, Workout.self, WorkoutLog.self]) + let config = ModelConfiguration(schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none) + return SyncEngine(container: try ModelContainer(for: schema, configurations: [config])) + } + + /// With no clone-on-edit fork recorded, `currentSplitID` is the identity — a view + /// holding a split id resolves to that same id. This pins the redirect-follow loop's + /// base case and its termination (no redirect → return input, no infinite loop). + @MainActor + @Test func currentSplitIDIsIdentityWithoutRedirects() throws { + let engine = try makeEngine() + #expect(engine.currentSplitID(for: "01SEEDABCDEFGHJKMNPQRSTVWX") == "01SEEDABCDEFGHJKMNPQRSTVWX") + #expect(engine.currentSplitID(for: "") == "") + } + + /// A freshly constructed engine is in the `.checking` state until `connect()` runs, + /// and exposes no sync error. + @MainActor + @Test func freshEngineIsCheckingWithNoError() throws { + let engine = try makeEngine() + #expect(engine.iCloudStatus == .checking) + #expect(engine.lastSyncError == nil) + #expect(!engine.isSyncing) + } +} diff --git a/WorkoutsTests/WCPayloadRoundTripTests.swift b/WorkoutsTests/WCPayloadRoundTripTests.swift new file mode 100644 index 0000000..16e4e7b --- /dev/null +++ b/WorkoutsTests/WCPayloadRoundTripTests.swift @@ -0,0 +1,155 @@ +import Foundation +import Testing +import IndieSync +@testable import Workouts + +/// Round-trips every `WCPayload` wire shape — the iPhone↔Watch bridge's only +/// serialization boundary. Splits/workouts ride as JSON `Data` blobs (via +/// `DocumentCoder`, ISO-8601), settings and edit locks as native plist scalars, and +/// the ephemeral live-run frames as native values (so their timers keep sub-second +/// precision). A field dropped or a `nil`-vs-empty confusion here silently desyncs +/// the two devices, so each direction is pinned end to end, including the +/// decode-failure contract (`nil` means "failed to decode", never "empty list"). +struct WCPayloadRoundTripTests { + + private static let ts = Date(timeIntervalSince1970: 1_700_000_000) // whole second: ISO-8601 safe + + private func split(id: String, name: String) -> SplitDocument { + SplitDocument( + schemaVersion: SplitDocument.currentSchemaVersion, id: id, name: name, color: "blue", + systemImage: "dumbbell.fill", order: 0, createdAt: Self.ts, updatedAt: Self.ts, + exercises: [ExerciseDocument(id: "EX-\(id)", name: "Bench Press", order: 0, sets: 4, + reps: 10, weight: 135, loadType: 1, durationSeconds: 0, + machineSettings: [MachineSetting(name: "Seat", value: "4")])], + activityType: nil + ) + } + + private func workout(id: String) -> WorkoutDocument { + WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, id: id, splitID: "S", splitName: "Push", + start: Self.ts, end: Self.ts, status: WorkoutStatus.completed.rawValue, + createdAt: Self.ts, updatedAt: Self.ts, + logs: [WorkoutLogDocument(id: "L-\(id)", exerciseName: "Bench Press", order: 0, sets: 4, + reps: 10, weight: 135, loadType: 1, durationSeconds: 0, + currentStateIndex: 4, status: WorkoutStatus.completed.rawValue, + notes: "solid", date: Self.ts)], + metrics: nil + ) + } + + // MARK: - Phone → Watch state + settings + + @Test func stateRoundTripsSplitsWorkoutsAndSettings() { + let splits = [split(id: "SP1", name: "Upper Body"), split(id: "SP2", name: "Lower Body")] + let workouts = [workout(id: "01WKA"), workout(id: "01WKB")] + let dict = WCPayload.encodeState( + splits: splits, workouts: workouts, restSeconds: 90, doneCountdownSeconds: 5, + weightUnit: "kg", editingWorkoutID: "01WKA", editingSplitID: "SP1" + ) + + #expect(WCPayload.decodeSplits(dict) == splits) + #expect(WCPayload.decodeWorkouts(dict) == workouts) + #expect(WCPayload.decodeRestSeconds(dict) == 90) + #expect(WCPayload.decodeDoneCountdownSeconds(dict) == 5) + #expect(WCPayload.decodeWeightUnit(dict) == "kg") + #expect(WCPayload.decodeEditingWorkoutID(dict) == "01WKA") + #expect(WCPayload.decodeEditingSplitID(dict) == "SP1") + } + + /// Nil edit locks mean "not editing" → the keys are absent, so the decoders return + /// nil (the receiver clears the lock). An empty splits/workouts list is legitimately + /// empty — it still encodes a key and decodes back to `[]`, never nil. + @Test func stateOmitsNilEditLocksAndKeepsEmptyListsDistinct() { + let dict = WCPayload.encodeState( + splits: [], workouts: [], restSeconds: 60, doneCountdownSeconds: 3, + weightUnit: "lb", editingWorkoutID: nil, editingSplitID: nil + ) + #expect(WCPayload.decodeEditingWorkoutID(dict) == nil) + #expect(WCPayload.decodeEditingSplitID(dict) == nil) + // Authoritative empty push: an empty (present) list decodes to [], not nil. + #expect(WCPayload.decodeSplits(dict) == []) + #expect(WCPayload.decodeWorkouts(dict) == []) + } + + /// An absent splits/workouts key decodes to `[]` (nothing to apply), NOT nil — nil is + /// reserved for a genuine decode failure below. + @Test func absentSplitsWorkoutsKeysDecodeToEmpty() { + let empty: [String: Any] = [:] + #expect(WCPayload.decodeSplits(empty) == []) + #expect(WCPayload.decodeWorkouts(empty) == []) + } + + /// A present-but-corrupt payload (a build-mismatch schema the receiver can't decode) + /// yields `nil` — the signal the watch bridge's nil-decode guard uses to skip + /// pruning against a bogus set. This must stay distinct from an empty list. + @Test func corruptStateDecodesToNilNotEmpty() { + let garbage = Data([0xFF, 0xFE, 0x00, 0x01]) + let dict: [String: Any] = [WCPayload.splitsKey: garbage, WCPayload.workoutsKey: garbage] + #expect(WCPayload.decodeSplits(dict) == nil) + #expect(WCPayload.decodeWorkouts(dict) == nil) + } + + // MARK: - Watch → Phone workout update + + @Test func workoutUpdateRoundTrips() { + let w = workout(id: "01WKUPDATE") + let dict = WCPayload.encodeWorkoutUpdate(w) + #expect(dict[WCPayload.typeKey] as? String == WCPayload.workoutUpdateType) + #expect(WCPayload.decodeWorkoutUpdate(dict) == w) + } + + @Test func workoutUpdateWithMissingBlobDecodesToNil() { + let dict: [String: Any] = [WCPayload.typeKey: WCPayload.workoutUpdateType] + #expect(WCPayload.decodeWorkoutUpdate(dict) == nil) + } + + // MARK: - Watch → Phone requestSync + + @Test func requestSyncMessageCarriesType() { + let dict = WCPayload.requestSyncMessage() + #expect(dict[WCPayload.typeKey] as? String == WCPayload.requestSyncType) + } + + // MARK: - Live-run mirror (ephemeral) + + private func liveProgress(phaseEnd: Date?) -> LiveProgress { + LiveProgress( + workoutID: "01WKLIVE", logID: "LOG-LIVE", exerciseName: "Leg Press", phase: .rest, + setIndex: 2, setCount: 4, detail: "30 sec", + phaseStart: Date(timeIntervalSince1970: 1_700_000_123.75), // sub-second: proves native (not ISO-8601) transport + phaseEnd: phaseEnd, version: 7 + ) + } + + @Test func liveProgressRoundTripsWithCountdownEnd() { + let frame = liveProgress(phaseEnd: Date(timeIntervalSince1970: 1_700_000_153.75)) + let dict = WCPayload.encodeLiveProgress(frame) + #expect(dict[WCPayload.typeKey] as? String == WCPayload.liveProgressType) + #expect(WCPayload.decodeLiveProgress(dict) == frame) + } + + /// A rep-based work set counts up and carries no `phaseEnd`; the key must be omitted + /// and decode back to nil, not a zero date. + @Test func liveProgressRoundTripsWithoutEnd() { + let frame = liveProgress(phaseEnd: nil) + let dict = WCPayload.encodeLiveProgress(frame) + #expect(dict[WCPayload.lpEndKey] == nil) + let decoded = WCPayload.decodeLiveProgress(dict) + #expect(decoded == frame) + #expect(decoded?.phaseEnd == nil) + } + + @Test func liveProgressMissingRequiredKeyDecodesToNil() { + var dict = WCPayload.encodeLiveProgress(liveProgress(phaseEnd: nil)) + dict[WCPayload.lpWorkoutIDKey] = nil // drop a required field + #expect(WCPayload.decodeLiveProgress(dict) == nil) + } + + @Test func liveEndedCarriesTypeAndIDs() { + let dict = WCPayload.encodeLiveEnded(workoutID: "01WKLIVE", logID: "LOG-LIVE") + #expect(dict[WCPayload.typeKey] as? String == WCPayload.liveEndedType) + #expect(dict[WCPayload.lpWorkoutIDKey] as? String == "01WKLIVE") + #expect(dict[WCPayload.lpLogIDKey] as? String == "LOG-LIVE") + } +} diff --git a/WorkoutsTests/WorkoutDocumentMapperTests.swift b/WorkoutsTests/WorkoutDocumentMapperTests.swift new file mode 100644 index 0000000..f618232 --- /dev/null +++ b/WorkoutsTests/WorkoutDocumentMapperTests.swift @@ -0,0 +1,195 @@ +import Foundation +import SwiftData +import Testing +import IndieSync +@testable import Workouts + +/// Mirrors `SeedLibraryTests.mapperRoundTripPreservesPristineness` for the *workout* +/// aggregate: a `WorkoutDocument` pushed through `CacheMapper.upsertWorkout` (the +/// document → cache direction the metadata observer / watch bridge use) and rebuilt +/// via `WorkoutDocument(from: entity)` (the cache → document write path) must +/// preserve every field in both directions — the aggregate's own columns, the full +/// embedded log list (order, machine settings, per-log timestamps), and the health +/// metrics. A field that survives one direction but not the other silently forks or +/// drops user data on the very first cache rebuild. +struct WorkoutDocumentMapperTests { + + // Whole-second epochs kept distinct per field so a swapped assignment can't hide. + private static let start = Date(timeIntervalSince1970: 1_700_000_000) + private static let end = Date(timeIntervalSince1970: 1_700_003_600) + private static let created = Date(timeIntervalSince1970: 1_699_999_000) + private static let updated = Date(timeIntervalSince1970: 1_700_004_000) + private static let logDate = Date(timeIntervalSince1970: 1_700_000_500) + private static let startedAt = Date(timeIntervalSince1970: 1_700_000_600) + private static let completedAt = Date(timeIntervalSince1970: 1_700_001_200) + private static let recordedAt = Date(timeIntervalSince1970: 1_700_003_700) + + @MainActor + private func makeContext() throws -> ModelContext { + let schema = Schema([Split.self, Exercise.self, Workout.self, WorkoutLog.self]) + // cloudKitDatabase: .none matches WorkoutsModelContainer (see SeedLibraryTests). + let config = ModelConfiguration(schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none) + return ModelContext(try ModelContainer(for: schema, configurations: [config])) + } + + private func log( + id: String, name: String, order: Int, status: WorkoutStatus, + machineSettings: [MachineSetting]? = nil, startedAt: Date? = nil, completedAt: Date? = nil + ) -> WorkoutLogDocument { + WorkoutLogDocument( + id: id, exerciseName: name, order: order, sets: 4, reps: 10, weight: 135, + loadType: LoadType.weight.rawValue, durationSeconds: 0, currentStateIndex: 2, + status: status.rawValue, notes: "note-\(name)", date: Self.logDate, + machineSettings: machineSettings, startedAt: startedAt, completedAt: completedAt + ) + } + + private func fullMetrics() -> WorkoutMetrics { + WorkoutMetrics( + activeEnergyKcal: 321.5, avgHeartRate: 128, maxHeartRate: 165, minHeartRate: 71, + totalVolume: 5400, hrZoneSeconds: [10, 20, 30, 40, 50], + healthKitWorkoutUUID: "HK-UUID-123", source: .watch, recordedAt: Self.recordedAt + ) + } + + /// A fully-populated workout (three logs exercising the nil / empty / populated + /// machineSettings distinction, distinct per-log timestamps, full metrics) must + /// round-trip byte-for-byte equal in both directions. + @MainActor + @Test func roundTripPreservesEveryField() throws { + let context = try makeContext() + let original = WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, + id: "01WORKOUTROUNDTRIP00000001", splitID: "SPLIT-42", splitName: "Push Day", + start: Self.start, end: Self.end, status: WorkoutStatus.completed.rawValue, + createdAt: Self.created, updatedAt: Self.updated, + logs: [ + // order 0: not a machine exercise (nil), never started. + log(id: "LOG-A", name: "Bench Press", order: 0, status: .notStarted, machineSettings: nil), + // order 1: machine exercise, nothing recorded yet (empty ≠ nil), completed with timestamps. + log(id: "LOG-B", name: "Leg Press", order: 1, status: .completed, machineSettings: [], + startedAt: Self.startedAt, completedAt: Self.completedAt), + // order 2: machine exercise with recorded settings, skipped. + log(id: "LOG-C", name: "Chest Press", order: 2, status: .skipped, + machineSettings: [MachineSetting(name: "Seat Height", value: "4"), + MachineSetting(name: "Back Rest", value: "45°")], + startedAt: Self.startedAt), + ], + metrics: fullMetrics() + ) + + CacheMapper.upsertWorkout(original, relativePath: original.relativePath, into: context) + let entity = try #require(CacheMapper.fetchWorkout(id: original.id, in: context)) + let rebuilt = WorkoutDocument(from: entity) + + #expect(rebuilt == original) + + // Spot-pin the load-bearing fields the whole-document `==` rolls up, so a future + // regression names itself. + #expect(rebuilt.splitID == "SPLIT-42") + #expect(rebuilt.splitName == "Push Day") + #expect(rebuilt.start == Self.start) + #expect(rebuilt.end == Self.end) + #expect(rebuilt.createdAt == Self.created) + #expect(rebuilt.updatedAt == Self.updated) + #expect(rebuilt.status == WorkoutStatus.completed.rawValue) + #expect(rebuilt.logs.count == 3) + #expect(rebuilt.logs[0].machineSettings == nil) // not a machine exercise + #expect(rebuilt.logs[1].machineSettings == []) // machine, nothing recorded + #expect(rebuilt.logs[1].startedAt == Self.startedAt) + #expect(rebuilt.logs[1].completedAt == Self.completedAt) + #expect(rebuilt.logs[2].machineSettings?.count == 2) + #expect(rebuilt.metrics == fullMetrics()) + #expect(rebuilt.metrics?.hrZoneSeconds == [10, 20, 30, 40, 50]) + #expect(rebuilt.metrics?.healthKitWorkoutUUID == "HK-UUID-123") + } + + /// A workout with no metrics yet (the common in-progress / just-started case) must + /// round-trip to `nil` metrics, not a phantom zero-filled `WorkoutMetrics`. + @MainActor + @Test func roundTripPreservesNilMetrics() throws { + let context = try makeContext() + let original = WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, + id: "01WORKOUTNOMETRICS0000001", splitID: nil, splitName: nil, + start: Self.start, end: nil, status: WorkoutStatus.inProgress.rawValue, + createdAt: Self.created, updatedAt: Self.updated, + logs: [log(id: "LOG-X", name: "Plank", order: 0, status: .inProgress, startedAt: Self.startedAt)], + metrics: nil + ) + + CacheMapper.upsertWorkout(original, relativePath: original.relativePath, into: context) + let entity = try #require(CacheMapper.fetchWorkout(id: original.id, in: context)) + let rebuilt = WorkoutDocument(from: entity) + + #expect(rebuilt == original) + #expect(rebuilt.metrics == nil) + #expect(rebuilt.splitID == nil) + #expect(rebuilt.end == nil) + } + + /// The embedded logs are reconciled by id and rebuilt via `logsArray` (sorted by + /// `order`), so a document whose logs arrive out of order comes back canonically + /// ordered — the round-trip normalizes, never scrambles. + @MainActor + @Test func roundTripNormalizesLogOrder() throws { + let context = try makeContext() + let scrambled = WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, + id: "01WORKOUTLOGORDER00000001", splitID: "S", splitName: "S", + start: Self.start, end: Self.end, status: WorkoutStatus.completed.rawValue, + createdAt: Self.created, updatedAt: Self.updated, + logs: [ + log(id: "LOG-2", name: "Third", order: 2, status: .completed), + log(id: "LOG-0", name: "First", order: 0, status: .completed), + log(id: "LOG-1", name: "Second", order: 1, status: .completed), + ], + metrics: nil + ) + + CacheMapper.upsertWorkout(scrambled, relativePath: scrambled.relativePath, into: context) + let entity = try #require(CacheMapper.fetchWorkout(id: scrambled.id, in: context)) + let rebuilt = WorkoutDocument(from: entity) + + #expect(rebuilt.logs.map(\.order) == [0, 1, 2]) + #expect(rebuilt.logs.map(\.id) == ["LOG-0", "LOG-1", "LOG-2"]) + #expect(rebuilt.logs.map(\.exerciseName) == ["First", "Second", "Third"]) + } + + /// Re-upserting a changed document over an existing cache entity reconciles the log + /// set by id: a removed log is dropped, a new one inserted, survivors updated in + /// place — matching the observer's frequent in-workout re-imports. + @MainActor + @Test func reUpsertReconcilesLogsByID() throws { + let context = try makeContext() + let first = WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, + id: "01WORKOUTREUPSERT00000001", splitID: "S", splitName: "S", + start: Self.start, end: nil, status: WorkoutStatus.inProgress.rawValue, + createdAt: Self.created, updatedAt: Self.updated, + logs: [ + log(id: "LOG-keep", name: "Kept", order: 0, status: .completed), + log(id: "LOG-drop", name: "Dropped", order: 1, status: .notStarted), + ], + metrics: nil + ) + CacheMapper.upsertWorkout(first, relativePath: first.relativePath, into: context) + try context.save() + + var second = first + second.logs = [ + log(id: "LOG-keep", name: "Kept Renamed", order: 0, status: .completed), // updated in place + log(id: "LOG-add", name: "Added", order: 1, status: .notStarted), // inserted + ] + CacheMapper.upsertWorkout(second, relativePath: second.relativePath, into: context) + // The observer saves after applying each delta batch; the relationship array only + // reflects a child deletion once saved, so mirror that here. + try context.save() + + let entity = try #require(CacheMapper.fetchWorkout(id: second.id, in: context)) + let rebuilt = WorkoutDocument(from: entity) + #expect(rebuilt.logs.map(\.id) == ["LOG-keep", "LOG-add"]) + #expect(rebuilt.logs.first?.exerciseName == "Kept Renamed") + #expect(!rebuilt.logs.contains { $0.id == "LOG-drop" }) + } +} diff --git a/WorkoutsTests/WorkoutPathBucketingTests.swift b/WorkoutsTests/WorkoutPathBucketingTests.swift new file mode 100644 index 0000000..f07aeec --- /dev/null +++ b/WorkoutsTests/WorkoutPathBucketingTests.swift @@ -0,0 +1,86 @@ +import Foundation +import Testing +import IndieSync +@testable import Workouts + +/// Pins `WorkoutDocument.relativePath` — the `Workouts/YYYY/MM/.json` month +/// bucketing that decides where every workout file lives. The bucket derives from +/// `start` through a gregorian `Calendar`, which uses `TimeZone.current`, so the +/// tests build their dates from explicit components in that same calendar and stay +/// mid-month/mid-day (day 15, noon) so no real-world UTC offset (±14 h) can shift the +/// month — making every assertion time-zone-independent. Also documents the orphan +/// condition FIX-1 addresses in `SyncEngine.save(workout:)`: editing `start` across a +/// month boundary re-buckets the SAME id to a new path. +struct WorkoutPathBucketingTests { + + /// A date fixed to the 15th at noon in the gregorian calendar's current time zone — + /// far enough from any month edge that the UTC-offset ambiguity can't move the month + /// (matches `DuplicateCleanupPlannerTests.fixedDate`). Decomposed by `relativePath`'s + /// own `Calendar(identifier: .gregorian)`, the year/month come back stable. + private func date(year: Int, month: Int, day: Int = 15, hour: Int = 12) -> Date { + var comps = DateComponents() + comps.year = year + comps.month = month + comps.day = day + comps.hour = hour + return Calendar(identifier: .gregorian).date(from: comps)! + } + + @Test func bucketsByYearAndMonth() { + let path = WorkoutDocument.relativePath(id: "01ABC", start: date(year: 2024, month: 3)) + #expect(path == "Workouts/2024/03/01ABC.json") + } + + /// The month is zero-padded to two digits so lexical sort matches chronological sort. + @Test func padsSingleDigitMonth() { + #expect(WorkoutDocument.relativePath(id: "X", start: date(year: 2025, month: 1)) + == "Workouts/2025/01/X.json") + #expect(WorkoutDocument.relativePath(id: "X", start: date(year: 2025, month: 9)) + == "Workouts/2025/09/X.json") + } + + /// December → January crosses the year, not just the month bucket. + @Test func crossesYearBoundary() { + #expect(WorkoutDocument.relativePath(id: "DEC", start: date(year: 2024, month: 12)) + == "Workouts/2024/12/DEC.json") + #expect(WorkoutDocument.relativePath(id: "JAN", start: date(year: 2025, month: 1)) + == "Workouts/2025/01/JAN.json") + } + + /// The instance `relativePath` property and the static computation agree — the + /// document's own bucket is exactly what the engine writes it to. + @Test func instancePropertyMatchesStatic() { + let doc = WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, id: "01WK", splitID: nil, + splitName: nil, start: date(year: 2024, month: 7), end: nil, + status: WorkoutStatus.notStarted.rawValue, createdAt: date(year: 2024, month: 7), + updatedAt: date(year: 2024, month: 7), logs: [], metrics: nil + ) + #expect(doc.relativePath == "Workouts/2024/07/01WK.json") + #expect(doc.relativePath == WorkoutDocument.relativePath(id: doc.id, start: doc.start)) + } + + /// FIX-1 (rebucketing orphan) — path-computation level. `SyncEngine.save(workout:)` + /// needs a live iCloud container to exercise directly, so this pins the invariant it + /// relies on: the SAME workout id, saved after its `start` moves to a different month, + /// resolves to a DIFFERENT path. The old file at the previous bucket is therefore an + /// orphan the engine must now remove (same id lives on at the new path → plain + /// removal, no tombstone). If these two paths were ever equal the fix would be a + /// no-op; this test guarantees they diverge exactly when the month changes. + @Test func editingStartMonthRebucketsSameID() { + let id = "01WORKOUTREBUCKET00000001" + let marchStart = date(year: 2024, month: 3) + let aprilStart = date(year: 2024, month: 4) + + let oldPath = WorkoutDocument.relativePath(id: id, start: marchStart) + let newPath = WorkoutDocument.relativePath(id: id, start: aprilStart) + + #expect(oldPath != newPath) // the orphan-producing divergence + #expect(oldPath == "Workouts/2024/03/\(id).json") + #expect(newPath == "Workouts/2024/04/\(id).json") + // Same-month edits (e.g. bumping the day/hour) keep the path stable → no orphan, + // no removal. + let sameMonthLater = date(year: 2024, month: 3, day: 28, hour: 6) + #expect(WorkoutDocument.relativePath(id: id, start: sameMonthLater) == oldPath) + } +} diff --git a/WorkoutsTests/WorkoutStatusMachineTests.swift b/WorkoutsTests/WorkoutStatusMachineTests.swift new file mode 100644 index 0000000..e84bfd5 --- /dev/null +++ b/WorkoutsTests/WorkoutStatusMachineTests.swift @@ -0,0 +1,179 @@ +import Foundation +import Testing +import IndieSync +@testable import Workouts + +/// Locks the workout status state machine on `WorkoutDocument` — the single +/// source of the status-from-logs rule every screen funnels through +/// (`recomputeStatusFromLogs`, `endKeepingProgress`) plus the per-log +/// `WorkoutLogDocument.transition(to:)` timestamp bookkeeping. Includes the +/// regression for the "end re-stamped on every edit" bug: recomputing an +/// already-completed workout must keep its original `end`, not jump it to now. +struct WorkoutStatusMachineTests { + + private static let start = Date(timeIntervalSince1970: 1_700_000_000) + private static let originalEnd = Date(timeIntervalSince1970: 1_700_003_600) + + private func log(id: String, order: Int, status: WorkoutStatus) -> WorkoutLogDocument { + WorkoutLogDocument( + id: id, exerciseName: "Ex-\(id)", order: order, sets: 3, reps: 10, weight: 100, + loadType: LoadType.weight.rawValue, durationSeconds: 0, currentStateIndex: 0, + status: status.rawValue, notes: nil, date: Self.start + ) + } + + private func workout( + status: WorkoutStatus, end: Date?, logs: [WorkoutLogDocument] + ) -> WorkoutDocument { + WorkoutDocument( + schemaVersion: WorkoutDocument.currentSchemaVersion, id: "01WK", splitID: "S", + splitName: "S", start: Self.start, end: end, status: status.rawValue, + createdAt: Self.start, updatedAt: Self.start, logs: logs, metrics: nil + ) + } + + // MARK: - recomputeStatusFromLogs + + /// All logs resolved (a mix of completed and skipped) → the workout is completed and + /// gets an `end` stamped on the transition. + @Test func allResolvedCompletesAndStampsEnd() { + var w = workout(status: .inProgress, end: nil, logs: [ + log(id: "a", order: 0, status: .completed), + log(id: "b", order: 1, status: .skipped), + log(id: "c", order: 2, status: .completed), + ]) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.completed.rawValue) + #expect(w.end != nil) // stamped on the transition into completed + } + + /// Any log started but not all resolved → in progress, `end` cleared. + @Test func anyStartedIsInProgressWithNoEnd() { + var w = workout(status: .completed, end: Self.originalEnd, logs: [ + log(id: "a", order: 0, status: .completed), + log(id: "b", order: 1, status: .inProgress), + log(id: "c", order: 2, status: .notStarted), + ]) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.inProgress.rawValue) + #expect(w.end == nil) // reopened — a stale end must clear + } + + /// No log started → not started, `end` cleared. + @Test func noneStartedIsNotStarted() { + var w = workout(status: .inProgress, end: Self.originalEnd, logs: [ + log(id: "a", order: 0, status: .notStarted), + log(id: "b", order: 1, status: .notStarted), + ]) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.notStarted.rawValue) + #expect(w.end == nil) + } + + /// A workout with no logs is not "all resolved" (the guard excludes the empty set) → + /// not started. + @Test func emptyLogsIsNotStarted() { + var w = workout(status: .inProgress, end: Self.originalEnd, logs: []) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.notStarted.rawValue) + #expect(w.end == nil) + } + + /// REGRESSION (FIX-3): an already-completed workout that still has all logs resolved + /// keeps its ORIGINAL `end` when recompute runs again (e.g. the user re-opens the + /// finished workout to fix a typo). The old code re-stamped `end = Date()` every + /// time, jerking a months-old workout's end forward to now. + @Test func alreadyCompletedKeepsOriginalEnd() { + var w = workout(status: .completed, end: Self.originalEnd, logs: [ + log(id: "a", order: 0, status: .completed), + log(id: "b", order: 1, status: .skipped), + ]) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.completed.rawValue) + #expect(w.end == Self.originalEnd) // preserved, not re-stamped to now + } + + /// A workout marked completed but somehow missing an `end` (defensive: a legacy or + /// partially-written file) gets one stamped — the preservation only applies when a + /// real end already exists. + @Test func completedWithNilEndGetsStamped() { + var w = workout(status: .completed, end: nil, logs: [ + log(id: "a", order: 0, status: .completed), + ]) + w.recomputeStatusFromLogs() + #expect(w.status == WorkoutStatus.completed.rawValue) + #expect(w.end != nil) + } + + // MARK: - endKeepingProgress + + /// Ending a run in progress skips every not-completed log, keeps completed ones, and + /// resolves the workout to completed with an `end` and a bumped `updatedAt`. + @Test func endKeepingProgressSkipsRemainderAndCompletes() { + var w = workout(status: .inProgress, end: nil, logs: [ + log(id: "a", order: 0, status: .completed), + log(id: "b", order: 1, status: .inProgress), + log(id: "c", order: 2, status: .notStarted), + ]) + w.endKeepingProgress() + + #expect(w.status == WorkoutStatus.completed.rawValue) + #expect(w.end != nil) + #expect(w.logs[0].status == WorkoutStatus.completed.rawValue) // kept + #expect(w.logs[1].status == WorkoutStatus.skipped.rawValue) // was in progress + #expect(w.logs[2].status == WorkoutStatus.skipped.rawValue) // was not started + #expect(w.updatedAt > Self.start) + } + + // MARK: - WorkoutLogDocument.transition + + @Test func transitionToInProgressStampsStartOnce() { + var l = log(id: "a", order: 0, status: .notStarted) + l.transition(to: .inProgress) + let firstStart = l.startedAt + #expect(firstStart != nil) + #expect(l.completedAt == nil) + + // Re-entering inProgress must not overwrite the original startedAt. + l.transition(to: .inProgress) + #expect(l.startedAt == firstStart) + } + + @Test func transitionToCompletedStampsCompletionOnceAndKeepsStart() { + var l = log(id: "a", order: 0, status: .notStarted) + l.transition(to: .inProgress) + let started = l.startedAt + l.transition(to: .completed) + let firstCompleted = l.completedAt + #expect(l.status == WorkoutStatus.completed.rawValue) + #expect(firstCompleted != nil) + #expect(l.startedAt == started) // start preserved through completion + + // A second completed transition must not move completedAt. + l.transition(to: .completed) + #expect(l.completedAt == firstCompleted) + } + + @Test func transitionToNotStartedClearsBothTimestamps() { + var l = log(id: "a", order: 0, status: .completed) + l.transition(to: .inProgress) + l.transition(to: .completed) + l.transition(to: .notStarted) + #expect(l.status == WorkoutStatus.notStarted.rawValue) + #expect(l.startedAt == nil) + #expect(l.completedAt == nil) + } + + /// Skipping clears any completion stamp but leaves `startedAt` (the exercise may + /// have been partway through before the user skipped the rest). + @Test func transitionToSkippedClearsCompletionKeepsStart() { + var l = log(id: "a", order: 0, status: .notStarted) + l.transition(to: .inProgress) + l.transition(to: .completed) + let started = l.startedAt + l.transition(to: .skipped) + #expect(l.status == WorkoutStatus.skipped.rawValue) + #expect(l.completedAt == nil) + #expect(l.startedAt == started) + } +}