From 15926cdf394f93777e904bceb707eadad31ff585 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sat, 16 May 2026 23:23:40 -0700 Subject: [PATCH 1/7] fix: detect ENOSPC via CustomStringConvertible when POSIX type is lost LZMAError.internalError flattens errno to a string message, so its localizedDescription returns "Internal Error" (not the errno text), causing isOutOfDiskSpace to miss the out-of-disk-space condition. The string fallback now also checks String(describing:) and failureReason, where the errno message ("Failed to write: No space left on device") actually lives. Fixes SF50-TOLD-29. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../xcshareddata/xcschemes/SF50 TOLD.xcscheme | 3 + .../TerrainLoader/TerrainDataLoader.swift | 13 ++- .../TerrainDataLoaderErrorTests.swift | 97 +++++++++++++++++++ 3 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 SF50 TOLDTests/TerrainDataLoaderErrorTests.swift diff --git a/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme b/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme index 0baf714..44bfbe6 100644 --- a/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme +++ b/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme @@ -36,6 +36,9 @@ reference = "container:SF50 SharedTests/SF50 Shared Unit Tests.xctestplan" default = "YES"> + + diff --git a/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift b/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift index 6d62187..3848602 100644 --- a/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift +++ b/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift @@ -652,13 +652,16 @@ enum TerrainDataLoaderError: LocalizedError { } } -private extension Error { +extension Error { /// Whether this error (or any error in its `NSUnderlyingErrorKey` chain) /// represents a POSIX `ENOSPC` "No space left on device" condition. /// /// `StreamingLZMA` wraps the underlying `errno` string in a String-typed - /// `LZMAError.internalError` case, so the string fallback is needed when - /// the typed POSIX error has been lost. + /// `LZMAError.internalError` case, so a string fallback is needed when + /// the typed POSIX error has been lost. The fallback checks + /// `String(describing:)` (CustomStringConvertible) and `failureReason` + /// because `localizedDescription` returns the localized `errorDescription` + /// ("Internal Error"), which does not contain the errno text. var isOutOfDiskSpace: Bool { let nsError = self as NSError if nsError.domain == NSPOSIXErrorDomain, nsError.code == Int(ENOSPC) { @@ -670,6 +673,8 @@ private extension Error { if let underlying = nsError.userInfo[NSUnderlyingErrorKey] as? Error { return underlying.isOutOfDiskSpace } - return localizedDescription.contains("No space left on device") + let enospcText = "No space left on device" + return String(describing: self).contains(enospcText) + || (self as? LocalizedError)?.failureReason?.contains(enospcText) == true } } diff --git a/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift b/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift new file mode 100644 index 0000000..f606a90 --- /dev/null +++ b/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift @@ -0,0 +1,97 @@ +import StreamingLZMA +import Testing + +@testable import SF50_TOLD + +struct TerrainDataLoaderErrorTests { + + // MARK: - ENOSPC detection via LZMAError + + /// Verifies that an `LZMAError.internalError` wrapping the POSIX "No space left on device" + /// message (as produced by StreamingLZMA after flattening `errno` to a string) is + /// classified as an out-of-disk-space error. This is the root cause of SF50-TOLD-29: + /// `localizedDescription` returns the localized `errorDescription` ("Internal Error"), + /// which does not contain the errno text, so the string fallback must look at + /// `String(describing:)` (CustomStringConvertible) or `failureReason` instead. + @Test + func lzmaInternalErrorWithENOSPCMessageIsDetectedAsOutOfDiskSpace() { + let lzmaError = LZMAError.internalError("Failed to write: No space left on device") + #expect(lzmaError.isOutOfDiskSpace) + } + + /// Verifies that an `LZMAError` whose message does not mention disk space is not + /// misclassified as an out-of-disk-space error. + @Test + func lzmaInternalErrorWithUnrelatedMessageIsNotOutOfDiskSpace() { + let lzmaError = LZMAError.internalError("Failed to write: Input/output error") + #expect(!lzmaError.isOutOfDiskSpace) + } + + /// Verifies that a genuine POSIX ENOSPC error is still detected. + @Test + func posixENOSPCErrorIsDetectedAsOutOfDiskSpace() { + let posixError = POSIXError(.ENOSPC) + #expect(posixError.isOutOfDiskSpace) + } + + /// Verifies that a POSIX error with a different code is not misclassified. + @Test + func posixEIOErrorIsNotOutOfDiskSpace() { + let posixError = POSIXError(.EIO) + #expect(!posixError.isOutOfDiskSpace) + } + + // MARK: - TerrainDataLoaderError reportability + + /// Verifies that `.outOfDiskSpace` is not reportable to Sentry, so the crash + /// tracker is not flooded with user-side storage conditions. + @Test + func outOfDiskSpaceErrorIsNotReportable() { + #expect(!TerrainDataLoaderError.outOfDiskSpace.isReportable) + } + + /// Verifies that `.decompressionFailed` wrapping an LZMA ENOSPC error is + /// never constructed — i.e. the mapping in `streamingDecompress` correctly + /// throws `.outOfDiskSpace` instead, making the error non-reportable and + /// presenting the correct recovery suggestion to the user. + @Test + func lzmaENOSPCErrorMapsToOutOfDiskSpaceNotDecompressionFailed() { + let lzmaError = LZMAError.internalError("Failed to write: No space left on device") + + // Reproduce the mapping logic from streamingDecompress + let terrainError: TerrainDataLoaderError = + if lzmaError.isOutOfDiskSpace { + .outOfDiskSpace + } else { + .decompressionFailed(lzmaError) + } + + #expect(terrainError == .outOfDiskSpace) + #expect(!terrainError.isReportable) + #expect(terrainError.recoverySuggestion != nil) + + // Make sure it does NOT give "Check your internet connection" advice + let badAdvice = "Check your internet connection" + if let suggestion = terrainError.recoverySuggestion { + #expect(!suggestion.contains(badAdvice)) + } + } +} + +// TerrainDataLoaderError must be Equatable for the test assertions above. +extension TerrainDataLoaderError: Equatable { + public static func == (lhs: TerrainDataLoaderError, rhs: TerrainDataLoaderError) -> Bool { + switch (lhs, rhs) { + case (.noStorageAccess, .noStorageAccess), + (.outOfDiskSpace, .outOfDiskSpace): + return true + case (.regionNotAvailable(let l), .regionNotAvailable(let r)): + return l == r + case (.downloadFailed, .downloadFailed), + (.decompressionFailed, .decompressionFailed): + return true + default: + return false + } + } +} From d229f61329127cb25b04841cc4936a61788f579d Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sat, 16 May 2026 23:29:25 -0700 Subject: [PATCH 2/7] Move all NavDataLoaderViewModel SwiftData work off the main thread Fixes SF50-TOLD-20: ~14s fully-blocked main thread (AppHang) firing shortly after launch on 3.5.3+44. Root cause: NavDataLoaderViewModel (an @Observable @MainActor class) still performed synchronous SwiftData operations on container.mainContext from the MainActor: - init() called recalculate() -> NavDataStateHelper.fetchState() -> mainContext.fetch (invoked from ContentView.onAppear at launch) - the .schemaVersion observer task called recalculate() on mainContext, firing exactly when load() set Defaults[.schemaVersion] immediately after the bulk import - load()'s completion block did mainContext.insert/save and a synchronous recalculate() on mainContext right after the import - clearCycles() deleted + saved on mainContext While the NavDataLoader @ModelActor holds the persistent store coordinator during the multi-batch nav-data import (resetData() cascade deletes plus tens of thousands of inserts), any of these mainContext accesses block in -[NSManagedObjectContext performBlockAndWait:] waiting for store-coordinator ownership, matching the Sentry stack (_dispatch_main_queue_drain -> swift_job_runImpl -> performBlockAndWait -> __DISPATCH_WAIT_FOR_QUEUE__). Commit c15f8ca moved the polling loop off-main and 5093289 removed the synchronous launch purge, but these synchronous mainContext paths remained. Fix (extends, does not regress, c15f8ca/5093289 intent): - init() no longer fetches synchronously; loader state is computed exclusively off-main via ModelContext(container) by the existing detached poller (first iteration runs immediately) and a detached schema-version observer, applied back on the MainActor. - Cycle persistence and the pre-load Cycle reset moved onto the NavDataLoader @ModelActor (writeCycles/clearCycles) so they run on its background context, never the main context. - load() now only writes cheap UserDefaults values on the MainActor; no SwiftData touches the main context anywhere in the view model. Added NavDataStateHelperTests: exercises the off-main state computation (NavDataStateHelper.fetchState) against a background ModelContext(container) across empty / current / expired-cycle / stale-schema scenarios, pinning the contract the fix relies on so a regression back to a main-context fetch is caught. A true unit test of the watchdog hang itself isn't practical (it requires real store-coordinator contention during a multi-minute network import), so coverage targets the extracted off-main unit instead. Wired the existing SF50 TOLDTests/Unit Tests.xctestplan into the SF50 TOLD scheme so the SF50 TOLDTests target's unit tests run. Verified: build-for-testing succeeded; NavDataStateHelperTests 4/4 passed; SF50 Shared Unit Tests plan fully green; swift-format and swiftlint clean on changed files. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Loaders/NavDataLoader/NavDataLoader.swift | 33 +++++ .../NavDataLoaderViewModel.swift | 137 ++++++----------- SF50 TOLDTests/NavDataStateHelperTests.swift | 140 ++++++++++++++++++ 3 files changed, 217 insertions(+), 93 deletions(-) create mode 100644 SF50 TOLDTests/NavDataStateHelperTests.swift diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift index bfc849f..2b12654 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift @@ -116,6 +116,8 @@ actor NavDataLoader { self.state = .loading(progress: Float(airportCount + obstaclesProcessed) / Float(totalItems)) } + writeCycles(nasr.cycles) + state = .finished return LoadResult( cycles: nasr.cycles, @@ -123,6 +125,37 @@ actor NavDataLoader { ) } + /// Deletes all persisted ``Cycle`` records on the loader's background context. + /// + /// Performed off the main thread so it never contends with the main + /// `NSManagedObjectContext` for the persistent store coordinator. + func clearCycles() throws { + try modelContext.delete(model: Cycle.self) + try modelContext.save() + } + + private func writeCycles(_ cycles: AirportDataCodable.DataCycles) { + insertCycle(cycles.nasr, source: .nasr) + insertCycle(cycles.cifp, source: .cifp) + insertCycle(cycles.dof, source: .dof) + try? modelContext.save() + } + + private func insertCycle( + _ info: AirportDataCodable.CycleInfo?, + source: CycleDataSource + ) { + guard let info else { return } + modelContext.insert( + Cycle( + dataSource: source, + name: info.name, + effective: info.effective, + expires: info.expires + ) + ) + } + private func download(progress: (Float) -> Void) async throws -> Data { try await withRetry(logger: logger, label: "nav data") { let session = URLSession(configuration: .ephemeral) diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift index e58731f..afec553 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift @@ -45,56 +45,51 @@ final class NavDataLoaderViewModel: WithIdentifiableError { init(container: ModelContainer) { self.container = container - do { - try recalculate() - } catch { - SentrySDK.capture(error: error) { scope in - scope.setFingerprint(["navData", "recalculate"]) - } - self.error = error - } - setupObservation() } private func setupObservation() { - addTask( - Task { - for await _ in Defaults.updates(.schemaVersion) - where !Task.isCancelled { - do { - try recalculate() - } catch { - SentrySDK.capture(error: error) { scope in - scope.setFingerprint(["navData", "recalculate"]) - } - self.error = error - } - } + addTask(schemaVersionObservationTask()) + addTask(statePollingTask()) + } + + private func schemaVersionObservationTask() -> Task { + Task.detached { [container] in + let context = ModelContext(container) + for await _ in Defaults.updates(.schemaVersion) + where !Task.isCancelled { + await self.refreshState(from: context, fingerprint: "recalculate") } - ) + } + } - addTask( - Task.detached { [container] in - do { - let context = ModelContext(container) - while !Task.isCancelled { - let state = try NavDataStateHelper.fetchState(context: context) - await MainActor.run { - self.applyState(state) - } - try? await Task.sleep(for: .seconds(0.5)) - } - } catch { - await MainActor.run { - SentrySDK.capture(error: error) { scope in - scope.setFingerprint(["navData", "airportCheck"]) - } - self.error = error - } + private func statePollingTask() -> Task { + Task.detached { [container] in + let context = ModelContext(container) + while !Task.isCancelled { + await self.refreshState(from: context, fingerprint: "airportCheck") + try? await Task.sleep(for: .seconds(0.5)) + } + } + } + + nonisolated private func refreshState( + from context: ModelContext, + fingerprint: String + ) async { + do { + let state = try NavDataStateHelper.fetchState(context: context) + await MainActor.run { + self.applyState(state) + } + } catch { + await MainActor.run { + SentrySDK.capture(error: error) { scope in + scope.setFingerprint(["navData", fingerprint]) } + self.error = error } - ) + } } private func addTask(_ task: Task) { @@ -112,46 +107,12 @@ final class NavDataLoaderViewModel: WithIdentifiableError { ) do { error = nil - try clearCycles() + try await loader.clearCycles() + Defaults[.ourAirportsLastUpdated] = nil let result = try await loader.load() - await MainActor.run { - let context = container.mainContext - if let nasr = result.cycles.nasr { - context.insert( - Cycle( - dataSource: .nasr, - name: nasr.name, - effective: nasr.effective, - expires: nasr.expires - ) - ) - } - if let cifp = result.cycles.cifp { - context.insert( - Cycle( - dataSource: .cifp, - name: cifp.name, - effective: cifp.effective, - expires: cifp.expires - ) - ) - } - if let dof = result.cycles.dof { - context.insert( - Cycle( - dataSource: .dof, - name: dof.name, - effective: dof.effective, - expires: dof.expires - ) - ) - } - try? context.save() - try? self.recalculate() - Defaults[.ourAirportsLastUpdated] = result.ourAirportsLastUpdated - Defaults[.schemaVersion] = latestSchemaVersion - } + Defaults[.ourAirportsLastUpdated] = result.ourAirportsLastUpdated + Defaults[.schemaVersion] = latestSchemaVersion transaction.finish() } catch { transaction.finish(status: .internalError) @@ -179,18 +140,6 @@ final class NavDataLoaderViewModel: WithIdentifiableError { if canSkip { deferred = true } } - private func clearCycles() throws { - let context = container.mainContext - try context.delete(model: Cycle.self) - try context.save() - Defaults[.ourAirportsLastUpdated] = nil - } - - private func recalculate() throws { - let state = try NavDataStateHelper.fetchState(context: container.mainContext) - applyState(state) - } - private func applyState(_ state: NavDataStateHelper.State) { if noData != state.noData { self.noData = state.noData } if needsLoad != state.needsLoad { self.needsLoad = state.needsLoad } @@ -202,7 +151,9 @@ final class NavDataLoaderViewModel: WithIdentifiableError { /// /// Declared outside `NavDataLoaderViewModel` so it is nonisolated by default /// and callable from both MainActor and background tasks without annotations. -private enum NavDataStateHelper { +/// It is intentionally non-`private` so the off-main state computation can be +/// exercised directly by unit tests via `@testable import`. +enum NavDataStateHelper { static func fetchState(context: ModelContext) throws -> State { var airportDescriptor = FetchDescriptor() airportDescriptor.fetchLimit = 1 diff --git a/SF50 TOLDTests/NavDataStateHelperTests.swift b/SF50 TOLDTests/NavDataStateHelperTests.swift new file mode 100644 index 0000000..74fbf53 --- /dev/null +++ b/SF50 TOLDTests/NavDataStateHelperTests.swift @@ -0,0 +1,140 @@ +import Defaults +import Foundation +import SF50_Shared +import SwiftData +import Testing + +@testable import SF50_TOLD + +/// Regression coverage for SF50-TOLD-20. +/// +/// The launch hang was caused by `NavDataLoaderViewModel` computing its loader +/// state with a synchronous `container.mainContext` fetch on the MainActor while +/// the `NavDataLoader` `@ModelActor` held the persistent store coordinator during +/// a bulk nav-data import. The fix routes all state computation through +/// `NavDataStateHelper.fetchState(context:)` running on a background +/// `ModelContext(container)`. +/// +/// These tests pin the contract the fix depends on: `fetchState` must produce +/// correct results when invoked against a background context that is *not* the +/// container's main context, so the view model never has to touch the main +/// context to decide whether to show the loader. +@MainActor +struct NavDataStateHelperTests { + private func makeContainer() throws -> ModelContainer { + try ModelContainer( + for: Airport.self, + Runway.self, + NOTAM.self, + Scenario.self, + Cycle.self, + Obstacle.self, + Procedure.self, + ProcedureSegment.self, + Leg.self, + configurations: .init(isStoredInMemoryOnly: true) + ) + } + + private func withSchemaVersion(_ version: Int, _ body: () throws -> T) rethrows -> T { + let original = Defaults[.schemaVersion] + Defaults[.schemaVersion] = version + defer { Defaults[.schemaVersion] = original } + return try body() + } + + @Test + func reportsNoDataWhenStoreIsEmpty() throws { + let container = try makeContainer() + let backgroundContext = ModelContext(container) + + let state = try withSchemaVersion(latestSchemaVersion) { + try NavDataStateHelper.fetchState(context: backgroundContext) + } + + #expect(state.noData) + #expect(state.needsLoad) + #expect(!state.canSkip) + } + + @Test + func backgroundContextSeesDataWrittenByMainContext() throws { + let container = try makeContainer() + + let (airport, runways) = AirportBuilder.KSQL.build() + container.mainContext.insert(airport) + for runway in runways { container.mainContext.insert(runway) } + container.mainContext.insert( + Cycle( + dataSource: .nasr, + name: "2501", + effective: Date.now.addingTimeInterval(-86_400), + expires: Date.now.addingTimeInterval(86_400) + ) + ) + try container.mainContext.save() + + let backgroundContext = ModelContext(container) + let state = try withSchemaVersion(latestSchemaVersion) { + try NavDataStateHelper.fetchState(context: backgroundContext) + } + + #expect(!state.noData) + #expect(!state.needsLoad) + #expect(state.canSkip) + } + + @Test + func needsLoadWhenNASRCycleHasExpired() throws { + let container = try makeContainer() + + let (airport, runways) = AirportBuilder.KSQL.build() + container.mainContext.insert(airport) + for runway in runways { container.mainContext.insert(runway) } + container.mainContext.insert( + Cycle( + dataSource: .nasr, + name: "2401", + effective: Date.now.addingTimeInterval(-172_800), + expires: Date.now.addingTimeInterval(-86_400) + ) + ) + try container.mainContext.save() + + let backgroundContext = ModelContext(container) + let state = try withSchemaVersion(latestSchemaVersion) { + try NavDataStateHelper.fetchState(context: backgroundContext) + } + + #expect(!state.noData) + #expect(state.needsLoad) + #expect(state.canSkip) + } + + @Test + func staleSchemaVersionForcesReloadAndBlocksSkipping() throws { + let container = try makeContainer() + + let (airport, runways) = AirportBuilder.KSQL.build() + container.mainContext.insert(airport) + for runway in runways { container.mainContext.insert(runway) } + container.mainContext.insert( + Cycle( + dataSource: .nasr, + name: "2501", + effective: Date.now.addingTimeInterval(-86_400), + expires: Date.now.addingTimeInterval(86_400) + ) + ) + try container.mainContext.save() + + let backgroundContext = ModelContext(container) + let state = try withSchemaVersion(latestSchemaVersion - 1) { + try NavDataStateHelper.fetchState(context: backgroundContext) + } + + #expect(!state.noData) + #expect(state.needsLoad) + #expect(!state.canSkip) + } +} From 9cb85db3c6607ae2455fc836356cba7165947546 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sat, 16 May 2026 23:34:54 -0700 Subject: [PATCH 3/7] test: assert ENOSPC mapping via pattern match, drop retroactive Equatable Removes the test-only `extension TerrainDataLoaderError: Equatable` (retroactive conformance of an imported type to an imported protocol, which the compiler warns about) in favor of an `if case` pattern match for the single equality assertion. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../TerrainDataLoaderErrorTests.swift | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift b/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift index f606a90..132ab8f 100644 --- a/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift +++ b/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift @@ -66,7 +66,9 @@ struct TerrainDataLoaderErrorTests { .decompressionFailed(lzmaError) } - #expect(terrainError == .outOfDiskSpace) + let mappedToOutOfDiskSpace: Bool = + if case .outOfDiskSpace = terrainError { true } else { false } + #expect(mappedToOutOfDiskSpace) #expect(!terrainError.isReportable) #expect(terrainError.recoverySuggestion != nil) @@ -77,21 +79,3 @@ struct TerrainDataLoaderErrorTests { } } } - -// TerrainDataLoaderError must be Equatable for the test assertions above. -extension TerrainDataLoaderError: Equatable { - public static func == (lhs: TerrainDataLoaderError, rhs: TerrainDataLoaderError) -> Bool { - switch (lhs, rhs) { - case (.noStorageAccess, .noStorageAccess), - (.outOfDiskSpace, .outOfDiskSpace): - return true - case (.regionNotAvailable(let l), .regionNotAvailable(let r)): - return l == r - case (.downloadFailed, .downloadFailed), - (.decompressionFailed, .decompressionFailed): - return true - default: - return false - } - } -} From ef1fe35d8c975c86725dc478af18921b4726e64a Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sun, 17 May 2026 15:20:07 -0700 Subject: [PATCH 4/7] Address PR review: propagate cycle-save error, drop white-box tests - NavDataLoader.writeCycles: stop swallowing the save error with try?; it now throws and propagates through load(), so a failed Cycle save surfaces (and is captured by the existing Sentry catch in NavDataLoaderViewModel) instead of silently corrupting cycle state. Consistent with the sibling clearCycles(). - Remove SF50 TOLDTests/NavDataStateHelperTests.swift (white-box tests of an internal helper) and restore NavDataStateHelper to private. Per review: unit tests should be black box; the off-main guarantee is not reasonably black-box testable, so it is left untested. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Loaders/NavDataLoader/NavDataLoader.swift | 6 +- .../NavDataLoaderViewModel.swift | 4 +- SF50 TOLDTests/NavDataStateHelperTests.swift | 140 ------------------ 3 files changed, 4 insertions(+), 146 deletions(-) delete mode 100644 SF50 TOLDTests/NavDataStateHelperTests.swift diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift index 2b12654..e0f9e9f 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift @@ -116,7 +116,7 @@ actor NavDataLoader { self.state = .loading(progress: Float(airportCount + obstaclesProcessed) / Float(totalItems)) } - writeCycles(nasr.cycles) + try writeCycles(nasr.cycles) state = .finished return LoadResult( @@ -134,11 +134,11 @@ actor NavDataLoader { try modelContext.save() } - private func writeCycles(_ cycles: AirportDataCodable.DataCycles) { + private func writeCycles(_ cycles: AirportDataCodable.DataCycles) throws { insertCycle(cycles.nasr, source: .nasr) insertCycle(cycles.cifp, source: .cifp) insertCycle(cycles.dof, source: .dof) - try? modelContext.save() + try modelContext.save() } private func insertCycle( diff --git a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift index afec553..c364506 100644 --- a/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift +++ b/SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift @@ -151,9 +151,7 @@ final class NavDataLoaderViewModel: WithIdentifiableError { /// /// Declared outside `NavDataLoaderViewModel` so it is nonisolated by default /// and callable from both MainActor and background tasks without annotations. -/// It is intentionally non-`private` so the off-main state computation can be -/// exercised directly by unit tests via `@testable import`. -enum NavDataStateHelper { +private enum NavDataStateHelper { static func fetchState(context: ModelContext) throws -> State { var airportDescriptor = FetchDescriptor() airportDescriptor.fetchLimit = 1 diff --git a/SF50 TOLDTests/NavDataStateHelperTests.swift b/SF50 TOLDTests/NavDataStateHelperTests.swift deleted file mode 100644 index 74fbf53..0000000 --- a/SF50 TOLDTests/NavDataStateHelperTests.swift +++ /dev/null @@ -1,140 +0,0 @@ -import Defaults -import Foundation -import SF50_Shared -import SwiftData -import Testing - -@testable import SF50_TOLD - -/// Regression coverage for SF50-TOLD-20. -/// -/// The launch hang was caused by `NavDataLoaderViewModel` computing its loader -/// state with a synchronous `container.mainContext` fetch on the MainActor while -/// the `NavDataLoader` `@ModelActor` held the persistent store coordinator during -/// a bulk nav-data import. The fix routes all state computation through -/// `NavDataStateHelper.fetchState(context:)` running on a background -/// `ModelContext(container)`. -/// -/// These tests pin the contract the fix depends on: `fetchState` must produce -/// correct results when invoked against a background context that is *not* the -/// container's main context, so the view model never has to touch the main -/// context to decide whether to show the loader. -@MainActor -struct NavDataStateHelperTests { - private func makeContainer() throws -> ModelContainer { - try ModelContainer( - for: Airport.self, - Runway.self, - NOTAM.self, - Scenario.self, - Cycle.self, - Obstacle.self, - Procedure.self, - ProcedureSegment.self, - Leg.self, - configurations: .init(isStoredInMemoryOnly: true) - ) - } - - private func withSchemaVersion(_ version: Int, _ body: () throws -> T) rethrows -> T { - let original = Defaults[.schemaVersion] - Defaults[.schemaVersion] = version - defer { Defaults[.schemaVersion] = original } - return try body() - } - - @Test - func reportsNoDataWhenStoreIsEmpty() throws { - let container = try makeContainer() - let backgroundContext = ModelContext(container) - - let state = try withSchemaVersion(latestSchemaVersion) { - try NavDataStateHelper.fetchState(context: backgroundContext) - } - - #expect(state.noData) - #expect(state.needsLoad) - #expect(!state.canSkip) - } - - @Test - func backgroundContextSeesDataWrittenByMainContext() throws { - let container = try makeContainer() - - let (airport, runways) = AirportBuilder.KSQL.build() - container.mainContext.insert(airport) - for runway in runways { container.mainContext.insert(runway) } - container.mainContext.insert( - Cycle( - dataSource: .nasr, - name: "2501", - effective: Date.now.addingTimeInterval(-86_400), - expires: Date.now.addingTimeInterval(86_400) - ) - ) - try container.mainContext.save() - - let backgroundContext = ModelContext(container) - let state = try withSchemaVersion(latestSchemaVersion) { - try NavDataStateHelper.fetchState(context: backgroundContext) - } - - #expect(!state.noData) - #expect(!state.needsLoad) - #expect(state.canSkip) - } - - @Test - func needsLoadWhenNASRCycleHasExpired() throws { - let container = try makeContainer() - - let (airport, runways) = AirportBuilder.KSQL.build() - container.mainContext.insert(airport) - for runway in runways { container.mainContext.insert(runway) } - container.mainContext.insert( - Cycle( - dataSource: .nasr, - name: "2401", - effective: Date.now.addingTimeInterval(-172_800), - expires: Date.now.addingTimeInterval(-86_400) - ) - ) - try container.mainContext.save() - - let backgroundContext = ModelContext(container) - let state = try withSchemaVersion(latestSchemaVersion) { - try NavDataStateHelper.fetchState(context: backgroundContext) - } - - #expect(!state.noData) - #expect(state.needsLoad) - #expect(state.canSkip) - } - - @Test - func staleSchemaVersionForcesReloadAndBlocksSkipping() throws { - let container = try makeContainer() - - let (airport, runways) = AirportBuilder.KSQL.build() - container.mainContext.insert(airport) - for runway in runways { container.mainContext.insert(runway) } - container.mainContext.insert( - Cycle( - dataSource: .nasr, - name: "2501", - effective: Date.now.addingTimeInterval(-86_400), - expires: Date.now.addingTimeInterval(86_400) - ) - ) - try container.mainContext.save() - - let backgroundContext = ModelContext(container) - let state = try withSchemaVersion(latestSchemaVersion - 1) { - try NavDataStateHelper.fetchState(context: backgroundContext) - } - - #expect(!state.noData) - #expect(state.needsLoad) - #expect(!state.canSkip) - } -} From 804c98f8abaa4f65355bb219ddaa4b525ec5a93e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sun, 17 May 2026 16:55:23 -0700 Subject: [PATCH 5/7] Adopt StreamingLZMA 1.2.0; drop the ENOSPC string-matching workaround StreamingLZMA 1.2.0 throws LZMAError.ioFailure carrying the raw errno and bridges it (via CustomNSError) to an NSError with an NSPOSIXErrorDomain underlying error. The existing typed NSUnderlyingErrorKey branch in isOutOfDiskSpace now detects ENOSPC directly, so the String(describing:)/failureReason fallback is no longer needed. - Bump StreamingLZMA pin 1.1.0 -> 1.2.0 - isOutOfDiskSpace: remove the string fallback, restore to private - Remove TerrainDataLoaderErrorTests (covered the removed workaround; the remaining typed detection is trivial/obvious composition over Foundation + StreamingLZMA behavior) - Revert the SF50 TOLDTests test-plan wiring added for those tests (scheme now matches main) Fully resolves the StreamingLZMA review comment on #7. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../xcshareddata/swiftpm/Package.resolved | 4 +- .../xcshareddata/xcschemes/SF50 TOLD.xcscheme | 3 - .../TerrainLoader/TerrainDataLoader.swift | 16 ++-- .../TerrainDataLoaderErrorTests.swift | 81 ------------------- 4 files changed, 8 insertions(+), 96 deletions(-) delete mode 100644 SF50 TOLDTests/TerrainDataLoaderErrorTests.swift diff --git a/SF50 TOLD.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/SF50 TOLD.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 7e88cf4..e5de942 100644 --- a/SF50 TOLD.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/SF50 TOLD.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -51,8 +51,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/RISCfuture/StreamingLZMA", "state" : { - "revision" : "3773bb684557d2cca47ef29bd04298291a4f7204", - "version" : "1.1.0" + "revision" : "17a9d98a56bdbae7c163f2b43460e19003ed39e3", + "version" : "1.2.0" } }, { diff --git a/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme b/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme index 44bfbe6..0baf714 100644 --- a/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme +++ b/SF50 TOLD.xcodeproj/xcshareddata/xcschemes/SF50 TOLD.xcscheme @@ -36,9 +36,6 @@ reference = "container:SF50 SharedTests/SF50 Shared Unit Tests.xctestplan" default = "YES"> - - diff --git a/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift b/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift index 3848602..ed188cf 100644 --- a/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift +++ b/SF50 TOLD/Loaders/TerrainLoader/TerrainDataLoader.swift @@ -652,16 +652,14 @@ enum TerrainDataLoaderError: LocalizedError { } } -extension Error { +private extension Error { /// Whether this error (or any error in its `NSUnderlyingErrorKey` chain) /// represents a POSIX `ENOSPC` "No space left on device" condition. /// - /// `StreamingLZMA` wraps the underlying `errno` string in a String-typed - /// `LZMAError.internalError` case, so a string fallback is needed when - /// the typed POSIX error has been lost. The fallback checks - /// `String(describing:)` (CustomStringConvertible) and `failureReason` - /// because `localizedDescription` returns the localized `errorDescription` - /// ("Internal Error"), which does not contain the errno text. + /// `StreamingLZMA` surfaces a failed-write `errno` as `LZMAError.ioFailure`, + /// which bridges (via `CustomNSError`) to an `NSError` carrying an + /// `NSPOSIXErrorDomain` underlying error, so the typed checks below are + /// sufficient. var isOutOfDiskSpace: Bool { let nsError = self as NSError if nsError.domain == NSPOSIXErrorDomain, nsError.code == Int(ENOSPC) { @@ -673,8 +671,6 @@ extension Error { if let underlying = nsError.userInfo[NSUnderlyingErrorKey] as? Error { return underlying.isOutOfDiskSpace } - let enospcText = "No space left on device" - return String(describing: self).contains(enospcText) - || (self as? LocalizedError)?.failureReason?.contains(enospcText) == true + return false } } diff --git a/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift b/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift deleted file mode 100644 index 132ab8f..0000000 --- a/SF50 TOLDTests/TerrainDataLoaderErrorTests.swift +++ /dev/null @@ -1,81 +0,0 @@ -import StreamingLZMA -import Testing - -@testable import SF50_TOLD - -struct TerrainDataLoaderErrorTests { - - // MARK: - ENOSPC detection via LZMAError - - /// Verifies that an `LZMAError.internalError` wrapping the POSIX "No space left on device" - /// message (as produced by StreamingLZMA after flattening `errno` to a string) is - /// classified as an out-of-disk-space error. This is the root cause of SF50-TOLD-29: - /// `localizedDescription` returns the localized `errorDescription` ("Internal Error"), - /// which does not contain the errno text, so the string fallback must look at - /// `String(describing:)` (CustomStringConvertible) or `failureReason` instead. - @Test - func lzmaInternalErrorWithENOSPCMessageIsDetectedAsOutOfDiskSpace() { - let lzmaError = LZMAError.internalError("Failed to write: No space left on device") - #expect(lzmaError.isOutOfDiskSpace) - } - - /// Verifies that an `LZMAError` whose message does not mention disk space is not - /// misclassified as an out-of-disk-space error. - @Test - func lzmaInternalErrorWithUnrelatedMessageIsNotOutOfDiskSpace() { - let lzmaError = LZMAError.internalError("Failed to write: Input/output error") - #expect(!lzmaError.isOutOfDiskSpace) - } - - /// Verifies that a genuine POSIX ENOSPC error is still detected. - @Test - func posixENOSPCErrorIsDetectedAsOutOfDiskSpace() { - let posixError = POSIXError(.ENOSPC) - #expect(posixError.isOutOfDiskSpace) - } - - /// Verifies that a POSIX error with a different code is not misclassified. - @Test - func posixEIOErrorIsNotOutOfDiskSpace() { - let posixError = POSIXError(.EIO) - #expect(!posixError.isOutOfDiskSpace) - } - - // MARK: - TerrainDataLoaderError reportability - - /// Verifies that `.outOfDiskSpace` is not reportable to Sentry, so the crash - /// tracker is not flooded with user-side storage conditions. - @Test - func outOfDiskSpaceErrorIsNotReportable() { - #expect(!TerrainDataLoaderError.outOfDiskSpace.isReportable) - } - - /// Verifies that `.decompressionFailed` wrapping an LZMA ENOSPC error is - /// never constructed — i.e. the mapping in `streamingDecompress` correctly - /// throws `.outOfDiskSpace` instead, making the error non-reportable and - /// presenting the correct recovery suggestion to the user. - @Test - func lzmaENOSPCErrorMapsToOutOfDiskSpaceNotDecompressionFailed() { - let lzmaError = LZMAError.internalError("Failed to write: No space left on device") - - // Reproduce the mapping logic from streamingDecompress - let terrainError: TerrainDataLoaderError = - if lzmaError.isOutOfDiskSpace { - .outOfDiskSpace - } else { - .decompressionFailed(lzmaError) - } - - let mappedToOutOfDiskSpace: Bool = - if case .outOfDiskSpace = terrainError { true } else { false } - #expect(mappedToOutOfDiskSpace) - #expect(!terrainError.isReportable) - #expect(terrainError.recoverySuggestion != nil) - - // Make sure it does NOT give "Check your internet connection" advice - let badAdvice = "Check your internet connection" - if let suggestion = terrainError.recoverySuggestion { - #expect(!suggestion.contains(badAdvice)) - } - } -} From 93231a31524050ecb6e2dfbedc09026b4c649f85 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sun, 17 May 2026 19:36:35 -0700 Subject: [PATCH 6/7] Fix flaky NOTAM contamination UI test on slower sim configs testNOTAMClearMultipleResetsBadge failed on iPhone 16 Pro / iOS 18.4 ("Failed to determine hittability of Water/Slush Button: Activation point invalid"): selectContamination tapped the picker menu option before the menu finished presenting, so the option's frame was still invalid and both isHittable and the coordinate fallback failed. Add BasePage.waitForHittable(_:timeout:) and wait for the option to be hittable before tapping. Fails with a clear assertion if the menu never presents, rather than a cryptic hittability error. Co-Authored-By: Claude Opus 4.7 (1M context) --- SF50 TOLDUITests/Pages/BasePage.swift | 15 +++++++++++++++ SF50 TOLDUITests/Pages/NOTAMPage.swift | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/SF50 TOLDUITests/Pages/BasePage.swift b/SF50 TOLDUITests/Pages/BasePage.swift index 9f7783b..da4a040 100644 --- a/SF50 TOLDUITests/Pages/BasePage.swift +++ b/SF50 TOLDUITests/Pages/BasePage.swift @@ -120,6 +120,21 @@ class BasePage { return scrollToElement(element)?.label ?? "" } + /// Polls until the element exists and is hittable, then returns whether it is. + /// + /// Use for elements presented behind an animation (menu/picker options), + /// whose frame is briefly invalid before presentation settles — tapping + /// during that window fails with "Activation point invalid". + @discardableResult + func waitForHittable(_ element: XCUIElement, timeout: TimeInterval = 5) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if element.exists, element.isHittable { return true } + Thread.sleep(forTimeInterval: 0.2) + } + return element.exists && element.isHittable + } + func clearAndType(_ element: XCUIElement, _ text: String) { element.clearAndType(text, app: app) } diff --git a/SF50 TOLDUITests/Pages/NOTAMPage.swift b/SF50 TOLDUITests/Pages/NOTAMPage.swift index 7c571fd..bf56120 100644 --- a/SF50 TOLDUITests/Pages/NOTAMPage.swift +++ b/SF50 TOLDUITests/Pages/NOTAMPage.swift @@ -44,6 +44,10 @@ final class NOTAMPage: BasePage { ensureHittable(contaminationTypePicker) forceTap(contaminationTypePicker) let option = app.buttons[type] + XCTAssertTrue( + waitForHittable(option, timeout: 5), + "Contamination option \"\(type)\" should be hittable after opening the picker" + ) forceTap(option) } } From d7fbf162f7c474e09ab14bb785b6d7f1ddbf1056 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sun, 17 May 2026 22:14:34 -0700 Subject: [PATCH 7/7] Verify numeric scenario fields committed; retry dropped keystrokes testScenarioFieldsPersistAfterNavigation failed intermittently on iPad Pro 13 (M5)/iOS latest ("OAT delta should persist, got: +1"): clearAndType occasionally dropped a keystroke on slower configs, so the MeasurementField (a FormatStyle TextField that commits on editing-end) persisted +1 instead of +15. Add a verifying clear/type for the numeric scenario fields: end editing to force the commit, read the field back, and retry if the shown digits don't reflect the input. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Pages/ScenarioDetailPage.swift | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/SF50 TOLDUITests/Pages/ScenarioDetailPage.swift b/SF50 TOLDUITests/Pages/ScenarioDetailPage.swift index 1ebd8b4..60536d2 100644 --- a/SF50 TOLDUITests/Pages/ScenarioDetailPage.swift +++ b/SF50 TOLDUITests/Pages/ScenarioDetailPage.swift @@ -18,12 +18,31 @@ final class ScenarioDetailPage: BasePage { func setOATDelta(_ value: String) { XCTAssertTrue(OATDeltaField.exists, "OAT delta field should exist") - OATDeltaField.clearAndType(value, app: app) + clearTypeAndVerify(OATDeltaField, value) } func setWeightDelta(_ value: String) { XCTAssertTrue(weightDeltaField.exists, "Weight delta field should exist") - weightDeltaField.clearAndType(value, app: app) + clearTypeAndVerify(weightDeltaField, value) + } + + /// Types `value` into a numeric ``MeasurementField`` and verifies the + /// committed value reflects the typed digits, retrying if a keystroke was + /// dropped (intermittent on slower simulator configs). Ends editing first + /// so the `FormatStyle`-backed field commits before it is read back. + private func clearTypeAndVerify(_ field: XCUIElement, _ value: String, retries: Int = 3) { + let digits = value.filter(\.isNumber) + for attempt in 1...retries { + field.clearAndType(value, app: app) + dismissKeyboard() + let shown = (field.value as? String ?? "").filter(\.isNumber) + if shown.contains(digits) { return } + XCTAssertNotEqual( + attempt, + retries, + "Field did not accept \"\(value)\"; shows \"\(field.value as? String ?? "")\"" + ) + } } func goBack() {