diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift index 2d6c08b..e4d966b 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/PrinttargArgs.swift @@ -56,23 +56,19 @@ public enum PrinttargArgs { } args.append(contentsOf: ["-R", "\(config.customSeed)"]) case .raster: - args.append("-r") + args.append(contentsOf: ArgsBuilder.flag("-r", when: true)) } - if let label = config.label?.trimmingCharacters(in: .whitespacesAndNewlines), - !label.isEmpty { - args.append(contentsOf: ["-d", label]) - } + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-d", config.label)) guard (72...600).contains(config.dpi) else { throw PrinttargArgError.invalidDPI(config.dpi) } args.append(contentsOf: [config.bitDepth.flag, "\(config.dpi)"]) - if !CalibrationIdentity.isCalibration(cleanBasename), - let cal = config.calibrationFile?.trimmingCharacters(in: .whitespacesAndNewlines), - !cal.isEmpty { - args.append(contentsOf: [config.calibrationEmbedOnly ? "-I" : "-K", cal]) + if !CalibrationIdentity.isCalibration(cleanBasename) { + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty( + config.calibrationEmbedOnly ? "-I" : "-K", config.calibrationFile)) } args.append(cleanBasename) diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift index b9c14b8..6ad53ad 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/TargenArgs.swift @@ -70,18 +70,12 @@ public enum TargenArgs { if let n = config.neutralSteps, n > 0 { args.append(contentsOf: ["-n", "\(n)"]) } - if let nConc = config.neutralConcentration, abs(nConc - 0.50) >= 0.001 { - args.append(contentsOf: ["-N", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), nConc)]) - } - if let c = config.preconditioningProfile?.trimmingCharacters(in: .whitespacesAndNewlines), !c.isEmpty { - args.append(contentsOf: ["-c", c]) - } - if config.ofpsHighQuality == true { - args.append("-G") - } - if let a = config.ofpsAdaptation { - args.append(contentsOf: ["-A", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), a)]) - } + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-N", config.neutralConcentration, skip: 0.50)) + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-c", config.preconditioningProfile)) + args.append(contentsOf: ArgsBuilder.flag("-G", when: config.ofpsHighQuality == true)) + args.append(contentsOf: ArgsBuilder.option("-A", config.ofpsAdaptation.map { + String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), $0) + })) if let algFlag = config.fullSpreadAlgorithm?.flag { args.append(algFlag) } @@ -91,11 +85,9 @@ public enum TargenArgs { } args.append(contentsOf: ["-l", "\(inkLimit)"]) } - if let v = config.darkEmphasis, abs(v - 1.0) >= 0.001 { - args.append(contentsOf: ["-V", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), v)]) - } - if let p = config.devicePower, p > 0, abs(p - 1.0) >= 0.001 { - args.append(contentsOf: ["-p", String(format: "%.2f", locale: Locale(identifier: "en_US_POSIX"), p)]) + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-V", config.darkEmphasis, skip: 1.0)) + if let p = config.devicePower, p > 0 { + args.append(contentsOf: ArgsBuilder.optionUnlessApprox("-p", p, skip: 1.0)) } args.append(cleanBasename) diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Files/ArtefactProbe.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Files/ArtefactProbe.swift index 9bb9777..0e32fb1 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Files/ArtefactProbe.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Files/ArtefactProbe.swift @@ -79,14 +79,17 @@ public enum ArtefactProbe { } /// Resolve an explicit profile URL, flipping `.icc` ↔ `.icm` when the - /// requested path is missing (#69 / issue #83). + /// requested path is missing (#69 / issue #83). Any other extension + /// (`.mpp`, `.txt`, …) is returned unchanged — never rewritten. public static func resolveProfile( _ url: URL, fileManager: FileManager = .default ) -> URL { if fileManager.fileExists(atPath: url.path) { return url } - let altExt = url.pathExtension.lowercased() == "icc" ? "icm" : "icc" - let alt = url.deletingPathExtension().appendingPathExtension(altExt) + let ext = url.pathExtension.lowercased() + guard ext == "icc" || ext == "icm" else { return url } + let alt = url.deletingPathExtension() + .appendingPathExtension(ext == "icc" ? "icm" : "icc") return fileManager.fileExists(atPath: alt.path) ? alt : url } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Files/JSONFileStore.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Files/JSONFileStore.swift index 3da31f3..d9a5afe 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Files/JSONFileStore.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Files/JSONFileStore.swift @@ -27,10 +27,7 @@ public struct JSONFileStore: Sendable { self.fileURL = fileURL self.corrupt = corrupt self.defaultValue = defaultValue - let encoder = JSONEncoder() - encoder.outputFormatting = [.prettyPrinted, .sortedKeys] - encoder.dateEncodingStrategy = dateEncoding - self.encoder = encoder + self.encoder = JSONEncoder.icceryPretty(dateEncoding: dateEncoding) let decoder = JSONDecoder() decoder.dateDecodingStrategy = dateDecoding self.decoder = decoder @@ -75,10 +72,14 @@ public struct JSONFileStore: Sendable { } extension JSONEncoder { - /// Pretty-printed, sorted-keys encoder used by preset export. - public static func icceryPretty() -> JSONEncoder { + /// Shared pretty-printed, sorted-keys encoder used by `JSONFileStore` + /// and preset export. + static func icceryPretty( + dateEncoding: JSONEncoder.DateEncodingStrategy = .deferredToDate + ) -> JSONEncoder { let encoder = JSONEncoder() encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + encoder.dateEncodingStrategy = dateEncoding return encoder } } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift index a084908..f4ba148 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift @@ -145,9 +145,7 @@ public actor ProcessManager { ) let process = prepared.process - AppLogger(category: "process").debug( - "spawn \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))" - ) + logSpawn(id: id, binary: binary, arguments: arguments, captured: false) children[id] = RunningChild( process: process, @@ -214,9 +212,7 @@ public actor ProcessManager { let stdoutPipe = prepared.stdoutPipe let stderrPipe = prepared.stderrPipe - AppLogger(category: "process").debug( - "spawn(captured) \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))" - ) + logSpawn(id: id, binary: binary, arguments: arguments, captured: true) // Register and set up the termination hand-off before run() so // a very fast exit is never missed (#50, #52). @@ -466,6 +462,18 @@ public actor ProcessManager { ) } + private nonisolated func logSpawn( + id: String, + binary: URL, + arguments: [String], + captured: Bool + ) { + let prefix = captured ? "spawn(captured)" : "spawn" + AppLogger(category: "process").debug( + "\(prefix) \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))" + ) + } + /// `terminationHandler` can lose a fast-exit race on a loaded host; /// `waitUntilExit` on a detached thread is the fallback (#50, #52). /// The handler is attached before `run()`; the wait thread starts diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/CalibrationIdentity.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/CalibrationIdentity.swift index ac24329..afcf06f 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/CalibrationIdentity.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/CalibrationIdentity.swift @@ -33,16 +33,16 @@ public struct CalibrationIdentity: Equatable, Sendable { /// Derive identity from the live wizard basename and the persisted /// original. A non-empty persisted original wins over a `CAL_` live - /// name (Force Quit mid-calibration). + /// name (Force Quit mid-calibration). An empty live basename always + /// produces an empty identity — a persisted original must never + /// resurrect a target that no longer exists (#83). public static func parse(liveBasename: String, persistedOriginal: String) -> CalibrationIdentity { - if liveBasename.isEmpty && persistedOriginal.isEmpty { + guard !liveBasename.isEmpty else { return CalibrationIdentity(originalBasename: "", calibrationBasename: "") } let original: String if liveBasename.hasPrefix("CAL_") { original = persistedOriginal.isEmpty ? strip(liveBasename) : persistedOriginal - } else if liveBasename.isEmpty { - original = persistedOriginal } else { original = liveBasename } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift index 6c8c5fa..8ac938b 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/PrintcalArgs.swift @@ -79,16 +79,9 @@ public enum PrintcalArgs { var args: [String] = ["-v", "-e"] - if config.noInkLimit { - args.append("-I") - } - if config.verify { - args.append("-z") - } - if let previous = config.previousCalPath?.trimmingCharacters(in: .whitespacesAndNewlines), - !previous.isEmpty { - args.append(contentsOf: ["-a", previous]) - } + args.append(contentsOf: ArgsBuilder.flag("-I", when: config.noInkLimit)) + args.append(contentsOf: ArgsBuilder.flag("-z", when: config.verify)) + args.append(contentsOf: ArgsBuilder.optionIfNonEmpty("-a", config.previousCalPath)) if let tac = config.totalInkLimit, tac > 0 { args.append(contentsOf: ["-m", String(format: "%.1f", tac)]) } else if let tac = config.totalInkLimit { diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift index fea54c9..707ee7d 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift @@ -73,7 +73,11 @@ public actor VerificationHistoryStore { } /// Removes all history and updates disk. + /// + /// Loads the existing history first and propagates any load error so an + /// unparseable file is never overwritten. public func clear() throws { + try load() try write([]) records = [] } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Settings/PresetMapping.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Settings/PresetMapping.swift index f7f36dc..f963ea6 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Settings/PresetMapping.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Settings/PresetMapping.swift @@ -14,7 +14,9 @@ extension TargenConfig { neutralSteps: preset.neutralSteps, neutralConcentration: preset.neutralConcentration, preconditioningProfile: preset.preconditioningProfile, - ofpsHighQuality: preset.ofpsHighQuality == true ? true : nil, + // An explicit `false` is preserved — distinguishable from a + // missing key; `-G` is only emitted for `true` (#82). + ofpsHighQuality: preset.ofpsHighQuality, ofpsAdaptation: preset.ofpsAdaptation, fullSpreadAlgorithm: preset.fullSpreadAlgorithm.flatMap { FullSpreadAlgorithm(presetValue: $0) }.flatMap { $0 == .ofps ? nil : $0 }, totalInkLimit: preset.totalInkLimit, @@ -109,6 +111,51 @@ extension ColprofConfig { } } +/// User-facing FWA selection for the Stage 4 form, plus the two +/// directions of `colprof_fwa` conversion centralised here so the view +/// models carry no mapping switches of their own (#82). +public enum ColprofFwaSelection: String, CaseIterable, Sendable, Equatable { + case none = "none" + case empty = "" + case D50 = "D50" + case D65 = "D65" + case custom = "custom" + + public var displayName: String { + switch self { + case .none: return "None" + case .empty: return "Bare (-f)" + case .D50: return "D50" + case .D65: return "D65" + case .custom: return "Custom .sp" + } + } + + /// Preset `colprof_fwa` → selection. `nil`/`"none"` map to `.none`, + /// `""` to `.empty`, `D50`/`D65` case-insensitively, and any other + /// string is a custom `.sp` path. + public init(presetValue: String?) { + switch presetValue?.lowercased() { + case nil, "none": self = .none + case "": self = .empty + case "d50": self = .D50 + case "d65": self = .D65 + default: self = .custom + } + } + + /// Selection → `colprof_fwa` value. `.custom` returns `customPath`. + public func presetValue(customPath: String) -> String? { + switch self { + case .none: return nil + case .empty: return "" + case .D50: return "D50" + case .D65: return "D65" + case .custom: return customPath + } + } +} + extension PageSize { /// `"210x297"` custom page parse used by presets (issue #82). public static func parseCustom(_ raw: String) -> (Double, Double)? { diff --git a/README.md b/README.md index e58a5a5..77aeaba 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,8 @@ All measurement, chart generation, and profile mathematics live in the [Gronod A | Floor | macOS 14 Sonoma, universal `arm64` + `x86_64` | | Default branch | `develop` | | M6 | Stage 0 calibration, CGATS import, SceneKit gamut viewer, packaging — shipped on `develop` | -| M7 | UAT-ready hardening of the v2.0 wizard paths | +| M7 | Pre-UAT hardening & baseline consolidation — shipped on `develop` | +| M8 | Deduplication/consolidation contracts & UAT-ready hardening (#79–#86) — in flight on `milestone/m8-consolidation` | | Licence | Proprietary source in [`LICENCE.md`](LICENCE.md); bundled Argyll sidecars remain AGPLv3 | ## What it does @@ -170,11 +171,11 @@ Agent / branch rules: [`AGENTS.md`](AGENTS.md), [`BUILD-PLAN.md`](BUILD-PLAN.md) ``` develop - └── milestone/mN- # integration only + └── milestone/m8-consolidation # integration branch └── feat/- # one issue per branch ``` -Feature PRs target the current milestone branch, not `develop`. The milestone branch merges to `develop` when its issues are green. M7 is small; its PRs target `develop` directly. Do not open umbrella "bugfix" branches that mix tickets. +Feature PRs target the current milestone branch, not `develop`. The milestone branch merges to `develop` when its issues are green. Completion PRs for issues #79–#86 target `milestone/m8-consolidation`; `milestone/m8-consolidation` merges into `develop` once all milestone gates pass. Do not open umbrella "bugfix" branches that mix tickets. ## Licence diff --git a/Sources/ICCery/CalibrationView.swift b/Sources/ICCery/CalibrationView.swift index a2d25dc..b4eb94c 100644 --- a/Sources/ICCery/CalibrationView.swift +++ b/Sources/ICCery/CalibrationView.swift @@ -95,13 +95,6 @@ struct CalibrationView: View { .frame(minHeight: 80, maxHeight: 120) } } - - if let error = model.lastError { - Section { - Text(error) - .foregroundStyle(.red) - } - } } .formStyle(.grouped) diff --git a/Sources/ICCery/CalibrationViewModel.swift b/Sources/ICCery/CalibrationViewModel.swift index a327019..d19db7f 100644 --- a/Sources/ICCery/CalibrationViewModel.swift +++ b/Sources/ICCery/CalibrationViewModel.swift @@ -25,7 +25,6 @@ final class CalibrationViewModel { var calibrationLog: [String] = [] var isGenerating = false var isComputing = false - var lastError: String? init(workflow: TargetWorkflowViewModel, profile: ProfileWorkflowViewModel, environment: AppEnvironment) { self.workflow = workflow @@ -77,10 +76,6 @@ final class CalibrationViewModel { wizard.basename = identity.calibrationBasename wizard.sessionMode = .calibration - isGenerating = true - calibrationLog = [] - lastError = nil - let config = CalibrationTargenConfig( colourSpace: colourSpace, steps: steps, @@ -92,17 +87,19 @@ final class CalibrationViewModel { ) Task { @MainActor in - defer { self.isGenerating = false } - do { - _ = try await self.environment.runner.runCalibrationTargen(config: config, onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - self?.calibrationLog.append(contentsOf: batch) - }) + _ = try await ProcessRunSupport.runLogged( + setRunning: { self.isGenerating = $0 }, + resetLog: { self.calibrationLog = [] }, + onLog: { self.calibrationLog.append(contentsOf: $0) } + ) { onLog in + try await self.environment.runner.runCalibrationTargen( + config: config, onLogBatch: onLog) + } self.wizard.refreshGating() self.wizard.showNotice("Calibration target generated.") self.wizard.go(to: .layOutPrint) } catch { - self.lastError = error.localizedDescription self.wizard.showNotice( "Calibration target failed: \(error.localizedDescription)", kind: .error @@ -137,15 +134,13 @@ final class CalibrationViewModel { // "already exists" when the user declines overwrite. We do not // silently clobber. if FileManager.default.fileExists(atPath: outputURL.path) { - lastError = "\(outputURL.lastPathComponent) already exists. Rename or overwrite it first." - wizard.showNotice(lastError!, kind: .error) + wizard.showNotice( + "\(outputURL.lastPathComponent) already exists. Rename or overwrite it first.", + kind: .error + ) return } - isComputing = true - calibrationLog = [] - lastError = nil - let config = PrintcalConfig( ti3Basename: calBasename, workingDirectory: cwd, @@ -158,19 +153,21 @@ final class CalibrationViewModel { ) Task { @MainActor in - defer { self.isComputing = false } - do { - let url = try await self.environment.runner.runPrintcal(config: config, onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - self?.calibrationLog.append(contentsOf: batch) - }) + let url = try await ProcessRunSupport.runLogged( + setRunning: { self.isComputing = $0 }, + resetLog: { self.calibrationLog = [] }, + onLog: { self.calibrationLog.append(contentsOf: $0) } + ) { onLog in + try await self.environment.runner.runPrintcal( + config: config, onLogBatch: onLog) + } self.computedCalURL = url self.profile.calibrationFile = url.path self.profile.applyCalibration = self.applyToProfile self.wizard.showNotice("Calibration curves computed.") self.wizard.restoreCalibration() } catch { - self.lastError = error.localizedDescription self.wizard.showNotice( "Calibration curve computation failed: \(error.localizedDescription)", kind: .error diff --git a/Sources/ICCery/MeasurementWorkflowViewModel.swift b/Sources/ICCery/MeasurementWorkflowViewModel.swift index 688ae3a..6ec4bba 100644 --- a/Sources/ICCery/MeasurementWorkflowViewModel.swift +++ b/Sources/ICCery/MeasurementWorkflowViewModel.swift @@ -63,7 +63,8 @@ final class MeasurementWorkflowViewModel { var rows: [ChartreadRow] = [] var swatchRows: [SwatchRow] = [] var showRemoveSheetNotice = false - var lastError: String? + /// Stage-local chartread error notice (`#chartreadLastError`, #80). + var chartreadNotice: Notice? private var chartreadTask: Task? // MARK: - Averaging @@ -185,7 +186,7 @@ final class MeasurementWorkflowViewModel { isChartreadRunning = true chartreadState = .idle currentPrompt = nil - lastError = nil + chartreadNotice = nil chartreadLog.removeAll() // Optional: reset rows when starting a fresh first pass. @@ -227,7 +228,10 @@ final class MeasurementWorkflowViewModel { case .exit(let code): if code != 0 { - lastError = "chartread exited with code \(code)" + chartreadNotice = Notice( + kind: .error, + text: "chartread exited with code \(code)" + ) } case .completed(let canonicalURL): @@ -235,7 +239,7 @@ final class MeasurementWorkflowViewModel { completePass(canonicalURL: canonicalURL) case .failed(let error): - lastError = error.localizedDescription + chartreadNotice = Notice(kind: .error, text: error.localizedDescription) chartreadState = .error isChartreadRunning = false } @@ -381,7 +385,10 @@ final class MeasurementWorkflowViewModel { discoverPassSnapshots() wizard.refreshGating() } catch { - lastError = "Could not snapshot pass: \(error.localizedDescription)" + chartreadNotice = Notice( + kind: .error, + text: "Could not snapshot pass: \(error.localizedDescription)" + ) } } @@ -397,30 +404,32 @@ final class MeasurementWorkflowViewModel { func finishAndAverage() { guard !isFinishing, let cwd = workingDirectory, !passSnapshots.isEmpty else { return } - isFinishing = true finishNotice = nil Task { @MainActor [weak self] in guard let self else { return } do { - let canonical: URL - if self.passSnapshots.count == 1, let pass = self.passSnapshots.first { - canonical = try MeasurementArtefacts.promotePass( - pass: pass, - basename: self.basename, - cwd: cwd - ) - } else { + // No log reset: prior chartread output must be preserved. + let canonical = try await ProcessRunSupport.runLogged( + setRunning: { self.isFinishing = $0 }, + resetLog: {}, + onLog: { self.chartreadLog.append(contentsOf: $0) } + ) { onLog in + if self.passSnapshots.count == 1, let pass = self.passSnapshots.first { + return try MeasurementArtefacts.promotePass( + pass: pass, + basename: self.basename, + cwd: cwd + ) + } let config = AverageConfig( workingDirectory: cwd, basename: self.basename, passFiles: self.passSnapshots ) - canonical = try await self.environment.runner.runAverage( + return try await self.environment.runner.runAverage( config: config, - onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - self?.chartreadLog.append(contentsOf: batch) - } + onLogBatch: onLog ) } self.discoverPassSnapshots() @@ -465,7 +474,6 @@ final class MeasurementWorkflowViewModel { ) } } - self.isFinishing = false } } } diff --git a/Sources/ICCery/NoticeBanner.swift b/Sources/ICCery/NoticeBanner.swift index 554c222..a5c7454 100644 --- a/Sources/ICCery/NoticeBanner.swift +++ b/Sources/ICCery/NoticeBanner.swift @@ -21,6 +21,14 @@ struct Notice: Identifiable, Equatable { case .error: return .red } } + + var accessibilityValue: String { + switch self { + case .info: return "info" + case .warning: return "warning" + case .error: return "error" + } + } } let id = UUID() diff --git a/Sources/ICCery/ProfileWorkflowViewModel.swift b/Sources/ICCery/ProfileWorkflowViewModel.swift index 589ed21..8426e61 100644 --- a/Sources/ICCery/ProfileWorkflowViewModel.swift +++ b/Sources/ICCery/ProfileWorkflowViewModel.swift @@ -3,25 +3,6 @@ import Observation import SwiftUI import ICCeryCore -/// User-facing FWA selection for the Stage 4 form. -enum ColprofFwaSelection: String, CaseIterable, Sendable, Equatable { - case none = "none" - case empty = "" - case D50 = "D50" - case D65 = "D65" - case custom = "custom" - - var displayName: String { - switch self { - case .none: return "None" - case .empty: return "Bare (-f)" - case .D50: return "D50" - case .D65: return "D65" - case .custom: return "Custom .sp" - } - } -} - /// Stage 4/5 workflow: build a profile, verify it, track drift, and install. @MainActor @Observable @@ -50,7 +31,6 @@ final class ProfileWorkflowViewModel { var isColprofRunning = false var colprofLog: [String] = [] var colprofProgress: String? - var lastError: String? var createdProfileURL: URL? /// Path to the `.gam` gamut mesh extracted post-`colprof` (issue #28). var createdGamutURL: URL? @@ -110,13 +90,7 @@ final class ProfileWorkflowViewModel { } var fwaValue: String? { - switch fwaSelection { - case .none: return nil - case .empty: return "" - case .D50: return "D50" - case .D65: return "D65" - case .custom: return fwaCustomPath - } + fwaSelection.presetValue(customPath: fwaCustomPath) } // MARK: - Preset application @@ -131,21 +105,16 @@ final class ProfileWorkflowViewModel { algorithm = config.algorithm quality = config.quality intent = config.intent ?? "" - if let fwa = config.fwa { - switch fwa.lowercased() { - case "none": fwaSelection = .none - case "": fwaSelection = .empty - case "d50": fwaSelection = .D50 - case "d65": fwaSelection = .D65 - default: - fwaSelection = .custom - fwaCustomPath = fwa - } - } + fwaSelection = ColprofFwaSelection(presetValue: config.fwa) + fwaCustomPath = fwaSelection == .custom ? (config.fwa ?? "") : "" illuminant = config.illuminant ?? "" observer = config.observer ?? "" inputViewingCond = config.inputViewingCond ?? "" outputViewingCond = config.outputViewingCond ?? "" + profileDescription = "" + copyright = "" + applyCalibration = preset.applyCalibration == true + calibrationFile = preset.calibrationFile ?? "" } /// Stage 4 form values for saving into a custom preset. @@ -195,60 +164,59 @@ final class ProfileWorkflowViewModel { guard canCreateProfile, let _ = wizard.effectiveWorkingDirectory else { return } let config = buildColprofConfig() - isColprofRunning = true - colprofLog = [] colprofProgress = nil - lastError = nil createdProfileURL = nil createdGamutURL = nil let runner = environment.runner Task { @MainActor [weak self] in guard let self else { return } - defer { self.isColprofRunning = false } - do { - let url = try await runner.runColprof(config: config, onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - guard let self else { return } - self.colprofLog.append(contentsOf: batch) - if let last = batch.last { - self.updateProgress(ColprofProgressClassifier.classify(line: last)) + let outcome = try await ProcessRunSupport.runLogged( + setRunning: { self.isColprofRunning = $0 }, + resetLog: { self.colprofLog = [] }, + onLog: { batch in + self.colprofLog.append(contentsOf: batch) + if let last = batch.last { + self.updateProgress(ColprofProgressClassifier.classify(line: last)) + } } - }) + ) { onLog in + let url = try await runner.runColprof(config: config, onLogBatch: onLog) - var finalProfileURL = url + var finalProfileURL = url - if self.applyCalibration, !self.calibrationFile.isEmpty { - let applyConfig = ApplycalConfig( - calibrationPath: self.calibrationFile, - inputProfileURL: url - ) - assert(!applyConfig.unapply, "applycal unapply is not supported in v2.0") - finalProfileURL = try await runner.runApplycal(config: applyConfig) - self.colprofLog.append("Calibration embedded: \(self.calibrationFile)") + if self.applyCalibration, !self.calibrationFile.isEmpty { + let applyConfig = ApplycalConfig( + calibrationPath: self.calibrationFile, + inputProfileURL: url + ) + assert(!applyConfig.unapply, "applycal unapply is not supported in v2.0") + finalProfileURL = try await runner.runApplycal(config: applyConfig) + self.colprofLog.append("Calibration embedded: \(self.calibrationFile)") + } + + // Gamut extraction is best-effort for Stage 5 / M6 viewer. + var gamutURL: URL? + do { + let gamConfig = IccgamutConfig(profileURL: finalProfileURL) + let url = try await runner.runIccgamut(config: gamConfig, onLogBatch: onLog) + gamutURL = url + self.colprofLog.append("Gamut mesh extracted: \(url.lastPathComponent)") + } catch { + self.wizard.showNotice( + "Gamut extraction skipped: \(error.localizedDescription)", + kind: .info + ) + } + return (profileURL: finalProfileURL, gamutURL: gamutURL) } - - // Gamut extraction is best-effort for Stage 5 / M6 viewer. - do { - let gamConfig = IccgamutConfig(profileURL: finalProfileURL) - let gamURL = try await runner.runIccgamut(config: gamConfig, onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - self?.colprofLog.append(contentsOf: batch) - }) - self.createdGamutURL = gamURL - self.colprofLog.append("Gamut mesh extracted: \(gamURL.lastPathComponent)") - } catch { - self.wizard.showNotice( - "Gamut extraction skipped: \(error.localizedDescription)", - kind: .info - ) - } - - self.createdProfileURL = finalProfileURL + self.createdProfileURL = outcome.profileURL + self.createdGamutURL = outcome.gamutURL self.wizard.refreshGating() - self.wizard.showNotice("Profile created: \(finalProfileURL.lastPathComponent)") + self.wizard.showNotice("Profile created: \(outcome.profileURL.lastPathComponent)") self.wizard.go(to: .verifyInstall) } catch { - self.lastError = error.localizedDescription self.wizard.showNotice( "Profile creation failed: \(error.localizedDescription)", kind: .error @@ -315,23 +283,28 @@ final class ProfileWorkflowViewModel { let ti3URL = ArtefactProbe.artefact(wizard.basename, "ti3", cwd) let config = ProfcheckConfig(ti3URL: ti3URL, iccURL: profileURL) - isProfcheckRunning = true profcheckReport = nil profcheckWarning = nil let runner = environment.runner Task { @MainActor [weak self] in guard let self else { return } - defer { self.isProfcheckRunning = false } - do { - let report = try await runner.runProfcheck(config: config, onLogBatch: ProcessRunSupport.logSink { [weak self] batch in - self?.colprofLog.append(contentsOf: batch) - }) - self.profcheckReport = report - if let record = self.makeVerificationRecord(from: report) { - let updated = try await self.environment.historyStore.append(record) - self.verificationHistory = updated + let outcome = try await ProcessRunSupport.runLogged( + setRunning: { self.isProfcheckRunning = $0 }, + resetLog: {}, + onLog: { self.colprofLog.append(contentsOf: $0) } + ) { onLog in + let report = try await runner.runProfcheck(config: config, onLogBatch: onLog) + var history: [VerificationRecord]? + if let record = self.makeVerificationRecord(from: report) { + history = try await self.environment.historyStore.append(record) + } + return (report: report, history: history) + } + self.profcheckReport = outcome.report + if let history = outcome.history { + self.verificationHistory = history self.driftAlert = DriftAlert.compute(from: self.filteredHistory) } } catch let error as ArgyllRunnerError where error == .profcheckUnparseable { diff --git a/Sources/ICCery/Stage2View.swift b/Sources/ICCery/Stage2View.swift index d201198..9d2c784 100644 --- a/Sources/ICCery/Stage2View.swift +++ b/Sources/ICCery/Stage2View.swift @@ -218,6 +218,7 @@ struct Stage2View: View { .foregroundStyle(notice.kind == .error ? .red : .blue) .accessibilityIdentifier("printNotificationIcon") + .accessibilityValue(notice.kind.accessibilityValue) Text(notice.text) .font(.caption) .foregroundStyle(notice.kind == .error diff --git a/Sources/ICCery/Stage3View.swift b/Sources/ICCery/Stage3View.swift index 1c9aa2c..4ee7a65 100644 --- a/Sources/ICCery/Stage3View.swift +++ b/Sources/ICCery/Stage3View.swift @@ -164,28 +164,22 @@ struct Stage3View: View { .foregroundStyle(Theme.accent) } - if let lastError = model.lastError { - Text(lastError) + if let notice = model.chartreadNotice { + Text(notice.text) .font(.caption) - .foregroundStyle(.red) + .foregroundStyle(notice.kind.tint) .accessibilityIdentifier("chartreadLastError") - .accessibilityValue(lastError) + .accessibilityValue(notice.text) } controlButtons if !model.chartreadLog.isEmpty { - DisclosureGroup("Log") { - VStack(alignment: .leading) { - ForEach(model.chartreadLog, id: \.self) { line in - Text(line) - .font(.system(.caption, design: .monospaced)) - .foregroundStyle(.secondary) - } - } - } - .foregroundStyle(Theme.text) - .accessibilityIdentifier("chartreadLogContainer") + ProcessLogView( + lines: model.chartreadLog, + containerId: "chartreadLogContainer", + logId: "chartreadLog" + ) } } .padding(16) @@ -374,6 +368,8 @@ struct Stage3View: View { Text(notice.text) .font(.caption) .foregroundStyle(notice.kind == .error ? .red : .green) + .accessibilityIdentifier("chartreadFinishNotice") + .accessibilityValue(notice.kind.accessibilityValue) } } .padding(16) diff --git a/Sources/ICCery/Stage4View.swift b/Sources/ICCery/Stage4View.swift index e879abb..21333a6 100644 --- a/Sources/ICCery/Stage4View.swift +++ b/Sources/ICCery/Stage4View.swift @@ -166,27 +166,14 @@ struct Stage4View: View { } Spacer() - - if let lastError = model.lastError { - Text(lastError) - .font(.caption) - .foregroundStyle(.red) - .accessibilityIdentifier("colprofLastError") - } } if !model.colprofLog.isEmpty { - DisclosureGroup("Log") { - VStack(alignment: .leading) { - ForEach(model.colprofLog, id: \.self) { line in - Text(line) - .font(.system(.caption, design: .monospaced)) - .foregroundStyle(.secondary) - } - } - } - .foregroundStyle(Theme.text) - .accessibilityIdentifier("colprofLogContainer") + ProcessLogView( + lines: model.colprofLog, + containerId: "colprofLogContainer", + logId: "colprofLog" + ) } } .padding(16) diff --git a/Sources/ICCery/TargetWorkflowViewModel.swift b/Sources/ICCery/TargetWorkflowViewModel.swift index 52159d2..3afa2e7 100644 --- a/Sources/ICCery/TargetWorkflowViewModel.swift +++ b/Sources/ICCery/TargetWorkflowViewModel.swift @@ -206,8 +206,6 @@ final class TargetWorkflowViewModel { func generateTarget() { guard canGenerate, !targenRunning else { return } let config = buildTargenConfig() - targenRunning = true - targenLog = [] resumedFromTi2 = false let runner = environment.runner Task { @MainActor in @@ -228,7 +226,6 @@ final class TargetWorkflowViewModel { } catch { wizard.showNotice( "targen failed: \(error.localizedDescription)", kind: .error) - targenRunning = false } } } @@ -242,7 +239,12 @@ final class TargetWorkflowViewModel { ? UITestHooks.datasetImportURL : fileDialogs.selectDatasetFile() guard let url else { return } + importMeasurementDataset(from: url) + } + /// Test seam (issue #80): unit tests pass missing or malformed URLs + /// directly instead of mutating the global environment. + func importMeasurementDataset(from url: URL) { do { let dataset = try CGATSParser.parse(url: url) guard let directory = targetDirectory ?? wizard.effectiveWorkingDirectory else { @@ -339,8 +341,6 @@ final class TargetWorkflowViewModel { func createLayout() { guard wizard.isUnlocked(.layOutPrint), !printtargRunning else { return } let config = buildPrinttargConfig() - printtargRunning = true - printtargLog = [] printtargResult = nil let runner = environment.runner Task { @MainActor in @@ -380,14 +380,18 @@ final class TargetWorkflowViewModel { func applyPreset(_ preset: ProfilingPreset) { let targen = TargenConfig(preset: preset, basename: targetBasename, workingDirectory: targetDirectory) applyTargenForm(targen) + // Stage 4 state (incl. calibration) is applied before Stage 2 so + // the layout config receives the preset's calibration path, not + // stale live state (#82). + profile.applyPreset(preset) let printtarg = PrinttargConfig( preset: preset, basename: wizard.basename, workingDirectory: wizard.effectiveWorkingDirectory, - calibrationFile: profile.applyCalibration ? profile.calibrationFile : nil + calibrationFile: profile.applyCalibration && !profile.calibrationFile.isEmpty + ? profile.calibrationFile : nil ) applyPrinttargForm(printtarg) - profile.applyPreset(preset) selectedPresetID = preset.id } diff --git a/Tests/ICCeryCoreTests/ArgsBuilderTests.swift b/Tests/ICCeryCoreTests/ArgsBuilderTests.swift new file mode 100644 index 0000000..51e9605 --- /dev/null +++ b/Tests/ICCeryCoreTests/ArgsBuilderTests.swift @@ -0,0 +1,94 @@ +import Testing +import Foundation +@testable import ICCeryCore + +@Suite("ArgsBuilder") +struct ArgsBuilderTests { + + // MARK: - option + + @Test("option: nil emits nothing") + func optionNil() { + #expect(ArgsBuilder.option("-f", nil) == []) + } + + @Test("option: present value emits flag and value verbatim") + func optionPresent() { + #expect(ArgsBuilder.option("-f", "abc") == ["-f", "abc"]) + #expect(ArgsBuilder.option("-f", "") == ["-f", ""]) + #expect(ArgsBuilder.option("-f", " padded ") == ["-f", " padded "]) + } + + // MARK: - optionIfNonEmpty + + @Test("optionIfNonEmpty: nil and empty emit nothing") + func optionIfNonEmptyNilEmpty() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", nil) == []) + #expect(ArgsBuilder.optionIfNonEmpty("-d", "") == []) + } + + @Test("optionIfNonEmpty: whitespace-only emits nothing") + func optionIfNonEmptyWhitespace() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", " ") == []) + #expect(ArgsBuilder.optionIfNonEmpty("-d", " \t\n ") == []) + } + + @Test("optionIfNonEmpty: trims surrounding whitespace") + func optionIfNonEmptyTrims() { + #expect(ArgsBuilder.optionIfNonEmpty("-d", " label ") == ["-d", "label"]) + #expect(ArgsBuilder.optionIfNonEmpty("-d", "\tcal.cal\n") == ["-d", "cal.cal"]) + } + + // MARK: - optionUnlessApprox + + @Test("optionUnlessApprox: nil emits nothing") + func optionUnlessApproxNil() { + #expect(ArgsBuilder.optionUnlessApprox("-N", nil, skip: 0.50) == []) + } + + @Test("optionUnlessApprox: exact skip value emits nothing") + func optionUnlessApproxExactSkip() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.50, skip: 0.50) == []) + #expect(ArgsBuilder.optionUnlessApprox("-V", 1.0, skip: 1.0) == []) + } + + @Test("optionUnlessApprox: within epsilon emits nothing") + func optionUnlessApproxWithinEpsilon() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.5005, skip: 0.50) == []) + #expect(ArgsBuilder.optionUnlessApprox("-V", 0.9995, skip: 1.0) == []) + } + + @Test("optionUnlessApprox: outside epsilon emits flag") + func optionUnlessApproxOutsideEpsilon() { + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.75, skip: 0.50) == ["-N", "0.75"]) + #expect(ArgsBuilder.optionUnlessApprox("-V", 1.50, skip: 1.0) == ["-V", "1.50"]) + #expect(ArgsBuilder.optionUnlessApprox("-N", 0.498, skip: 0.50) == ["-N", "0.50"]) + } + + @Test("optionUnlessApprox: POSIX formatting is locale-stable") + func optionUnlessApproxPOSIX() { + // 1234.5 must never produce a grouping separator or comma decimal. + #expect(ArgsBuilder.optionUnlessApprox("-p", 1234.5, skip: 1.0) == ["-p", "1234.50"]) + #expect(ArgsBuilder.optionUnlessApprox("-p", 2.0, skip: 1.0) == ["-p", "2.00"]) + } + + @Test("optionUnlessApprox: custom epsilon and format honoured") + func optionUnlessApproxCustom() { + #expect(ArgsBuilder.optionUnlessApprox("-x", 1.005, skip: 1.0, epsilon: 0.01) == []) + #expect(ArgsBuilder.optionUnlessApprox("-x", 1.5, skip: 1.0, format: "%.1f") == ["-x", "1.5"]) + } + + // MARK: - flag + + @Test("flag: true emits the bare flag") + func flagTrue() { + #expect(ArgsBuilder.flag("-G", when: true) == ["-G"]) + #expect(ArgsBuilder.flag("-r", when: true) == ["-r"]) + } + + @Test("flag: false emits nothing") + func flagFalse() { + #expect(ArgsBuilder.flag("-G", when: false) == []) + #expect(ArgsBuilder.flag("-r", when: false) == []) + } +} diff --git a/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift index abe5696..204fdc0 100644 --- a/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift +++ b/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift @@ -5,13 +5,13 @@ import Testing @Suite("ArgyllRunner Calibration") struct ArgyllRunnerCalibrationTests { - private func makeRunner() -> ArgyllRunner { + private func makeRunner(processManager: ProcessManager = ProcessManager()) -> ArgyllRunner { let binDir = URL(fileURLWithPath: #filePath) .deletingLastPathComponent() .deletingLastPathComponent() .appendingPathComponent("ICCeryUITests/Fixtures/bin") return ArgyllRunner( - processManager: .shared, + processManager: processManager, binaryResolver: BinaryResolver(overrideDir: binDir) ) } @@ -41,6 +41,35 @@ struct ArgyllRunnerCalibrationTests { try? FileManager.default.removeItem(at: testRoot) } + @Test("Calibration targen from foo runs as process id targen_CAL_foo") + func calibrationTargenProcessId() async throws { + let testRoot = try makeTestDir() + let pm = ProcessManager() + let runner = makeRunner(processManager: pm) + let events = pm.events() + // Subscribed before spawn; the exit event is emitted before + // runCalibrationTargen returns, so this always terminates. + let sawExit = Task { + for await event in events { + guard event.id == "targen_CAL_foo" else { continue } + if case .exit = event { return true } + } + return false + } + let config = CalibrationTargenConfig( + colourSpace: .rgb, + steps: 21, + basename: "foo", + workingDirectory: testRoot + ) + + let url = try await runner.runCalibrationTargen(config: config) + + #expect(url.lastPathComponent == "CAL_foo.ti1") + #expect(await sawExit.value) + try? FileManager.default.removeItem(at: testRoot) + } + @Test("printcal captured run creates .cal") func printcalProducesCal() async throws { let testRoot = try makeTestDir() @@ -59,10 +88,28 @@ struct ArgyllRunnerCalibrationTests { try? FileManager.default.removeItem(at: testRoot) } - @Test("printcal failure throws printcalFailed") + @Test("printcal failure throws toolFailed") func printcalFailureThrows() async throws { let testRoot = try makeTestDir() - let runner = makeRunner() + defer { try? FileManager.default.removeItem(at: testRoot) } + + // Per-test mock printcal that always fails — no global + // environment mutation, no shared fixture changes. + let binDir = try makeTestDir() + defer { try? FileManager.default.removeItem(at: binDir) } + let mockURL = binDir.appendingPathComponent("printcal") + try """ + #!/bin/sh + echo "printcal mock failure" >&2 + exit 1 + """.write(to: mockURL, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: mockURL.path) + + let runner = ArgyllRunner( + processManager: ProcessManager(), + binaryResolver: BinaryResolver(bundledRoot: binDir, overrideDir: binDir) + ) let output = testRoot.appendingPathComponent("CAL_demo.cal") let config = PrintcalConfig( ti3Basename: "CAL_demo", @@ -70,12 +117,9 @@ struct ArgyllRunnerCalibrationTests { outputURL: output ) - setenv("ICCERY_MOCK_PRINTCAL_EXIT", "1", 1) - defer { unsetenv("ICCERY_MOCK_PRINTCAL_EXIT") } - - await #expect(throws: (any Error).self) { + await #expect(throws: ArgyllRunnerError.toolFailed( + tool: "printcal", code: 1, logs: ["printcal mock failure\n"])) { _ = try await runner.runPrintcal(config: config) } - try? FileManager.default.removeItem(at: testRoot) } } diff --git a/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift index ce92e47..6cf0d4e 100644 --- a/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift +++ b/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift @@ -33,7 +33,7 @@ struct ArgyllRunnerColprofTests { try FileManager.default.createDirectory(at: testRoot, withIntermediateDirectories: true) let runner = ArgyllRunner( - processManager: .shared, + processManager: ProcessManager(), binaryResolver: BinaryResolver(overrideDir: binDir) ) @@ -49,4 +49,32 @@ struct ArgyllRunnerColprofTests { try? FileManager.default.removeItem(at: testRoot) } + + @Test("Failing colprof throws toolFailed with code and logs") + func colprofFailureThrowsToolFailed() async throws { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("colprof-fail-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + let mockURL = dir.appendingPathComponent("colprof") + try """ + #!/bin/sh + echo "colprof broke" >&2 + exit 4 + """.write(to: mockURL, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: mockURL.path) + + let runner = ArgyllRunner( + processManager: ProcessManager(), + binaryResolver: BinaryResolver(bundledRoot: dir, overrideDir: dir) + ) + let config = ColprofConfig(basename: "failrun", workingDirectory: dir) + + await #expect(throws: ArgyllRunnerError.toolFailed( + tool: "colprof", code: 4, logs: ["colprof broke"])) { + try await runner.runColprof(config: config) + } + } } diff --git a/Tests/ICCeryCoreTests/ArgyllRunnerStreamingLoopTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerStreamingLoopTests.swift new file mode 100644 index 0000000..c1d114b --- /dev/null +++ b/Tests/ICCeryCoreTests/ArgyllRunnerStreamingLoopTests.swift @@ -0,0 +1,161 @@ +import Foundation +import Testing +@testable import ICCeryCore + +/// Focused contracts for the shared `runStreamingTool` loop (#79). +/// +/// Every test uses a per-test temporary directory, unique basenames, +/// and a fresh `ProcessManager` — no shared UI fixture scripts and no +/// process-environment mutation. +@Suite("ArgyllRunner streaming loop contracts") +struct ArgyllRunnerStreamingLoopTests { + + private func makeTempDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("runner-loop-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + return dir + } + + private func writeMock(_ name: String, _ body: String, in dir: URL) throws { + let url = dir.appendingPathComponent(name) + try body.write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: url.path) + } + + private func makeRunner(binDir: URL) -> ArgyllRunner { + ArgyllRunner( + processManager: ProcessManager(), + binaryResolver: BinaryResolver(bundledRoot: binDir, overrideDir: binDir)) + } + + @Test("Non-zero exit throws toolFailed retaining code and collected stdout/stderr lines") + func nonZeroExitThrowsToolFailed() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + try writeMock("targen", """ + #!/bin/sh + echo "Generating patches..." + echo "targen: too few patches" >&2 + exit 3 + """, in: dir) + let runner = makeRunner(binDir: dir) + let config = TargenConfig( + colourSpace: .rgb, patchCount: 800, whitePatches: 4, + blackPatches: 4, basename: "fail", workingDirectory: dir) + + do { + _ = try await runner.runTargen(config: config) + Issue.record("Expected toolFailed") + } catch let error as ArgyllRunnerError { + guard case .toolFailed(let tool, let code, let logs) = error else { + Issue.record("Expected toolFailed, got \(error)") + return + } + #expect(tool == "targen") + #expect(code == 3) + #expect(logs.contains("Generating patches...")) + #expect(logs.contains("targen: too few patches")) + } + } + + @Test("Exit 0 without expected artefact throws missingArtefact with the artefact path") + func zeroExitMissingArtefact() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + try writeMock("targen", """ + #!/bin/sh + echo "done but wrote nothing" + exit 0 + """, in: dir) + let runner = makeRunner(binDir: dir) + let expectedPath = dir.appendingPathComponent("gone.ti1").path + let config = TargenConfig( + colourSpace: .rgb, patchCount: 800, whitePatches: 4, + blackPatches: 4, basename: "gone", workingDirectory: dir) + + await #expect(throws: ArgyllRunnerError.missingArtefact(expectedPath)) { + try await runner.runTargen(config: config) + } + } + + @Test("Immediate exit after one stdout line still delivers the line and succeeds") + func immediateExitDeliversLine() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + try writeMock("targen", """ + #!/bin/sh + last="" + for arg in "$@"; do last="$arg"; done + echo "only line" + touch "$last.ti1" + exit 0 + """, in: dir) + let runner = makeRunner(binDir: dir) + let config = TargenConfig( + colourSpace: .rgb, patchCount: 800, whitePatches: 4, + blackPatches: 4, basename: "quick", workingDirectory: dir) + + let holder = LogHolder() + let url = try await runner.runTargen(config: config) { batch in + holder.append(batch) + } + #expect(url.lastPathComponent == "quick.ti1") + #expect(FileManager.default.fileExists(atPath: url.path)) + #expect(holder.lines.contains("only line")) + } + + @Test("colprof unterminated progress fragment reaches onLogBatch before exit") + func colprofPartialLineFlush() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + // The fragment is printed without a newline, then the mock sleeps + // past the 500 ms partial-line flush interval before writing the + // artefact and exiting — so the tail is delivered mid-run. + try writeMock("colprof", """ + #!/bin/sh + last="" + for arg in "$@"; do last="$arg"; done + printf 'Doing gamut mapping' + sleep 2 + touch "$last.icc" + exit 0 + """, in: dir) + let runner = makeRunner(binDir: dir) + let config = ColprofConfig(basename: "frag", workingDirectory: dir) + + let holder = LogHolder() + let url = try await runner.runColprof(config: config) { batch in + holder.append(batch) + } + #expect(url.lastPathComponent == "frag.icc") + #expect(FileManager.default.fileExists(atPath: url.path)) + #expect(holder.lines.contains("Doing gamut mapping")) + } + + @Test("toolFailed maps each tool to its user-facing description", + arguments: [ + (tool: "chartread", expected: "Chartread failed: boom"), + (tool: "average", expected: "Averaging failed: boom"), + (tool: "colprof", expected: "Profile creation failed: boom"), + (tool: "printcal", expected: "Calibration curve computation failed: boom"), + (tool: "applycal", expected: "Apply calibration failed: boom"), + (tool: "iccgamut", expected: "Gamut extraction failed: boom"), + (tool: "profcheck", expected: "Profile verification failed: boom"), + ]) + func toolDescriptions(tool: String, expected: String) { + let error = ArgyllRunnerError.toolFailed(tool: tool, code: 1, logs: ["boom"]) + #expect(error.errorDescription == expected) + } + + @Test("toolFailed falls back to a generic description for unmapped tools and empty logs") + func genericFallbacks() { + let unknown = ArgyllRunnerError.toolFailed(tool: "targen", code: 7, logs: ["boom"]) + #expect(unknown.errorDescription == "Process exited with code 7") + + let emptyLogs = ArgyllRunnerError.toolFailed(tool: "colprof", code: 2, logs: []) + #expect(emptyLogs.errorDescription + == "Profile creation failed: exited with code 2") + } +} diff --git a/Tests/ICCeryCoreTests/ArtefactFilesTests.swift b/Tests/ICCeryCoreTests/ArtefactFilesTests.swift index 439dfce..aa85368 100644 --- a/Tests/ICCeryCoreTests/ArtefactFilesTests.swift +++ b/Tests/ICCeryCoreTests/ArtefactFilesTests.swift @@ -125,26 +125,102 @@ struct ArtefactFilesTests { @Suite("ArtefactProbe profile resolve") struct ArtefactProbeProfileTests { - @Test("basename probe prefers .icm") - func icmWins() throws { + private func makeDir() throws -> URL { let dir = FileManager.default.temporaryDirectory .appendingPathComponent("probe-\(UUID().uuidString)") try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + return dir + } + + // MARK: Basename probe matrix (#69) + + @Test("basename probe: only .icc exists") + func onlyIcc() throws { + let dir = try makeDir() + let icc = dir.appendingPathComponent("job.icc") + try Data("icc".utf8).write(to: icc) + #expect(ArtefactProbe.resolveProfile(basename: "job", cwd: dir)?.path == icc.path) + } + + @Test("basename probe: only .icm exists") + func onlyIcm() throws { + let dir = try makeDir() + let icm = dir.appendingPathComponent("job.icm") + try Data("icm".utf8).write(to: icm) + #expect(ArtefactProbe.resolveProfile(basename: "job", cwd: dir)?.path == icm.path) + } + + @Test("basename probe prefers .icm") + func icmWins() throws { + let dir = try makeDir() try Data("icc".utf8).write(to: dir.appendingPathComponent("job.icc")) - try Data("icm".utf8).write(to: dir.appendingPathComponent("job.icm")) + let icm = dir.appendingPathComponent("job.icm") + try Data("icm".utf8).write(to: icm) let url = ArtefactProbe.resolveProfile(basename: "job", cwd: dir) - #expect(url?.pathExtension == "icm") + #expect(url?.path == icm.path) + } + + @Test("basename probe: neither exists returns nil") + func neitherExists() throws { + let dir = try makeDir() + #expect(ArtefactProbe.resolveProfile(basename: "job", cwd: dir) == nil) + } + + // MARK: Explicit URL matrix (#69 / #83) + + @Test("explicit existing .icc wins even when .icm exists") + func explicitIccWins() throws { + let dir = try makeDir() + let icc = dir.appendingPathComponent("job.icc") + try Data("icc".utf8).write(to: icc) + try Data("icm".utf8).write(to: dir.appendingPathComponent("job.icm")) + #expect(ArtefactProbe.resolveProfile(icc).path == icc.path) + } + + @Test("explicit existing .icm wins even when .icc exists") + func explicitIcmWins() throws { + let dir = try makeDir() + try Data("icc".utf8).write(to: dir.appendingPathComponent("job.icc")) + let icm = dir.appendingPathComponent("job.icm") + try Data("icm".utf8).write(to: icm) + #expect(ArtefactProbe.resolveProfile(icm).path == icm.path) } @Test("explicit missing .icc flips to sibling .icm") func flipExtension() throws { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("probe-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let dir = try makeDir() let icc = dir.appendingPathComponent("job.icc") let icm = dir.appendingPathComponent("job.icm") try Data("icm".utf8).write(to: icm) let resolved = ArtefactProbe.resolveProfile(icc) #expect(resolved.path == icm.path) } + + @Test("explicit missing .icm flips to sibling .icc") + func flipToIcc() throws { + let dir = try makeDir() + let icc = dir.appendingPathComponent("job.icc") + let icm = dir.appendingPathComponent("job.icm") + try Data("icc".utf8).write(to: icc) + #expect(ArtefactProbe.resolveProfile(icm).path == icc.path) + } + + @Test("explicit missing both returns the original URL") + func missingBoth() throws { + let dir = try makeDir() + let icc = dir.appendingPathComponent("job.icc") + #expect(ArtefactProbe.resolveProfile(icc).path == icc.path) + } + + @Test("unrelated extension is never rewritten") + func unrelatedExtension() throws { + let dir = try makeDir() + let mpp = dir.appendingPathComponent("job.mpp") + let icc = dir.appendingPathComponent("job.icc") + try Data("icc".utf8).write(to: icc) + // Even though a sibling .icc exists, a missing .mpp stays .mpp. + #expect(ArtefactProbe.resolveProfile(mpp).path == mpp.path) + let txt = dir.appendingPathComponent("job.txt") + #expect(ArtefactProbe.resolveProfile(txt).path == txt.path) + } } diff --git a/Tests/ICCeryCoreTests/CalibrationIdentityTests.swift b/Tests/ICCeryCoreTests/CalibrationIdentityTests.swift new file mode 100644 index 0000000..4422e87 --- /dev/null +++ b/Tests/ICCeryCoreTests/CalibrationIdentityTests.swift @@ -0,0 +1,78 @@ +import Foundation +import Testing +@testable import ICCeryCore + +/// Issue #83 — canonical `CAL_` / original-stem pairing. +@Suite("CalibrationIdentity") +struct CalibrationIdentityTests { + @Test("live foo, no persisted") + func livePlain() { + let id = CalibrationIdentity.parse(liveBasename: "foo", persistedOriginal: "") + #expect(id.originalBasename == "foo") + #expect(id.calibrationBasename == "CAL_foo") + } + + @Test("live foo ignores stale persisted") + func livePlainIgnoresPersisted() { + let id = CalibrationIdentity.parse(liveBasename: "foo", persistedOriginal: "bar") + #expect(id.originalBasename == "foo") + #expect(id.calibrationBasename == "CAL_foo") + } + + @Test("live CAL_foo, persisted foo") + func liveCalPersisted() { + let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "foo") + #expect(id.originalBasename == "foo") + #expect(id.calibrationBasename == "CAL_foo") + } + + @Test("live CAL_foo, empty persisted strips prefix") + func liveCalNoPersist() { + let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "") + #expect(id.originalBasename == "foo") + #expect(id.calibrationBasename == "CAL_foo") + } + + @Test("persisted original wins over CAL_ live") + func persistedWins() { + let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "bar") + #expect(id.originalBasename == "bar") + #expect(id.calibrationBasename == "CAL_bar") + } + + @Test("empty live yields empty identity even with persisted original") + func emptyLiveWithPersisted() { + let id = CalibrationIdentity.parse(liveBasename: "", persistedOriginal: "foo") + #expect(id.originalBasename.isEmpty) + #expect(id.calibrationBasename.isEmpty) + } + + @Test("empty live, empty persisted") + func emptyLive() { + let id = CalibrationIdentity.parse(liveBasename: "", persistedOriginal: "") + #expect(id.originalBasename.isEmpty) + #expect(id.calibrationBasename.isEmpty) + } + + @Test("prefix is idempotent on already-prefixed input") + func alreadyPrefixed() { + #expect(CalibrationIdentity.prefix("CAL_foo") == "CAL_foo") + #expect(CalibrationIdentity.prefix("foo") == "CAL_foo") + let id = CalibrationIdentity.parse(liveBasename: "CAL_CAL_foo", persistedOriginal: "") + #expect(id.originalBasename == "CAL_foo") + #expect(id.calibrationBasename == "CAL_foo") + } + + @Test("prefix never invents a name from empty input") + func prefixEmpty() { + #expect(CalibrationIdentity.prefix("").isEmpty) + #expect(CalibrationIdentity.strip("foo") == "foo") + #expect(CalibrationIdentity.strip("CAL_foo") == "foo") + } + + @Test("runner process id for a calibration targen is targen_CAL_*") + func processIdMatches() { + let cal = CalibrationIdentity.prefix("foo") + #expect(ProcessID.targen(cal) == "targen_CAL_foo") + } +} diff --git a/Tests/ICCeryCoreTests/JSONFileStoreTests.swift b/Tests/ICCeryCoreTests/JSONFileStoreTests.swift index b9f20c2..c64bdaf 100644 --- a/Tests/ICCeryCoreTests/JSONFileStoreTests.swift +++ b/Tests/ICCeryCoreTests/JSONFileStoreTests.swift @@ -9,7 +9,17 @@ struct JSONFileStoreTests { .appendingPathComponent("json-store-\(UUID().uuidString).json") } - @Test("Corrupt file with replaceWithDefault returns default") + @Test("Missing file returns default") + func missingFileDefaults() throws { + let store = JSONFileStore( + fileURL: tempURL(), + corrupt: .throwCorrupt, + defaultValue: { .default } + ) + #expect(try store.load() == .default) + } + + @Test("Corrupt file with replaceWithDefault returns default and leaves bytes") func corruptDefaults() throws { let url = tempURL() try "{ not json".write(to: url, atomically: true, encoding: .utf8) @@ -19,7 +29,8 @@ struct JSONFileStoreTests { defaultValue: { .default } ) #expect(try store.load() == .default) - #expect(FileManager.default.fileExists(atPath: url.path)) + let kept = try String(contentsOf: url, encoding: .utf8) + #expect(kept == "{ not json") } @Test("Corrupt file with throwCorrupt throws and leaves bytes") @@ -50,43 +61,24 @@ struct JSONFileStoreTests { let text = try String(contentsOf: url, encoding: .utf8) #expect(text.contains("\n")) #expect(text.contains("\"delta_e_good_max\"")) - } -} - -@Suite("CalibrationIdentity") -struct CalibrationIdentityTests { - @Test("live foo, no persisted") - func livePlain() { - let id = CalibrationIdentity.parse(liveBasename: "foo", persistedOriginal: "") - #expect(id.originalBasename == "foo") - #expect(id.calibrationBasename == "CAL_foo") - } - - @Test("live CAL_foo, persisted foo") - func liveCalPersisted() { - let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "foo") - #expect(id.originalBasename == "foo") - #expect(id.calibrationBasename == "CAL_foo") - } - - @Test("live CAL_foo, empty persisted strips prefix") - func liveCalNoPersist() { - let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "") - #expect(id.originalBasename == "foo") - #expect(id.calibrationBasename == "CAL_foo") - } - - @Test("persisted original wins") - func persistedWins() { - let id = CalibrationIdentity.parse(liveBasename: "CAL_foo", persistedOriginal: "bar") - #expect(id.originalBasename == "bar") - #expect(id.calibrationBasename == "CAL_bar") - } - - @Test("empty live does not invent a name") - func emptyLive() { - let id = CalibrationIdentity.parse(liveBasename: "", persistedOriginal: "") - #expect(id.originalBasename.isEmpty) - #expect(id.calibrationBasename.isEmpty) + // Lexical key sorting: ascending order of top-level keys. + let keys = [ + "ask_before_overwrite_profile", + "calibration_stale_days", + "custom_presets", + "default_install_location", + "delta_e_good_max", + "delta_e_warning_max", + "enable_i1pro2_leds", + "open_color_panel_after_install", + ] + var lastIndex = text.startIndex + for key in keys { + guard let range = text.range(of: "\"\(key)\"", range: lastIndex.. (TestAppEnvironment, TargetWorkflowViewModel) { + let env = try TestAppEnvironment.make() + return (env, TargetWorkflowViewModel(environment: env.environment)) + } + + @Test("Applying a nil-FWA preset after a custom FWA clears the stale path") + func nilFwaClearsCustomPath() throws { + let (env, vm) = try makeWorkflow() + defer { env.cleanup() } + + var customPreset = ProfilingPreset( + id: "c-fwa", name: "FWA", patchCount: 800, + colprofFwa: "/tmp/fwa.sp" + ) + vm.applyPreset(customPreset) + #expect(vm.profile.fwaSelection == .custom) + #expect(vm.profile.fwaCustomPath == "/tmp/fwa.sp") + + customPreset.colprofFwa = nil + vm.applyPreset(customPreset) + #expect(vm.profile.fwaSelection == .none) + #expect(vm.profile.fwaCustomPath == "") + #expect(vm.profile.fwaValue == nil) + } + + @Test("Custom FWA preset path survives the round-trip to colprof_fwa") + func customFwaRoundTrip() throws { + let (env, vm) = try makeWorkflow() + defer { env.cleanup() } + + let preset = ProfilingPreset( + id: "c-fwa2", name: "FWA2", patchCount: 800, + colprofFwa: "/tmp/other.sp" + ) + vm.applyPreset(preset) + #expect(vm.profile.fwaSelection == .custom) + #expect(vm.profile.fwaCustomPath == "/tmp/other.sp") + #expect(vm.profile.fwaValue == "/tmp/other.sp") + } + + @Test("Preset calibration reaches Stage 2 instead of stale live state") + func presetCalibrationReachesStage2() throws { + let (env, vm) = try makeWorkflow() + defer { env.cleanup() } + + // Stale live state must not leak into the preset-applied layout. + vm.profile.applyCalibration = true + vm.profile.calibrationFile = "/tmp/stale.cal" + + let preset = ProfilingPreset( + id: "c-cal", name: "Cal", patchCount: 800, + calibrationFile: "/tmp/preset.cal", + applyCalibration: true + ) + vm.applyPreset(preset) + + #expect(vm.profile.applyCalibration) + #expect(vm.profile.calibrationFile == "/tmp/preset.cal") + #expect(vm.buildPrinttargConfig().calibrationFile == "/tmp/preset.cal") + } + + @Test("Preset with calibration disabled clears Stage 2 calibration") + func disabledCalibrationClearsStage2() throws { + let (env, vm) = try makeWorkflow() + defer { env.cleanup() } + + vm.profile.applyCalibration = true + vm.profile.calibrationFile = "/tmp/stale.cal" + + let preset = ProfilingPreset( + id: "c-nocal", name: "NoCal", patchCount: 800, + calibrationFile: "/tmp/preset.cal", + applyCalibration: nil + ) + vm.applyPreset(preset) + + #expect(!vm.profile.applyCalibration) + #expect(vm.buildPrinttargConfig().calibrationFile == nil) + } + + @Test("Preset Stage 1/2 form fields apply to the live form") + func formFieldsApply() throws { + let (env, vm) = try makeWorkflow() + defer { env.cleanup() } + + var preset = ProfilingPreset( + id: "c-form", name: "Form", + colourSpace: "cmyk", patchCount: 1500, + whitePatches: 6, + blackPatches: 8, + greySteps: 9, + fullSpreadAlgorithm: "r", + pageSize: "250x300", + dpi: 150 + ) + vm.applyPreset(preset) + + #expect(vm.colourSpace == .cmyk) + #expect(vm.effectivePatchCount == 1500) + #expect(vm.whitePatches == 6) + #expect(vm.blackPatches == 8) + #expect(vm.greyStepsEnabled && vm.greySteps == 9) + #expect(vm.algorithm == .random) + #expect(vm.tiffDpi == 150) + #expect(vm.pageSize == .custom) + #expect(vm.customPageW == 250 && vm.customPageH == 300) + #expect(vm.selectedPresetID == "c-form") + + // Disabled advanced controls stay nil in the snapshot, not + // numeric sentinels. + preset.greySteps = nil + vm.applyPreset(preset) + #expect(!vm.greyStepsEnabled) + #expect(vm.buildTargenConfig().greySteps == nil) + } +} diff --git a/Tests/ICCeryCoreTests/PrintcalArgsTests.swift b/Tests/ICCeryCoreTests/PrintcalArgsTests.swift index e13af6f..57caf9e 100644 --- a/Tests/ICCeryCoreTests/PrintcalArgsTests.swift +++ b/Tests/ICCeryCoreTests/PrintcalArgsTests.swift @@ -44,6 +44,29 @@ struct PrintcalArgsTests { ]) } + @Test("Whitespace-only previous calibration path emits no -a") + func whitespacePreviousCal() throws { + let config = PrintcalConfig( + ti3Basename: "demo", + outputURL: tmp, + previousCalPath: " \n\t " + ) + let args = try PrintcalArgs.build(config: config) + #expect(!args.contains("-a")) + #expect(args == ["-v", "-e", "-o", "/tmp/out.cal", "CAL_demo"]) + } + + @Test("Previous calibration path is trimmed before emission") + func previousCalTrimmed() throws { + let config = PrintcalConfig( + ti3Basename: "demo", + outputURL: tmp, + previousCalPath: " /tmp/old.cal " + ) + let args = try PrintcalArgs.build(config: config) + #expect(args[args.firstIndex(of: "-a")! + 1] == "/tmp/old.cal") + } + @Test("Rejects invalid per-channel limit") func rejectsBadChannelLimit() { let config = PrintcalConfig( diff --git a/Tests/ICCeryCoreTests/PrinttargTests.swift b/Tests/ICCeryCoreTests/PrinttargTests.swift index 440b919..bab9fdc 100644 --- a/Tests/ICCeryCoreTests/PrinttargTests.swift +++ b/Tests/ICCeryCoreTests/PrinttargTests.swift @@ -131,6 +131,23 @@ struct PrinttargArgsTests { #expect(!args.contains("-I")) } + @Test("Whitespace-only label emits no -d; whitespace-only calibration emits no -K/-I") + func whitespaceOptions() throws { + let args = try PrinttargArgs.build( + config: config(label: " \n ", calFile: " \t ")) + #expect(!args.contains("-d")) + #expect(!args.contains("-K")) + #expect(!args.contains("-I")) + } + + @Test("Label and calibration values are trimmed before emission") + func trimmedOptions() throws { + let args = try PrinttargArgs.build( + config: config(label: " My Label ", calFile: " /tmp/a.cal ")) + #expect(args[args.firstIndex(of: "-d")! + 1] == "My Label") + #expect(args[args.firstIndex(of: "-K")! + 1] == "/tmp/a.cal") + } + @Test("Unsafe basename throws") func unsafeBasename() { #expect(throws: PathSecurity.Error.self) { @@ -349,7 +366,7 @@ struct ArgyllRunnerPrinttargTests { } } - @Test("Non-zero exit throws processFailed and stays on stage") + @Test("Non-zero exit throws toolFailed and stays on stage") func failure() async throws { let dir = try makeFixture(""" #!/bin/sh @@ -360,7 +377,8 @@ struct ArgyllRunnerPrinttargTests { let runner = ArgyllRunner( processManager: ProcessManager(), binaryResolver: BinaryResolver(bundledRoot: dir, overrideDir: dir)) - await #expect(throws: ArgyllRunnerError.self) { + await #expect(throws: ArgyllRunnerError.toolFailed( + tool: "printtarg", code: 3, logs: ["oops"])) { try await runner.runPrinttarg( config: PrinttargConfig(basename: "x", workingDirectory: dir)) } diff --git a/Tests/ICCeryCoreTests/ProcessManagerTests.swift b/Tests/ICCeryCoreTests/ProcessManagerTests.swift index 083d5f7..716bf92 100644 --- a/Tests/ICCeryCoreTests/ProcessManagerTests.swift +++ b/Tests/ICCeryCoreTests/ProcessManagerTests.swift @@ -58,6 +58,59 @@ struct ProcessManagerTests { func finish() -> Bool { lock.lock(); defer { lock.unlock() }; if finished { return false }; finished = true; return true } } + /// Subscribes synchronously (registration happens inside `events()`) + /// then records every event for `id` until the task is cancelled. + /// Unlike `collect`, observation continues past `.exit` so tests can + /// prove exactly-once exit emission. + private func observe( + _ manager: ProcessManager, + id: String, + into box: Box + ) -> Task { + let stream = manager.events() + return Task { + for await event in stream { + guard event.id == id else { continue } + box.append(event) + } + } + } + + private func exitCount(in box: Box) -> Int { + box.events.filter { if case .exit = $0 { return true }; return false }.count + } + + private func waitForExit(in box: Box, timeout: TimeInterval = 10) async -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if exitCount(in: box) > 0 { return true } + try? await Task.sleep(for: .milliseconds(10)) + } + return false + } + + private func waitForFile(_ url: URL, timeout: TimeInterval = 5) async -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if FileManager.default.fileExists(atPath: url.path) { return true } + try? await Task.sleep(for: .milliseconds(10)) + } + return false + } + + private func waitForRunning( + _ manager: ProcessManager, + id: String, + timeout: TimeInterval = 5 + ) async -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if await manager.isRunning(id) { return true } + try? await Task.sleep(for: .milliseconds(10)) + } + return false + } + // MARK: - Tests @Test func streamsStdoutAndEmitsExit() async throws { @@ -214,6 +267,145 @@ struct ProcessManagerTests { try await pm.sendStdin(id: "nope", text: "d\n") } } + + @Test func explicitPartialFlushEmitsRowColorsJSON() async throws { + let pm = ProcessManager() + let marker = Self.fixtureDir + .appendingPathComponent("partial-row-ready-\(UUID().uuidString)") + let bin = try script( + "partial-row.sh", + "#!/bin/sh\nprintf 'ROW_COLORS_JSON: {\"row\":9}'\ntouch \"$1\"\nsleep 30\n" + ) + let box = Box() + let observer = observe(pm, id: "t11", into: box) + try await pm.runStreaming(id: "t11", binary: bin, arguments: [marker.path]) + #expect(await waitForFile(marker)) + // Retry the flush so the pipe-ingest task can win the actor race + // on a loaded host; the first successful flush emits the row. + var flushed = false + for _ in 0..<50 { + await pm.flushPartialLine(id: "t11") + if box.events.contains(where: { if case .jsonRow = $0 { return true }; return false }) { + flushed = true + break + } + try await Task.sleep(for: .milliseconds(20)) + } + #expect(flushed) + await pm.kill(id: "t11") + #expect(await waitForExit(in: box)) + observer.cancel() + let events = box.events + let rows = events.compactMap { e -> String? in + if case .jsonRow(_, let d) = e { return String(decoding: d, as: UTF8.self) } + return nil + } + #expect(rows == ["{\"row\":9}"]) + // Prefixed tails must not leak into stdout, even via finalize. + #expect(!events.contains(.stdout(id: "t11", line: "ROW_COLORS_JSON: {\"row\":9}"))) + #expect(exitCount(in: box) == 1) + } + + @Test func unterminatedRowTailFinalizesAsJSONRow() async throws { + let pm = ProcessManager() + let bin = try script( + "row-tail.sh", + "#!/bin/sh\nprintf 'ROW_COLORS_JSON: {\"row\":42}'\n" + ) + let box = Box() + let observer = observe(pm, id: "t12", into: box) + try await pm.runStreaming(id: "t12", binary: bin, arguments: []) + #expect(await waitForExit(in: box)) + observer.cancel() + let events = box.events + let rows = events.compactMap { e -> String? in + if case .jsonRow(_, let d) = e { return String(decoding: d, as: UTF8.self) } + return nil + } + #expect(rows == ["{\"row\":42}"]) + #expect(!events.contains(.stdout(id: "t12", line: "ROW_COLORS_JSON: {\"row\":42}"))) + let rowIndex = events.firstIndex { + if case .jsonRow = $0 { return true }; return false + } + let exitIndexes = events.indices.filter { + if case .exit = events[$0] { return true }; return false + } + #expect(exitIndexes.count == 1) + if let rowIndex, let exitIndex = exitIndexes.first { + #expect(rowIndex < exitIndex) + } else { + Issue.record("expected a jsonRow before the exit event") + } + } + + @Test func fastStreamingExitEmitsExactlyOneExit() async throws { + let pm = ProcessManager() + let bin = try script("fast-stream.sh", "#!/bin/sh\nexit 0\n") + let box = Box() + let observer = observe(pm, id: "t13", into: box) + try await pm.runStreaming(id: "t13", binary: bin, arguments: []) + #expect(await waitForExit(in: box)) + // The grace window must outlast the 2 s finalize watchdog so a + // duplicate emission from it would be observed. + try await Task.sleep(for: .milliseconds(2500)) + observer.cancel() + #expect(box.events == [.exit(id: "t13", code: 0)]) + } + + @Test func fastCapturedExitEmitsExactlyOneExit() async throws { + let pm = ProcessManager() + let bin = try script("fast-cap.sh", "#!/bin/sh\nexit 7\n") + let box = Box() + let observer = observe(pm, id: "t14", into: box) + let result = try await pm.runCaptured(id: "t14", binary: bin, arguments: []) + #expect(result.exitCode == 7) + // Both the termination handler and the waitUntilExit watchdog + // resume the same box; give the slower path time to fire. + try await Task.sleep(for: .milliseconds(500)) + observer.cancel() + #expect(box.events == [.exit(id: "t14", code: 7)]) + } + + @Test func capturedRunSetsArgyllNotInteractive() async throws { + let pm = ProcessManager() + let bin = try script( + "cap-env.sh", + "#!/bin/sh\necho \"ANI=$ARGYLL_NOT_INTERACTIVE\"\n" + ) + let result = try await pm.runCaptured(id: "t15", binary: bin, arguments: []) + #expect(result.stdout == "ANI=1\n") + } + + @Test func killAllTerminatesStreamingAndCapturedChildren() async throws { + let pm = ProcessManager() + let marker = Self.fixtureDir + .appendingPathComponent("mixed-cap-ready-\(UUID().uuidString)") + let slowBin = try script("mixed-slow.sh", "#!/bin/sh\nsleep 30\n") + let capBin = try script("mixed-cap.sh", "#!/bin/sh\ntouch \"$1\"\nsleep 30\n") + let streamBox = Box() + let capBox = Box() + let streamObserver = observe(pm, id: "t16", into: streamBox) + let capObserver = observe(pm, id: "t17", into: capBox) + try await pm.runStreaming(id: "t16", binary: slowBin, arguments: []) + let capTask = Task { + try await pm.runCaptured(id: "t17", binary: capBin, arguments: [marker.path]) + } + #expect(await waitForFile(marker)) + #expect(await waitForRunning(pm, id: "t16")) + #expect(await waitForRunning(pm, id: "t17")) + #expect(await pm.killAll() == 2) + _ = try await capTask.value + #expect(await waitForExit(in: streamBox)) + #expect(await waitForExit(in: capBox)) + // Grace window outlasts the streaming finalize watchdog. + try await Task.sleep(for: .milliseconds(2500)) + streamObserver.cancel() + capObserver.cancel() + #expect(!(await pm.isRunning("t16"))) + #expect(!(await pm.isRunning("t17"))) + #expect(exitCount(in: streamBox) == 1) + #expect(exitCount(in: capBox) == 1) + } } @Suite("ProcessLineDecoder") diff --git a/Tests/ICCeryCoreTests/ProcessRunSupportTests.swift b/Tests/ICCeryCoreTests/ProcessRunSupportTests.swift new file mode 100644 index 0000000..79ebe1d --- /dev/null +++ b/Tests/ICCeryCoreTests/ProcessRunSupportTests.swift @@ -0,0 +1,67 @@ +import Foundation +import Testing +@testable import ICCery + +/// Direct contracts for the shared logged-run helper (issue #80). +/// +/// `runLogged` owns the running-flag transition (`false → true → false`) +/// and the log-reset decision; these tests pin both sides of the +/// contract plus the coalesced `@MainActor` log hop. +@Suite("ProcessRunSupport runLogged") +@MainActor +struct ProcessRunSupportTests { + + private struct SentinelError: Error {} + + @Test("Success: running transitions [true, false], log resets once, batches reach the main actor, value preserved") + func successTransitions() async throws { + var running: [Bool] = [] + var resets = 0 + var received: [String] = [] + + let result = try await ProcessRunSupport.runLogged( + setRunning: { running.append($0) }, + resetLog: { resets += 1 }, + onLog: { batch in + MainActor.assertIsolated() + received.append(contentsOf: batch) + } + ) { onLog in + onLog(["alpha", "beta"]) + return 42 + } + + #expect(result == 42) + #expect(running == [true, false]) + #expect(resets == 1) + + // The sink hops back through a main-actor Task; yield until the + // coalesced batch lands. + for _ in 0..<200 where received.isEmpty { + try await Task.sleep(for: .milliseconds(10)) + } + #expect(received == ["alpha", "beta"]) + } + + @Test("Failure: running still transitions [true, false], log resets once, error is rethrown") + func failureTransitions() async throws { + var running: [Bool] = [] + var resets = 0 + + do { + _ = try await ProcessRunSupport.runLogged( + setRunning: { running.append($0) }, + resetLog: { resets += 1 }, + onLog: { _ in } + ) { _ -> Int in + throw SentinelError() + } + Issue.record("Expected runLogged to rethrow") + } catch is SentinelError { + // Expected path. + } + + #expect(running == [true, false]) + #expect(resets == 1) + } +} diff --git a/Tests/ICCeryCoreTests/SettingsTests.swift b/Tests/ICCeryCoreTests/SettingsTests.swift index 5fed795..9b86b15 100644 --- a/Tests/ICCeryCoreTests/SettingsTests.swift +++ b/Tests/ICCeryCoreTests/SettingsTests.swift @@ -93,6 +93,28 @@ struct SettingsStoreTests { #expect(!FileManager.default.fileExists(atPath: url.path)) } + @Test func invalidSaveOverValidFilePreservesBytesAndPostsNothing() throws { + let url = tempStoreURL() + let store = SettingsStore(fileURL: url) + var valid = AppSettings.default + valid.deltaEGoodMax = 1.5 + try store.save(valid) + let originalBytes = try Data(contentsOf: url) + + var fired = false + let token = NotificationCenter.default.addObserver( + forName: SettingsStore.settingsDidChange, object: nil, queue: nil + ) { _ in fired = true } + defer { NotificationCenter.default.removeObserver(token) } + + var invalid = AppSettings.default + invalid.deltaEGoodMax = 9.0 + #expect(throws: SettingsStore.SettingsError.self) { try store.save(invalid) } + #expect(try Data(contentsOf: url) == originalBytes) + #expect(!fired) + #expect(store.load() == valid) + } + @Test func savePostsNotification() async throws { let url = tempStoreURL() let store = SettingsStore(fileURL: url) diff --git a/Tests/ICCeryCoreTests/TargenTests.swift b/Tests/ICCeryCoreTests/TargenTests.swift index 3329bcc..821efce 100644 --- a/Tests/ICCeryCoreTests/TargenTests.swift +++ b/Tests/ICCeryCoreTests/TargenTests.swift @@ -162,6 +162,34 @@ struct TargenArgsTests { #expect(!args.contains("-p")) } + @Test("Whitespace-only preconditioning profile emits no -c") + func whitespacePreconditioner() throws { + let config = TargenConfig( + colourSpace: .rgb, + patchCount: 800, + whitePatches: 4, + blackPatches: 4, + preconditioningProfile: " \n\t ", + basename: "ws_pre" + ) + let args = try TargenArgs.build(config: config) + #expect(!args.contains("-c")) + } + + @Test("Preconditioning profile is trimmed before emission") + func preconditionerTrimmed() throws { + let config = TargenConfig( + colourSpace: .rgb, + patchCount: 800, + whitePatches: 4, + blackPatches: 4, + preconditioningProfile: " /path/to/profile.icc ", + basename: "trim_pre" + ) + let args = try TargenArgs.build(config: config) + #expect(args[args.firstIndex(of: "-c")! + 1] == "/path/to/profile.icc") + } + @Test("Invalid basename throws") func invalidBasenameThrows() { let config = TargenConfig( @@ -262,7 +290,7 @@ struct ArgyllRunnerTargenTests { #expect(ti1URL.lastPathComponent == "mock_test.ti1") } - @Test("Failed targen execution throws processFailed") + @Test("Failed targen execution throws toolFailed") func failedTargenExecution() async throws { let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) @@ -290,7 +318,8 @@ struct ArgyllRunnerTargenTests { workingDirectory: tempDir ) - await #expect(throws: ArgyllRunnerError.self) { + await #expect(throws: ArgyllRunnerError.toolFailed( + tool: "targen", code: 1, logs: ["Error: something went wrong"])) { try await runner.runTargen(config: config) } } @@ -323,7 +352,8 @@ struct ArgyllRunnerTargenTests { workingDirectory: tempDir ) - await #expect(throws: ArgyllRunnerError.self) { + await #expect(throws: ArgyllRunnerError.missingArtefact( + tempDir.appendingPathComponent("no_file.ti1").path)) { try await runner.runTargen(config: config) } } diff --git a/Tests/ICCeryCoreTests/TargetWorkflowViewModelTests.swift b/Tests/ICCeryCoreTests/TargetWorkflowViewModelTests.swift new file mode 100644 index 0000000..beac3f3 --- /dev/null +++ b/Tests/ICCeryCoreTests/TargetWorkflowViewModelTests.swift @@ -0,0 +1,42 @@ +import Foundation +import Testing +@testable import ICCeryCore +@testable import ICCery + +/// Dataset-import error contracts through the +/// `importMeasurementDataset(from:)` seam (issue #80): parser and I/O +/// failures must surface identically as a single `.error` Notice. +@Suite("TargetWorkflowViewModel dataset import") +@MainActor +struct TargetWorkflowViewModelTests { + + @Test("Malformed content (CGATSParseError) produces one .error notice prefixed 'Import failed:'") + func malformedDatasetNotice() throws { + let env = try TestAppEnvironment.make() + defer { env.cleanup() } + let vm = TargetWorkflowViewModel(environment: env.environment) + + let bad = env.root.appendingPathComponent("broken.ti3") + try Data("this is not CGATS data".utf8).write(to: bad) + + vm.importMeasurementDataset(from: bad) + + let notice = try #require(vm.wizard.notice) + #expect(notice.kind == .error) + #expect(notice.text.hasPrefix("Import failed:")) + } + + @Test("Missing file (CocoaError) produces one .error notice prefixed 'Import failed:'") + func missingDatasetNotice() throws { + let env = try TestAppEnvironment.make() + defer { env.cleanup() } + let vm = TargetWorkflowViewModel(environment: env.environment) + + let missing = env.root.appendingPathComponent("does-not-exist.ti3") + vm.importMeasurementDataset(from: missing) + + let notice = try #require(vm.wizard.notice) + #expect(notice.kind == .error) + #expect(notice.text.hasPrefix("Import failed:")) + } +} diff --git a/Tests/ICCeryCoreTests/TestAppEnvironment.swift b/Tests/ICCeryCoreTests/TestAppEnvironment.swift new file mode 100644 index 0000000..ac27fe4 --- /dev/null +++ b/Tests/ICCeryCoreTests/TestAppEnvironment.swift @@ -0,0 +1,62 @@ +import Testing +import Foundation +@testable import ICCeryCore +@testable import ICCery + +/// Shared app-test dependency factory (issue #82). +/// +/// Every store is pointed at a unique temporary directory so tests never +/// read or write the user's real Application Support tree, and a fresh +/// `ProcessManager` keeps child-process state isolated per test. The +/// global process environment is never mutated. +struct TestAppEnvironment { + + /// Root temp directory holding all per-test state files. + let root: URL + let environment: AppEnvironment + + var settingsURL: URL { root.appendingPathComponent("settings.json") } + var stateURL: URL { root.appendingPathComponent("wizard_state.json") } + var historyURL: URL { + root.appendingPathComponent("verification_history.json") + } + + /// Creates an isolated environment under `NSTemporaryDirectory()`. + /// Call `cleanup()` when finished. + static func make() throws -> TestAppEnvironment { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("iccery-test-env-\(UUID().uuidString)") + try FileManager.default.createDirectory( + at: root, withIntermediateDirectories: true + ) + + let processManager = ProcessManager() + let settingsStore = SettingsStore( + fileURL: root.appendingPathComponent("settings.json") + ) + let environment = AppEnvironment( + stateStore: WizardStateStore( + fileURL: root.appendingPathComponent("wizard_state.json") + ), + settingsStore: settingsStore, + presetStore: PresetStore(settingsStore: settingsStore), + runner: ArgyllRunner( + processManager: processManager, + binaryResolver: BinaryResolver(overrideDir: nil) + ), + cupsService: CupsService( + processManager: processManager, + binaryDir: root.appendingPathComponent("cups-bin") + ), + historyStore: VerificationHistoryStore( + url: root.appendingPathComponent("verification_history.json") + ) + ) + return TestAppEnvironment(root: root, environment: environment) + } + + /// Removes the temporary root directory. + func cleanup() { + try? FileManager.default.removeItem(at: root) + } +} diff --git a/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift index 91a63fc..313b297 100644 --- a/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift +++ b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift @@ -131,6 +131,57 @@ struct VerificationHistoryStoreTests { } } + @Test("Clear does not overwrite an unparseable file") + func clearPreservesUnparseableFile() async { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try? fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + let badJSON = "not json" + try? badJSON.write(to: url, atomically: true, encoding: .utf8) + + let store = VerificationHistoryStore(url: url) + do { + try await store.clear() + Issue.record("clear() should propagate the load error") + } catch { + let contents = try? String(contentsOf: url, encoding: .utf8) + #expect(contents == badJSON) + } + } + + @Test("ISO-8601 timestamps round-trip through a fresh store") + func iso8601RoundTrip() async throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + let timestamp = Date(timeIntervalSince1970: 1_700_000_000) + let record = VerificationRecord( + id: "vr-iso", + profileName: "p", + printerName: "", + avgDE: 1.0, + maxDE: 2.0, + rmsDE: 1.5, + patchCount: 1, + status: .good, + timestamp: timestamp + ) + let store1 = VerificationHistoryStore(url: url) + _ = try await store1.append(record) + + let text = try String(contentsOf: url, encoding: .utf8) + #expect(text.contains(ISO8601DateFormatter().string(from: timestamp))) + + let store2 = VerificationHistoryStore(url: url) + let loaded = try await store2.load() + #expect(loaded.count == 1) + #expect(loaded.first?.timestamp == timestamp) + } + @Test("CSV export quoting") func csvQuoting() async throws { let fm = FileManager.default diff --git a/Tests/ICCeryCoreTests/WizardGatingTests.swift b/Tests/ICCeryCoreTests/WizardGatingTests.swift index afd7a91..2f8a6b9 100644 --- a/Tests/ICCeryCoreTests/WizardGatingTests.swift +++ b/Tests/ICCeryCoreTests/WizardGatingTests.swift @@ -117,6 +117,17 @@ struct WizardStateStoreTests { #expect(WizardStateStore(fileURL: url).load().stage == .generate) } + @Test func corruptJsonReturnsDefaultAndKeepsBytes() throws { + let url = tempURL() + try FileManager.default.createDirectory( + at: url.deletingLastPathComponent(), withIntermediateDirectories: true + ) + try "not json".write(to: url, atomically: true, encoding: .utf8) + #expect(WizardStateStore(fileURL: url).load() == .default) + let kept = try String(contentsOf: url, encoding: .utf8) + #expect(kept == "not json") + } + @Test func sessionModeCalibrationRoundTrips() throws { var s = WizardState(sessionMode: .calibration) let data = try JSONEncoder().encode(s) diff --git a/Tests/ICCeryUITests/Milestone2UITests.swift b/Tests/ICCeryUITests/Milestone2UITests.swift index a648e7e..31bdded 100644 --- a/Tests/ICCeryUITests/Milestone2UITests.swift +++ b/Tests/ICCeryUITests/Milestone2UITests.swift @@ -133,6 +133,18 @@ final class Milestone2UITests: XCTestCase { XCTAssertTrue(element("targenInkLimitGroup").waitForExistence(timeout: 5)) } + /// Stage 1/2 process-log containers resolve under the shared + /// `ProcessLogView` identifiers (issue #80). + func testProcessLogContainersResolve() throws { + launchApp() + XCTAssertTrue(waitFor("targenLogContainer").exists) + + app.buttons["btnBrowse"].click() + app.buttons["btnGenerate"].click() + XCTAssertTrue(waitFor("btnCreateLayout", timeout: 20).exists) + XCTAssertTrue(element("printtargLogContainer").exists) + } + /// Fixture-backed targen run creates .ti1 and unlocks Stage 2. func testTargenFixtureUnlocksStage2() throws { launchApp() diff --git a/Tests/ICCeryUITests/Milestone3UITests.swift b/Tests/ICCeryUITests/Milestone3UITests.swift index 94a4218..ad4a294 100644 --- a/Tests/ICCeryUITests/Milestone3UITests.swift +++ b/Tests/ICCeryUITests/Milestone3UITests.swift @@ -146,6 +146,8 @@ final class Milestone3UITests: XCTestCase { XCTAssertTrue(notice.waitForExistence(timeout: 10)) XCTAssertTrue((notice.value as? String ?? "") .contains("cancelled")) + // Cancellation is informational, never an error (#80). + XCTAssertEqual(element("printNotificationIcon").value as? String, "info") } /// Preferences OK → captured options are replayed verbatim in the @@ -214,6 +216,8 @@ final class Milestone3UITests: XCTestCase { let notice = app.staticTexts.containing(predicate).firstMatch XCTAssertTrue(notice.waitForExistence(timeout: 10)) XCTAssertTrue(notice.label.contains("Print failed")) + // Spool failure exposes the .error kind on the icon (#80). + XCTAssertEqual(element("printNotificationIcon").value as? String, "error") } /// wizardState.printerName records the queue used for spooling (#95). diff --git a/Tests/ICCeryUITests/Milestone4UITests.swift b/Tests/ICCeryUITests/Milestone4UITests.swift index 92759aa..66fda1e 100644 --- a/Tests/ICCeryUITests/Milestone4UITests.swift +++ b/Tests/ICCeryUITests/Milestone4UITests.swift @@ -140,4 +140,57 @@ final class Milestone4UITests: XCTestCase { } XCTAssertTrue(FileManager.default.fileExists(atPath: ti3.path)) } + + /// Two passes + a failing `average` run promote pass 1 to the + /// canonical .ti3 and show the sticky finish error notice via + /// `chartreadFinishNotice` (issue #80). + func testTwoPassAverageFailurePromotesFirstPass() throws { + app.launchEnvironment["MOCK_AVERAGE_FAIL"] = "1" + reachStage3() + + app.buttons["btnDetectInstruments"].click() + _ = waitFor("chartreadInstrumentSelect", timeout: 20) + + driveOnePass(startButton: "btnStartRead") + _ = waitFor("chartreadAveragingPanel", timeout: 20) + + driveOnePass(startButton: "btnMeasureAnotherSheet") + + XCTAssertTrue(waitFor("btnFinishAndAverage", timeout: 20).exists) + app.buttons["btnFinishAndAverage"].click() + + // Averaging failed → pass 1 is promoted to the canonical .ti3 + // and the sticky error notice stays on Stage 3. + let ti3 = workDir.appendingPathComponent("mytarget.ti3") + let deadline = Date().addingTimeInterval(20) + while Date() < deadline, !FileManager.default.fileExists(atPath: ti3.path) { + RunLoop.current.run(until: Date().addingTimeInterval(0.2)) + } + XCTAssertTrue(FileManager.default.fileExists(atPath: ti3.path)) + + let notice = element("chartreadFinishNotice") + XCTAssertTrue(notice.waitForExistence(timeout: 10)) + XCTAssertEqual(notice.value as? String, "error") + } + + /// Runs the mock handheld chartread session to completion + /// (start → calibrate → strip A → strip B → Done & Save). + private func driveOnePass(startButton: String) { + let start = app.buttons[startButton] + XCTAssertTrue(start.waitForExistence(timeout: 10)) + let deadline = Date().addingTimeInterval(10) + while Date() < deadline, !start.isEnabled { + RunLoop.current.run(until: Date().addingTimeInterval(0.1)) + } + XCTAssertTrue(start.isEnabled) + start.click() + _ = waitFor("btnCalibrate", timeout: 25) + app.buttons["btnCalibrate"].click() + _ = waitFor("btnTrigger", timeout: 20) + app.buttons["btnTrigger"].click() + _ = waitFor("btnTrigger", timeout: 20) + app.buttons["btnTrigger"].click() + _ = waitFor("btnDoneRead", timeout: 20) + app.buttons["btnDoneRead"].firstMatch.click() + } } diff --git a/Tests/ICCeryUITests/Milestone5UITests.swift b/Tests/ICCeryUITests/Milestone5UITests.swift index 3c6929b..91150d7 100644 --- a/Tests/ICCeryUITests/Milestone5UITests.swift +++ b/Tests/ICCeryUITests/Milestone5UITests.swift @@ -111,4 +111,21 @@ final class Milestone5UITests: XCTestCase { "Expected verification status, got '\(statusValue)'" ) } + + /// A failing colprof run surfaces through the session-wide wizard + /// notice only — no duplicate stage-local error view (issue #80). + func testProfileFailureShowsWizardNotice() throws { + app.launchEnvironment["ICCERY_MOCK_COLPROF_EXIT"] = "2" + launchApp() + + let create = waitFor("btnCreateProfile") + XCTAssertTrue(create.isEnabled) + create.click() + + let notice = element("noticeText") + XCTAssertTrue(notice.waitForExistence(timeout: 20)) + XCTAssertTrue((notice.value as? String ?? "") + .contains("Profile creation failed")) + XCTAssertFalse(element("colprofLastError").exists) + } } diff --git a/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift b/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift index 272219b..450fe4a 100644 --- a/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift +++ b/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift @@ -32,6 +32,7 @@ final class Milestone6CalibrationUITests: XCTestCase { "ICCERY_TEST_WORKDIR": testWorkDir.path ] app.launch() + app.activate() } override func tearDown() async throws { @@ -77,4 +78,60 @@ final class Milestone6CalibrationUITests: XCTestCase { let layout = app.buttons["btnCreateLayout"] XCTAssertTrue(layout.waitForExistence(timeout: 25)) } + + /// A failing calibration targen surfaces the error through the + /// wizard notice and restores the original basename (issue #80). + func testCalibrationTargenFailureRestoresBasename() throws { + let testRoot = FileManager.default.temporaryDirectory + .appendingPathComponent("cal-fail-\(UUID().uuidString)") + let appData = testRoot.appendingPathComponent("AppData") + try FileManager.default.createDirectory( + at: appData, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: testRoot) } + + // Pre-stage wizard state so the failing mock targen is only + // exercised by the calibration run, not target generation. + let state: [String: Any] = [ + "currentStage": 1, + "basename": "DemoTarget", + "cwd": testWorkDir.path, + "sessionMode": "profile", + "calibrationOriginalBasename": "" + ] + let stateURL = appData.appendingPathComponent("wizard_state.json") + try JSONSerialization.data(withJSONObject: state).write(to: stateURL) + + app.terminate() + app.launchEnvironment["ICCERY_TEST_ROOT"] = testRoot.path + app.launchEnvironment["ICCERY_MOCK_TARGEN_EXIT"] = "2" + app.launch() + app.activate() + + let calButton = app.buttons["btnCalibratePrinter"] + XCTAssertTrue(calButton.waitForExistence(timeout: 10)) + calButton.tap() + + let calGenerate = app.buttons["btnCalGenerate"] + XCTAssertTrue(calGenerate.waitForExistence(timeout: 10)) + calGenerate.tap() + + let notice = app.descendants(matching: .any)["noticeText"] + XCTAssertTrue(notice.waitForExistence(timeout: 20)) + XCTAssertTrue((notice.value as? String ?? "") + .contains("Calibration target failed")) + + // The pre-CAL_ basename is restored and persisted. + let deadline = Date().addingTimeInterval(10) + var restoredBasename: String? + while Date() < deadline { + if let data = try? Data(contentsOf: stateURL), + let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let basename = object["basename"] as? String { + restoredBasename = basename + if basename == "DemoTarget" { break } + } + RunLoop.current.run(until: Date().addingTimeInterval(0.1)) + } + XCTAssertEqual(restoredBasename, "DemoTarget") + } }