From 0332a2bb4f26f7f88e7ff8e36b084a145f69b01c Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 10:02:38 +0100 Subject: [PATCH 1/9] docs: normalise milestone tracking and branch workflow for M8 in README --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 -- 2.39.5 From 12584d156a1ef2bed8129b5da85e716a17ca2069 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 10:34:36 +0100 Subject: [PATCH 2/9] fix(persistence): complete M8 JSON store contracts (#81) --- .../ICCeryCore/Files/JSONFileStore.swift | 13 ++--- .../Profile/VerificationHistoryStore.swift | 4 ++ .../ICCeryCoreTests/JSONFileStoreTests.swift | 34 ++++++++++++- Tests/ICCeryCoreTests/SettingsTests.swift | 22 ++++++++ .../VerificationHistoryStoreTests.swift | 51 +++++++++++++++++++ Tests/ICCeryCoreTests/WizardGatingTests.swift | 11 ++++ 6 files changed, 127 insertions(+), 8 deletions(-) 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/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/Tests/ICCeryCoreTests/JSONFileStoreTests.swift b/Tests/ICCeryCoreTests/JSONFileStoreTests.swift index b9f20c2..550a0fb 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,6 +61,25 @@ struct JSONFileStoreTests { let text = try String(contentsOf: url, encoding: .utf8) #expect(text.contains("\n")) #expect(text.contains("\"delta_e_good_max\"")) + // 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.. Date: Fri, 11 Sep 2026 10:53:41 +0100 Subject: [PATCH 3/9] fix(presets): complete config mapping contracts (#82) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Settings/PresetMapping.swift | 49 ++++- Sources/ICCery/ProfileWorkflowViewModel.swift | 44 +--- Sources/ICCery/TargetWorkflowViewModel.swift | 8 +- Tests/ICCeryCoreTests/PresetTests.swift | 206 ++++++++++++++++++ .../PresetViewModelMappingTests.swift | 127 +++++++++++ .../ICCeryCoreTests/TestAppEnvironment.swift | 62 ++++++ 6 files changed, 456 insertions(+), 40 deletions(-) create mode 100644 Tests/ICCeryCoreTests/PresetViewModelMappingTests.swift create mode 100644 Tests/ICCeryCoreTests/TestAppEnvironment.swift 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/Sources/ICCery/ProfileWorkflowViewModel.swift b/Sources/ICCery/ProfileWorkflowViewModel.swift index 589ed21..f0f419a 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 @@ -110,13 +91,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 +106,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. diff --git a/Sources/ICCery/TargetWorkflowViewModel.swift b/Sources/ICCery/TargetWorkflowViewModel.swift index 52159d2..67f4ab8 100644 --- a/Sources/ICCery/TargetWorkflowViewModel.swift +++ b/Sources/ICCery/TargetWorkflowViewModel.swift @@ -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/PresetTests.swift b/Tests/ICCeryCoreTests/PresetTests.swift index eaf482b..7a5167a 100644 --- a/Tests/ICCeryCoreTests/PresetTests.swift +++ b/Tests/ICCeryCoreTests/PresetTests.swift @@ -287,4 +287,210 @@ struct PresetMappingTests { #expect(back.colprofFwa == "D50") #expect(back.greySteps == nil) } + + @Test("Full preset round-trips through all three configs with every field asserted") + func fullRoundTrip() { + let preset = ProfilingPreset( + id: "custom-full", + name: "Full", + description: "All fields", + colourSpace: "cmyk", + patchCount: 1500, + whitePatches: 6, + blackPatches: 8, + greySteps: 9, + singleChannelSteps: 7, + neutralSteps: 4, + neutralConcentration: 0.7, + preconditioningProfile: "/tmp/pre.icm", + ofpsHighQuality: true, + ofpsAdaptation: 0.2, + fullSpreadAlgorithm: "R", + totalInkLimit: 280, + darkEmphasis: 1.3, + devicePower: 1.2, + instrument: "p3", + pageSize: "250x300", + bitDepth: 16, + dpi: 360, + randomSeed: 42, + noRandomize: false, + calibrationFile: "/tmp/a.cal", + applyCalibration: true, + colprofAlgorithm: "x", + colprofQuality: "u", + colprofIntent: "p", + colprofFwa: "D65", + colprofIlluminant: "D65", + colprofObserver: "1931_2", + colprofInputViewingCond: "D50_2", + colprofOutputViewingCond: "D65_2" + ) + + let targen = TargenConfig(preset: preset, basename: "j", workingDirectory: nil) + #expect(targen.colourSpace == .cmyk) + #expect(targen.patchCount == 1500) + #expect(targen.whitePatches == 6) + #expect(targen.blackPatches == 8) + #expect(targen.greySteps == 9) + #expect(targen.singleChannelSteps == 7) + #expect(targen.neutralSteps == 4) + #expect(targen.neutralConcentration == 0.7) + #expect(targen.preconditioningProfile == "/tmp/pre.icm") + #expect(targen.ofpsHighQuality == true) + #expect(targen.ofpsAdaptation == 0.2) + #expect(targen.fullSpreadAlgorithm == .uniformRandom) + #expect(targen.totalInkLimit == 280) + #expect(targen.darkEmphasis == 1.3) + #expect(targen.devicePower == 1.2) + + let printtarg = PrinttargConfig( + preset: preset, + basename: "j", + workingDirectory: nil, + calibrationFile: preset.calibrationFile + ) + #expect(printtarg.instrument == .p3) + #expect(printtarg.pageSize == .custom) + #expect(printtarg.customPageWidth == 250) + #expect(printtarg.customPageHeight == 300) + #expect(printtarg.bitDepth == .sixteen) + #expect(printtarg.dpi == 360) + #expect(printtarg.layoutOrder == .customSeed) + #expect(printtarg.customSeed == 42) + #expect(printtarg.calibrationFile == "/tmp/a.cal") + + let colprof = ColprofConfig(preset: preset, basename: "j", workingDirectory: nil) + #expect(colprof.algorithm == "x") + #expect(colprof.quality == "u") + #expect(colprof.intent == "p") + #expect(colprof.fwa == "D65") + #expect(colprof.illuminant == "D65") + #expect(colprof.observer == "1931_2") + #expect(colprof.inputViewingCond == "D50_2") + #expect(colprof.outputViewingCond == "D65_2") + + let back = ProfilingPreset( + id: preset.id, + name: preset.name, + description: preset.description, + targen: targen, + printtarg: printtarg, + colprof: colprof, + calibrationFile: preset.calibrationFile, + applyCalibration: preset.applyCalibration + ) + #expect(back == preset) + } + + @Test("Every full-spread algorithm round-trips", arguments: [ + ("ofps", FullSpreadAlgorithm.ofps), + ("t", .target), + ("r", .random), + ("R", .uniformRandom), + ("q", .quasiRandom), + ("Q", .uniformQuasiRandom), + ("i", .invertedQuasiRandom), + ("I", .invertedUniformQuasiRandom) + ]) + func fullSpreadAlgorithms(value: String, expected: FullSpreadAlgorithm) { + var preset = ProfilingPreset(id: "x", name: "n", patchCount: 100) + preset.fullSpreadAlgorithm = value + let cfg = TargenConfig(preset: preset, basename: "t", workingDirectory: nil) + if expected == .ofps { + // ofps is the default — no flag emitted, stored value is nil. + #expect(cfg.fullSpreadAlgorithm == nil) + } else { + #expect(cfg.fullSpreadAlgorithm == expected) + } + let back = ProfilingPreset( + id: "x", name: "n", description: "", + targen: cfg, + printtarg: PrinttargConfig( + preset: preset, basename: "t", + workingDirectory: nil, calibrationFile: nil + ), + colprof: ColprofConfig(preset: preset, basename: "t", workingDirectory: nil), + calibrationFile: nil, + applyCalibration: nil + ) + #expect(back.fullSpreadAlgorithm == value) + } + + @Test("Explicit ofpsHighQuality=false is preserved, distinct from nil") + func ofpsHighQualityFalse() { + var preset = ProfilingPreset(id: "x", name: "n", patchCount: 100) + preset.ofpsHighQuality = false + let cfg = TargenConfig(preset: preset, basename: "t", workingDirectory: nil) + #expect(cfg.ofpsHighQuality == false) + + preset.ofpsHighQuality = nil + let nilCfg = TargenConfig(preset: preset, basename: "t", workingDirectory: nil) + #expect(nilCfg.ofpsHighQuality == nil) + } + + @Test("noRandomize/seed layout mapping rules", arguments: [ + (true, nil, LayoutOrder.raster, 1), + (true, 7, .raster, 7), + (false, nil, .deterministic, 1), + (false, 1, .deterministic, 1), + (nil, 1, .deterministic, 1), + (false, 5, .customSeed, 5) + ] as [(Bool?, Int?, LayoutOrder, Int)]) + func layoutMapping(noRandomize: Bool?, seed: Int?, layout: LayoutOrder, expectedSeed: Int) { + var preset = ProfilingPreset(id: "x", name: "n", patchCount: 100) + preset.noRandomize = noRandomize + preset.randomSeed = seed + let cfg = PrinttargConfig( + preset: preset, basename: "t", + workingDirectory: nil, calibrationFile: nil + ) + #expect(cfg.layoutOrder == layout) + #expect(cfg.customSeed == expectedSeed) + } + + @Test("Custom page fallback matrix", arguments: [ + ("250x300", PageSize.custom, 250.0, 300.0), + ("50x50", .custom, 50.0, 50.0), + ("foo", .a4, 210.0, 297.0), + ("30x40", .a4, 210.0, 297.0), + ("210x", .a4, 210.0, 297.0) + ] as [(String, PageSize, Double, Double)]) + func customPageFallback(raw: String, page: PageSize, w: Double, h: Double) { + var preset = ProfilingPreset(id: "x", name: "n", patchCount: 100) + preset.pageSize = raw + let cfg = PrinttargConfig( + preset: preset, basename: "t", + workingDirectory: nil, calibrationFile: nil + ) + #expect(cfg.pageSize == page) + #expect(cfg.customPageWidth == w) + #expect(cfg.customPageHeight == h) + } + + @Test("FWA preset value → selection matrix", arguments: [ + (nil, ColprofFwaSelection.none), + ("none", .none), + ("NONE", .none), + ("", .empty), + ("D50", .D50), + ("d50", .D50), + ("D65", .D65), + ("d65", .D65), + ("/tmp/fwa.sp", .custom) + ] as [(String?, ColprofFwaSelection)]) + func fwaToSelection(raw: String?, expected: ColprofFwaSelection) { + #expect(ColprofFwaSelection(presetValue: raw) == expected) + } + + @Test("FWA selection → preset value matrix", arguments: [ + (ColprofFwaSelection.none, nil), + (.empty, ""), + (.D50, "D50"), + (.D65, "D65"), + (.custom, "/tmp/fwa.sp") + ] as [(ColprofFwaSelection, String?)]) + func fwaToPresetValue(selection: ColprofFwaSelection, expected: String?) { + #expect(selection.presetValue(customPath: "/tmp/fwa.sp") == expected) + } } diff --git a/Tests/ICCeryCoreTests/PresetViewModelMappingTests.swift b/Tests/ICCeryCoreTests/PresetViewModelMappingTests.swift new file mode 100644 index 0000000..30a96e4 --- /dev/null +++ b/Tests/ICCeryCoreTests/PresetViewModelMappingTests.swift @@ -0,0 +1,127 @@ +import Testing +import Foundation +@testable import ICCeryCore +@testable import ICCery + +/// Issue #82 — preset application through the live view models, under an +/// isolated `TestAppEnvironment` (temp stores, fresh ProcessManager). +@Suite("PresetViewModelMapping") +@MainActor +struct PresetViewModelMappingTests { + + private func makeWorkflow() throws -> (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/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) + } +} -- 2.39.5 From c539507d5d8c39d49ed6dbeda37ad4949e361b5d Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 11:10:10 +0100 Subject: [PATCH 4/9] fix(profile): complete calibration identity and profile resolution (#83) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Files/ArtefactProbe.swift | 9 +- .../Profile/CalibrationIdentity.swift | 8 +- .../ArgyllRunnerCalibrationTests.swift | 28 ++++++ .../ICCeryCoreTests/ArtefactFilesTests.swift | 90 +++++++++++++++++-- .../CalibrationIdentityTests.swift | 78 ++++++++++++++++ .../ICCeryCoreTests/JSONFileStoreTests.swift | 38 -------- 6 files changed, 199 insertions(+), 52 deletions(-) create mode 100644 Tests/ICCeryCoreTests/CalibrationIdentityTests.swift 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/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/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift index abe5696..2f50ea7 100644 --- a/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift +++ b/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift @@ -41,6 +41,34 @@ 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 runner = makeRunner() + let events = ProcessManager.shared.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() 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 550a0fb..c64bdaf 100644 --- a/Tests/ICCeryCoreTests/JSONFileStoreTests.swift +++ b/Tests/ICCeryCoreTests/JSONFileStoreTests.swift @@ -82,41 +82,3 @@ struct JSONFileStoreTests { } } } - -@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) - } -} -- 2.39.5 From e48c3f6840d66266cff7c5d835adf268c0971331 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 11:36:50 +0100 Subject: [PATCH 5/9] test(process): complete ProcessManager edge contracts (#84) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Process/ProcessManager.swift | 20 +- .../ICCeryCoreTests/ProcessManagerTests.swift | 192 ++++++++++++++++++ 2 files changed, 206 insertions(+), 6 deletions(-) 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/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") -- 2.39.5 From da602e2775613a072548f89b722d6d6659e582d0 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 12:17:52 +0100 Subject: [PATCH 6/9] refactor(args): finish shared option helpers (#86) Adopt ArgsBuilder helpers across the remaining argument generators while preserving byte-identical argv and exact flag ordering: - TargenArgs: optionUnlessApprox for -N/-V/-p, optionIfNonEmpty for -c, flag for -G, option for -A. - PrinttargArgs: flag for -r, optionIfNonEmpty for -d and the dynamic -K/-I calibration value (CAL_ protection retained). - PrintcalArgs: flag for -I/-z, optionIfNonEmpty for -a. - ColprofArgs left explicit: FWA and "none" viewing-condition branches cannot be represented by the helpers without changing argv. New ArgsBuilderTests cover nil/present/empty/whitespace/trim, epsilon boundaries, POSIX formatting, and flag handling. Added whitespace-only option cases for targen -c, printtarg -d/-K/-I, and printcal -a, plus chartread row decode and unkeyed XYZ/Lab wire-encoding tests. Refs #86 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Argyll/PrinttargArgs.swift | 14 +-- .../ICCeryCore/Argyll/TargenArgs.swift | 26 ++--- .../ICCeryCore/Profile/PrintcalArgs.swift | 13 +-- Tests/ICCeryCoreTests/ArgsBuilderTests.swift | 94 +++++++++++++++++++ Tests/ICCeryCoreTests/MeasurementTests.swift | 45 +++++++++ Tests/ICCeryCoreTests/PrintcalArgsTests.swift | 23 +++++ Tests/ICCeryCoreTests/PrinttargTests.swift | 17 ++++ Tests/ICCeryCoreTests/TargenTests.swift | 28 ++++++ 8 files changed, 224 insertions(+), 36 deletions(-) create mode 100644 Tests/ICCeryCoreTests/ArgsBuilderTests.swift 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/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/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/MeasurementTests.swift b/Tests/ICCeryCoreTests/MeasurementTests.swift index ce9f343..3687237 100644 --- a/Tests/ICCeryCoreTests/MeasurementTests.swift +++ b/Tests/ICCeryCoreTests/MeasurementTests.swift @@ -159,6 +159,51 @@ struct ChartreadRowTests { #expect(row.patchCount == 1) #expect(row.patches[0].measured.lab?.l == 51) } + + @Test("Decodes a row carrying both XYZ and Lab arrays") + func decodeXYZAndLab() throws { + let json = """ + {"event": "row_complete", "row_id": "B", "row_index": 1, "total_rows": 2, + "patch_count": 1, "patches": [ + {"id": "7", "loc": "B7", "is_pad": false, "device": [10, 20, 30, 40], + "measured": {"XYZ": [30.5, 32.1, 25.9], "Lab": [63.4, 2.5, -8.2]}} + ]} + """ + let row = try JSONDecoder().decode(ChartreadRow.self, from: Data(json.utf8)) + let measured = row.patches[0].measured + #expect(measured.xyz == CIEXYZ(x: 30.5, y: 32.1, z: 25.9)) + #expect(measured.lab == CIELab(l: 63.4, a: 2.5, b: -8.2)) + } + + @Test("XYZColor/CIEXYZ encode as an unkeyed three-number array") + func xyzWireEncoding() throws { + for color in [XYZColor(x: 1.5, y: 2.5, z: 3.5), CIEXYZ(x: 1.5, y: 2.5, z: 3.5)] { + let value = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) + #expect(value as? [Double] == [1.5, 2.5, 3.5]) + } + } + + @Test("LabColor/CIELab encode as an unkeyed three-number array") + func labWireEncoding() throws { + for color in [LabColor(l: 50, a: -1, b: 2), CIELab(l: 50, a: -1, b: 2)] { + let value = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) + #expect(value as? [Double] == [50, -1, 2]) + } + } + + @Test("PatchColor keeps the XYZ and Lab keys over unkeyed arrays") + func patchColorKeys() throws { + let color = PatchColor( + xyz: CIEXYZ(x: 10, y: 20, z: 30), + lab: CIELab(l: 55, a: 1, b: -2)) + let object = try JSONSerialization.jsonObject( + with: JSONEncoder().encode(color)) as? [String: Any] + #expect(object?["XYZ"] as? [Double] == [10, 20, 30]) + #expect(object?["Lab"] as? [Double] == [55, 1, -2]) + #expect(object?["spectral"] == nil) + } } @Suite("ColourMath") 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..99eec2a 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) { diff --git a/Tests/ICCeryCoreTests/TargenTests.swift b/Tests/ICCeryCoreTests/TargenTests.swift index 3329bcc..9929021 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( -- 2.39.5 From 78ffeff61bc4bd20add87535c4288268682cb9a7 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 12:33:16 +0100 Subject: [PATCH 7/9] test(runner): complete shared streaming loop contracts (#79) Refs #79 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ArgyllRunnerCalibrationTests.swift | 38 +++-- .../ArgyllRunnerColprofTests.swift | 30 +++- .../ArgyllRunnerStreamingLoopTests.swift | 161 ++++++++++++++++++ Tests/ICCeryCoreTests/PrinttargTests.swift | 5 +- Tests/ICCeryCoreTests/TargenTests.swift | 8 +- 5 files changed, 225 insertions(+), 17 deletions(-) create mode 100644 Tests/ICCeryCoreTests/ArgyllRunnerStreamingLoopTests.swift diff --git a/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerCalibrationTests.swift index 2f50ea7..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) ) } @@ -44,8 +44,9 @@ struct ArgyllRunnerCalibrationTests { @Test("Calibration targen from foo runs as process id targen_CAL_foo") func calibrationTargenProcessId() async throws { let testRoot = try makeTestDir() - let runner = makeRunner() - let events = ProcessManager.shared.events() + 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 { @@ -87,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", @@ -98,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/PrinttargTests.swift b/Tests/ICCeryCoreTests/PrinttargTests.swift index 99eec2a..bab9fdc 100644 --- a/Tests/ICCeryCoreTests/PrinttargTests.swift +++ b/Tests/ICCeryCoreTests/PrinttargTests.swift @@ -366,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 @@ -377,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/TargenTests.swift b/Tests/ICCeryCoreTests/TargenTests.swift index 9929021..821efce 100644 --- a/Tests/ICCeryCoreTests/TargenTests.swift +++ b/Tests/ICCeryCoreTests/TargenTests.swift @@ -290,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) @@ -318,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) } } @@ -351,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) } } -- 2.39.5 From 0d0233e8d69bc567dd60f68fffc2c2fdb07a6c13 Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 13:18:34 +0100 Subject: [PATCH 8/9] refactor(ui): complete logged-run and Notice consolidation (#80) Refs #80 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- Sources/ICCery/CalibrationView.swift | 7 -- Sources/ICCery/CalibrationViewModel.swift | 43 ++++---- .../ICCery/MeasurementWorkflowViewModel.swift | 46 ++++---- Sources/ICCery/NoticeBanner.swift | 8 ++ Sources/ICCery/ProfileWorkflowViewModel.swift | 103 +++++++++--------- Sources/ICCery/Stage2View.swift | 1 + Sources/ICCery/Stage3View.swift | 26 ++--- Sources/ICCery/Stage4View.swift | 23 +--- Sources/ICCery/TargetWorkflowViewModel.swift | 10 +- .../ProcessRunSupportTests.swift | 67 ++++++++++++ .../TargetWorkflowViewModelTests.swift | 42 +++++++ Tests/ICCeryUITests/Milestone2UITests.swift | 12 ++ Tests/ICCeryUITests/Milestone3UITests.swift | 4 + Tests/ICCeryUITests/Milestone4UITests.swift | 53 +++++++++ Tests/ICCeryUITests/Milestone5UITests.swift | 17 +++ .../Milestone6CalibrationUITests.swift | 55 ++++++++++ 16 files changed, 380 insertions(+), 137 deletions(-) create mode 100644 Tests/ICCeryCoreTests/ProcessRunSupportTests.swift create mode 100644 Tests/ICCeryCoreTests/TargetWorkflowViewModelTests.swift 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 f0f419a..8426e61 100644 --- a/Sources/ICCery/ProfileWorkflowViewModel.swift +++ b/Sources/ICCery/ProfileWorkflowViewModel.swift @@ -31,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? @@ -165,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 @@ -285,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 67f4ab8..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 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/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/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..cb8aced 100644 --- a/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift +++ b/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift @@ -77,4 +77,59 @@ 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() + + 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") + } } -- 2.39.5 From 891a504ee722037ab15e01c1ae801a4d4cb9415a Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 15:15:16 +0100 Subject: [PATCH 9/9] test(ui): activate app after launch in calibration UI tests testCalibrationDashboardOpensAndCanGenerate failed in every full-suite gate run while passing standalone: without an explicit activate() the synthesized btnCalGenerate click was consumed by window activation when focus sat on another app after the prior test app terminated. Matches the launchApp() convention used by the other UI suites. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- Tests/ICCeryUITests/Milestone6CalibrationUITests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift b/Tests/ICCeryUITests/Milestone6CalibrationUITests.swift index cb8aced..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 { @@ -104,6 +105,7 @@ final class Milestone6CalibrationUITests: XCTestCase { 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)) -- 2.39.5