From e83845fbd56bb1e3e5d3ca0b4cfcce589b505684 Mon Sep 17 00:00:00 2001 From: ChengHao Yang <17496418+tico88612@users.noreply.github.com> Date: Tue, 19 May 2026 11:42:14 +0800 Subject: [PATCH] Feat: support format validation Some commands don't support some formats (e.g., `yaml`, `toml`) yet; add validation to prevent users from thinking they were executed successfully. Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com> --- Makefile | 3 +- .../Builder/BuilderStatus.swift | 4 + .../Container/ContainerStats.swift | 4 + .../ContainerCommands/Image/ImageList.swift | 4 + Sources/ContainerCommands/ListFormat.swift | 14 ++++ .../ContainerCommands/System/SystemDF.swift | 4 + .../System/SystemStatus.swift | 4 + .../ContainerCommands/Volume/VolumeList.swift | 4 + .../Subcommands/TestCLIFormatValidation.swift | 80 +++++++++++++++++++ 9 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 Tests/CLITests/Subcommands/TestCLIFormatValidation.swift diff --git a/Makefile b/Makefile index e177fef95..a100b667d 100644 --- a/Makefile +++ b/Makefile @@ -211,7 +211,8 @@ INTEGRATION_TEST_SUITES := \ TestCLIKernelSet \ TestCLIAnonymousVolumes \ TestCLINotFound \ - TestCLINoParallelCases + TestCLINoParallelCases \ + TestCLIFormatValidation empty := space := $(empty) $(empty) diff --git a/Sources/ContainerCommands/Builder/BuilderStatus.swift b/Sources/ContainerCommands/Builder/BuilderStatus.swift index 3512fa5e0..c181b77b9 100644 --- a/Sources/ContainerCommands/Builder/BuilderStatus.swift +++ b/Sources/ContainerCommands/Builder/BuilderStatus.swift @@ -41,6 +41,10 @@ extension Application { public init() {} + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + public func run() async throws { do { let client = ContainerClient() diff --git a/Sources/ContainerCommands/Container/ContainerStats.swift b/Sources/ContainerCommands/Container/ContainerStats.swift index a915da86b..02ff853ee 100644 --- a/Sources/ContainerCommands/Container/ContainerStats.swift +++ b/Sources/ContainerCommands/Container/ContainerStats.swift @@ -42,6 +42,10 @@ extension Application { public init() {} + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + public func run() async throws { if format == .json || noStream { // Static mode - get stats once and exit diff --git a/Sources/ContainerCommands/Image/ImageList.swift b/Sources/ContainerCommands/Image/ImageList.swift index c1f2abdbd..e72705dcf 100644 --- a/Sources/ContainerCommands/Image/ImageList.swift +++ b/Sources/ContainerCommands/Image/ImageList.swift @@ -44,6 +44,10 @@ extension Application { @OptionGroup public var logOptions: Flags.Logging + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + public mutating func run() async throws { let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() try Self.validate(quiet: quiet, verbose: verbose) diff --git a/Sources/ContainerCommands/ListFormat.swift b/Sources/ContainerCommands/ListFormat.swift index fa67ccf01..94820aa03 100644 --- a/Sources/ContainerCommands/ListFormat.swift +++ b/Sources/ContainerCommands/ListFormat.swift @@ -15,10 +15,24 @@ //===----------------------------------------------------------------------===// import ArgumentParser +import ContainerizationError public enum ListFormat: String, CaseIterable, ExpressibleByArgument, Sendable { case json case table case yaml case toml + + /// Throws if `self` is not in `supported`. Use from a command's `validate()` + /// so unimplemented formats fail fast with a clear error instead of silently + /// falling through to table output. + public func ensureSupported(_ supported: Set) throws { + if !supported.contains(self) { + let list = supported.map(\.rawValue).sorted().joined(separator: ", ") + throw ContainerizationError( + .invalidArgument, + message: "--format \(self.rawValue) is not supported for this command; supported: \(list)" + ) + } + } } diff --git a/Sources/ContainerCommands/System/SystemDF.swift b/Sources/ContainerCommands/System/SystemDF.swift index db95c2be5..08c9a4737 100644 --- a/Sources/ContainerCommands/System/SystemDF.swift +++ b/Sources/ContainerCommands/System/SystemDF.swift @@ -33,6 +33,10 @@ extension Application { public init() {} + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + public func run() async throws { let stats = try await ClientDiskUsage.get() diff --git a/Sources/ContainerCommands/System/SystemStatus.swift b/Sources/ContainerCommands/System/SystemStatus.swift index 5d4f83c4f..4dc5e9a72 100644 --- a/Sources/ContainerCommands/System/SystemStatus.swift +++ b/Sources/ContainerCommands/System/SystemStatus.swift @@ -39,6 +39,10 @@ extension Application { public init() {} + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + struct PrintableStatus: Codable { let status: String let appRoot: String diff --git a/Sources/ContainerCommands/Volume/VolumeList.swift b/Sources/ContainerCommands/Volume/VolumeList.swift index 124205b63..7ce30e9d5 100644 --- a/Sources/ContainerCommands/Volume/VolumeList.swift +++ b/Sources/ContainerCommands/Volume/VolumeList.swift @@ -39,6 +39,10 @@ extension Application.VolumeCommand { public init() {} + public func validate() throws { + try format.ensureSupported([.json, .table]) + } + public func run() async throws { let volumes = try await ClientVolume.list() diff --git a/Tests/CLITests/Subcommands/TestCLIFormatValidation.swift b/Tests/CLITests/Subcommands/TestCLIFormatValidation.swift new file mode 100644 index 000000000..1c7703c5a --- /dev/null +++ b/Tests/CLITests/Subcommands/TestCLIFormatValidation.swift @@ -0,0 +1,80 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import Testing + +/// Verifies that commands which only implement `--format json|table` reject +/// `--format yaml` and `--format toml` instead of silently falling through to +/// table output. As each command grows real yaml/toml support, drop the +/// corresponding cases from these tests. +@Suite(.serialSuites) +final class TestCLIFormatValidation: CLITest { + + private func expectRejected(_ args: [String], format: String) throws { + let (_, _, error, status) = try run(arguments: args) + #expect(status != 0, "Expected non-zero exit for --format \(format) on \(args.joined(separator: " "))") + #expect(error.contains("--format \(format) is not supported")) + } + + @Test func testImageListRejectsYAML() throws { + try expectRejected(["image", "list", "--format", "yaml"], format: "yaml") + } + + @Test func testImageListRejectsTOML() throws { + try expectRejected(["image", "list", "--format", "toml"], format: "toml") + } + + @Test func testContainerStatsRejectsYAML() throws { + try expectRejected(["stats", "--format", "yaml"], format: "yaml") + } + + @Test func testContainerStatsRejectsTOML() throws { + try expectRejected(["stats", "--format", "toml"], format: "toml") + } + + @Test func testSystemDFRejectsYAML() throws { + try expectRejected(["system", "df", "--format", "yaml"], format: "yaml") + } + + @Test func testSystemDFRejectsTOML() throws { + try expectRejected(["system", "df", "--format", "toml"], format: "toml") + } + + @Test func testBuilderStatusRejectsYAML() throws { + try expectRejected(["builder", "status", "--format", "yaml"], format: "yaml") + } + + @Test func testBuilderStatusRejectsTOML() throws { + try expectRejected(["builder", "status", "--format", "toml"], format: "toml") + } + + @Test func testVolumeListRejectsYAML() throws { + try expectRejected(["volume", "list", "--format", "yaml"], format: "yaml") + } + + @Test func testVolumeListRejectsTOML() throws { + try expectRejected(["volume", "list", "--format", "toml"], format: "toml") + } + + @Test func testSystemStatusRejectsYAML() throws { + try expectRejected(["system", "status", "--format", "yaml"], format: "yaml") + } + + @Test func testSystemStatusRejectsTOML() throws { + try expectRejected(["system", "status", "--format", "toml"], format: "toml") + } +}