mirror of
https://github.com/kfattic/cupsadmin.git
synced 2026-11-13 19:06:43 -05:00
Delete Printer: a queue that's already gone is a successful delete
Judge the delete by the read-back (is the queue gone?) instead of lpadmin's exit status. Deleting a queue that was removed elsewhere before the sidebar refreshed made lpadmin fail with 'The printer or class does not exist', and the app showed a failure alert although the queue was gone. Now the status bar shows 'Deleted <queue>' and os_log notes it was already deleted. Unit tests for all four outcomes, plus an opt-in live test (CUPSKIT_LIVE_DELETE=1) on a name that never existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -68,7 +68,7 @@ swift build # debug build of everything
|
|||||||
verifies both BOM ownership entries. The app is notarized and stapled before packaging; the PKG after.
|
verifies both BOM ownership entries. The app is notarized and stapled before packaging; the PKG after.
|
||||||
- The app icon master is `Icon/AppIcon-1024.png`; `build.sh` generates `AppIcon.icns` from it. Don't redraw it.
|
- The app icon master is `Icon/AppIcon-1024.png`; `build.sh` generates `AppIcon.icns` from it. Don't redraw it.
|
||||||
- Live tests are opt-in and write to queues: `CUPSKIT_LIVE_QUEUE=<queue>`, `CUPSKIT_LIVE_JOBS=<a>,<b>`,
|
- Live tests are opt-in and write to queues: `CUPSKIT_LIVE_QUEUE=<queue>`, `CUPSKIT_LIVE_JOBS=<a>,<b>`,
|
||||||
`CUPSKIT_LIVE_QUICK=<ricoh>,<generic>`. Use throwaway queues (e.g. `lpd://127.0.0.1/…`, paused) and
|
`CUPSKIT_LIVE_QUICK=<ricoh>,<generic>`, `CUPSKIT_LIVE_DELETE=1` (deletes a queue name that never existed; writes nothing). Use throwaway queues (e.g. `lpd://127.0.0.1/…`, paused) and
|
||||||
delete them afterwards; the live tests restore what they change. Offline tests use vendor PPDs from
|
delete them afterwards; the live tests restore what they change. Offline tests use vendor PPDs from
|
||||||
`CUPSKIT_PPD_DIRS` (colon-separated, e.g. Canon/HP PPDs extracted with `pkgutil --expand-full`, not
|
`CUPSKIT_PPD_DIRS` (colon-separated, e.g. Canon/HP PPDs extracted with `pkgutil --expand-full`, not
|
||||||
installed) and `/Library/Printers/PPDs`, and skip when they aren't there.
|
installed) and `/Library/Printers/PPDs`, and skip when they aren't there.
|
||||||
|
|||||||
@@ -251,7 +251,7 @@ final class PrinterStore {
|
|||||||
deleted = outcome.succeeded
|
deleted = outcome.succeeded
|
||||||
if deleted {
|
if deleted {
|
||||||
report("Deleted \(queue) in \(elapsedText(since: started))")
|
report("Deleted \(queue) in \(elapsedText(since: started))")
|
||||||
log.info("deleted \(queue, privacy: .public)")
|
log.info("deleted \(queue, privacy: .public)\(outcome.message.map { " (\($0))" } ?? "", privacy: .public)")
|
||||||
} else {
|
} else {
|
||||||
report("ERROR: delete \(queue): \(outcome.message ?? "failed")")
|
report("ERROR: delete \(queue): \(outcome.message ?? "failed")")
|
||||||
fail("Couldn’t delete \(queue)", "\(outcome.message ?? "failed")\n\n\(outcome.command)")
|
fail("Couldn’t delete \(queue)", "\(outcome.message ?? "failed")\n\n\(outcome.command)")
|
||||||
|
|||||||
@@ -222,12 +222,21 @@ extension QueueActions {
|
|||||||
/// `lpadmin -x queue`, verified by the queue being gone.
|
/// `lpadmin -x queue`, verified by the queue being gone.
|
||||||
public static func delete(queue: String, client: CupsClient) async throws -> ActionOutcome {
|
public static func delete(queue: String, client: CupsClient) async throws -> ActionOutcome {
|
||||||
let result = try await Task.detached { try CupsTools.delete(queue: queue) }.value
|
let result = try await Task.detached { try CupsTools.delete(queue: queue) }.value
|
||||||
guard result.succeeded else {
|
return deleteOutcome(queue: queue, result: result, stillExists: try await client.queueExists(queue))
|
||||||
return ActionOutcome(succeeded: false, command: result.commandLine, message: result.message.isEmpty ? "exited \(result.status)" : result.message)
|
|
||||||
}
|
}
|
||||||
let stillThere = try await client.queueExists(queue)
|
|
||||||
return ActionOutcome(succeeded: !stillThere, command: result.commandLine,
|
/// Judged by the read-back, not lpadmin's exit status: the goal is that the queue is gone. A queue that
|
||||||
message: stillThere ? "Not applied: \(queue) still exists" : nil)
|
/// was already deleted (by another app, lpadmin, or a row the sidebar hadn't refreshed yet) makes
|
||||||
|
/// lpadmin fail with "The printer or class does not exist" — that's still a successful delete.
|
||||||
|
/// `message` is set on success only in that case ("already deleted").
|
||||||
|
static func deleteOutcome(queue: String, result: ToolResult, stillExists: Bool) -> ActionOutcome {
|
||||||
|
if stillExists {
|
||||||
|
let message = result.succeeded ? "Not applied: \(queue) still exists"
|
||||||
|
: result.message.isEmpty ? "exited \(result.status)" : result.message
|
||||||
|
return ActionOutcome(succeeded: false, command: result.commandLine, message: message)
|
||||||
|
}
|
||||||
|
return ActionOutcome(succeeded: true, command: result.commandLine,
|
||||||
|
message: result.succeeded ? nil : "\(queue) was already deleted")
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Sends the CUPS test page (or a generated one-page PDF when it's missing). Returns the job id when verified.
|
/// Sends the CUPS test page (or a generated one-page PDF when it's missing). Returns the job id when verified.
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import CupsKit
|
@testable import CupsKit
|
||||||
import Foundation
|
import Foundation
|
||||||
import Testing
|
import Testing
|
||||||
|
|
||||||
@@ -57,3 +57,45 @@ struct LiveQueueActions {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Delete is judged by whether the queue is gone afterwards, not by lpadmin's exit status.
|
||||||
|
@Suite struct DeleteReadBack {
|
||||||
|
private func result(_ status: Int32, _ stderr: String = "") -> ToolResult {
|
||||||
|
ToolResult(executable: CupsTools.lpadminPath, arguments: ["-x", "Old_Queue"], status: status,
|
||||||
|
standardOutput: Data(), standardError: Data(stderr.utf8))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func deletedNormally() {
|
||||||
|
let outcome = QueueActions.deleteOutcome(queue: "Old_Queue", result: result(0), stillExists: false)
|
||||||
|
#expect(outcome == ActionOutcome(succeeded: true, command: "/usr/sbin/lpadmin -x Old_Queue", message: nil))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The reported bug: the queue was removed elsewhere before Delete Printer ran, lpadmin said
|
||||||
|
/// "The printer or class does not exist", and the app showed a failure alert.
|
||||||
|
@Test func alreadyGoneIsASuccess() {
|
||||||
|
let outcome = QueueActions.deleteOutcome(queue: "Old_Queue",
|
||||||
|
result: result(1, "lpadmin: The printer or class does not exist."),
|
||||||
|
stillExists: false)
|
||||||
|
#expect(outcome.succeeded)
|
||||||
|
#expect(outcome.message == "Old_Queue was already deleted")
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func failureWhileQueueRemainsShowsToolOutput() {
|
||||||
|
let outcome = QueueActions.deleteOutcome(queue: "Old_Queue", result: result(1, "lpadmin: Forbidden"), stillExists: true)
|
||||||
|
#expect(!outcome.succeeded && outcome.message == "lpadmin: Forbidden")
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func exitZeroButStillThereIsNotApplied() {
|
||||||
|
let outcome = QueueActions.deleteOutcome(queue: "Old_Queue", result: result(0), stillExists: true)
|
||||||
|
#expect(!outcome.succeeded && outcome.message == "Not applied: Old_Queue still exists")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Opt-in (CUPSKIT_LIVE_DELETE=1): the real lpadmin and cupsd on a name that never existed. Writes nothing.
|
||||||
|
@Test(.enabled(if: ProcessInfo.processInfo.environment["CUPSKIT_LIVE_DELETE"] != nil))
|
||||||
|
func liveDeleteOfMissingQueue() async throws {
|
||||||
|
let name = "cupsadmin_missing_\(UInt32.random(in: 0 ... .max))"
|
||||||
|
let outcome = try await QueueActions.delete(queue: name, client: CupsClient())
|
||||||
|
#expect(outcome.succeeded, "\(outcome)")
|
||||||
|
#expect(outcome.message == "\(name) was already deleted")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user