From c539507d5d8c39d49ed6dbeda37ad4949e361b5d Mon Sep 17 00:00:00 2001 From: Gronod Date: Fri, 11 Sep 2026 11:10:10 +0100 Subject: [PATCH] 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