From b353869b5fe5df011fcd62208e7e92e0f5cf6de5 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Mon, 9 Sep 2024 10:25:05 +0100 Subject: [PATCH] Motivation: swift-testing has a number of advantages over XCTest (parameterisation, organisation, failure messages etc.), we should start using it instead of XCTest. Modifications: - Convert the Status tests - Add a dependency on swift-testing on Linux (this can be removed when it's included as part of the toolchain) - Fixed a couple of bugs found by additional testing Results: Better tests --- Package@swift-6.swift | 12 +++ Sources/GRPCCore/RPCError.swift | 19 ++++ Sources/GRPCCore/Status.swift | 20 +++++ Tests/GRPCCoreTests/StatusTests.swift | 125 +++++++++----------------- 4 files changed, 91 insertions(+), 85 deletions(-) diff --git a/Package@swift-6.swift b/Package@swift-6.swift index 7b5b81394..2b40f6182 100644 --- a/Package@swift-6.swift +++ b/Package@swift-6.swift @@ -72,6 +72,10 @@ let packageDependencies: [Package.Dependency] = [ url: "https://github.com/apple/swift-distributed-tracing.git", from: "1.0.0" ), + .package( + url: "https://github.com/swiftlang/swift-testing.git", + branch: "release/6.0" + ), ].appending( .package( url: "https://github.com/apple/swift-nio-ssl.git", @@ -147,6 +151,13 @@ extension Target.Dependency { static var dequeModule: Self { .product(name: "DequeModule", package: "swift-collections") } static var atomics: Self { .product(name: "Atomics", package: "swift-atomics") } static var tracing: Self { .product(name: "Tracing", package: "swift-distributed-tracing") } + static var testing: Self { + .product( + name: "Testing", + package: "swift-testing", + condition: .when(platforms: [.linux]) // Already included in the toolchain on Darwin + ) + } static var grpcCore: Self { .target(name: "GRPCCore") } static var grpcInProcessTransport: Self { .target(name: "GRPCInProcessTransport") } @@ -402,6 +413,7 @@ extension Target { .grpcInProcessTransport, .dequeModule, .protobuf, + .testing, ], swiftSettings: [.swiftLanguageMode(.v6), .enableUpcomingFeature("ExistentialAny")] ) diff --git a/Sources/GRPCCore/RPCError.swift b/Sources/GRPCCore/RPCError.swift index b9147007d..7354a7b83 100644 --- a/Sources/GRPCCore/RPCError.swift +++ b/Sources/GRPCCore/RPCError.swift @@ -107,6 +107,25 @@ extension RPCError { public var description: String { String(describing: self.wrapped) } + + package static let all: [Self] = [ + .cancelled, + .unknown, + .invalidArgument, + .deadlineExceeded, + .notFound, + .alreadyExists, + .permissionDenied, + .resourceExhausted, + .failedPrecondition, + .aborted, + .outOfRange, + .unimplemented, + .internalError, + .unavailable, + .dataLoss, + .unauthenticated, + ] } } diff --git a/Sources/GRPCCore/Status.swift b/Sources/GRPCCore/Status.swift index 738969c0c..d1c9e087b 100644 --- a/Sources/GRPCCore/Status.swift +++ b/Sources/GRPCCore/Status.swift @@ -166,6 +166,26 @@ extension Status { public var description: String { String(describing: self.wrapped) } + + package static let all: [Self] = [ + .ok, + .cancelled, + .unknown, + .invalidArgument, + .deadlineExceeded, + .notFound, + .alreadyExists, + .permissionDenied, + .resourceExhausted, + .failedPrecondition, + .aborted, + .outOfRange, + .unimplemented, + .internalError, + .unavailable, + .dataLoss, + .unauthenticated, + ] } } diff --git a/Tests/GRPCCoreTests/StatusTests.swift b/Tests/GRPCCoreTests/StatusTests.swift index 98d114934..936ff8e41 100644 --- a/Tests/GRPCCoreTests/StatusTests.swift +++ b/Tests/GRPCCoreTests/StatusTests.swift @@ -13,105 +13,60 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import GRPCCore -import XCTest - -final class StatusTests: XCTestCase { - private static let statusCodeRawValue: [(Status.Code, Int)] = [ - (.ok, 0), - (.cancelled, 1), - (.unknown, 2), - (.invalidArgument, 3), - (.deadlineExceeded, 4), - (.notFound, 5), - (.alreadyExists, 6), - (.permissionDenied, 7), - (.resourceExhausted, 8), - (.failedPrecondition, 9), - (.aborted, 10), - (.outOfRange, 11), - (.unimplemented, 12), - (.internalError, 13), - (.unavailable, 14), - (.dataLoss, 15), - (.unauthenticated, 16), - ] - func testCustomStringConvertible() { - XCTAssertDescription(Status(code: .ok, message: ""), #"ok: """#) - XCTAssertDescription(Status(code: .dataLoss, message: "message"), #"dataLoss: "message""#) - XCTAssertDescription(Status(code: .unknown, message: "message"), #"unknown: "message""#) - XCTAssertDescription(Status(code: .aborted, message: "message"), #"aborted: "message""#) - } +import GRPCCore +import Testing - func testStatusCodeRawValues() { - for (code, expected) in Self.statusCodeRawValue { - XCTAssertEqual(code.rawValue, expected, "\(code) had unexpected raw value") +@Suite("Status") +struct StatusTests { + @Suite("Code") + struct Code { + @Test("rawValue", arguments: zip(Status.Code.all, 0 ... 16)) + func rawValueOfStatusCodes(code: Status.Code, expected: Int) { + #expect(code.rawValue == expected) } - } - - func testStatusCodeFromErrorCode() throws { - XCTAssertEqual(Status.Code(RPCError.Code.cancelled), .cancelled) - XCTAssertEqual(Status.Code(RPCError.Code.unknown), .unknown) - XCTAssertEqual(Status.Code(RPCError.Code.invalidArgument), .invalidArgument) - XCTAssertEqual(Status.Code(RPCError.Code.deadlineExceeded), .deadlineExceeded) - XCTAssertEqual(Status.Code(RPCError.Code.notFound), .notFound) - XCTAssertEqual(Status.Code(RPCError.Code.alreadyExists), .alreadyExists) - XCTAssertEqual(Status.Code(RPCError.Code.permissionDenied), .permissionDenied) - XCTAssertEqual(Status.Code(RPCError.Code.resourceExhausted), .resourceExhausted) - XCTAssertEqual(Status.Code(RPCError.Code.failedPrecondition), .failedPrecondition) - XCTAssertEqual(Status.Code(RPCError.Code.aborted), .aborted) - XCTAssertEqual(Status.Code(RPCError.Code.outOfRange), .outOfRange) - XCTAssertEqual(Status.Code(RPCError.Code.unimplemented), .unimplemented) - XCTAssertEqual(Status.Code(RPCError.Code.internalError), .internalError) - XCTAssertEqual(Status.Code(RPCError.Code.unavailable), .unavailable) - XCTAssertEqual(Status.Code(RPCError.Code.dataLoss), .dataLoss) - XCTAssertEqual(Status.Code(RPCError.Code.unauthenticated), .unauthenticated) - } - func testStatusCodeFromValidRawValue() { - for (expected, rawValue) in Self.statusCodeRawValue { - XCTAssertEqual( - Status.Code(rawValue: rawValue), - expected, - "\(rawValue) didn't convert to expected code \(expected)" + @Test( + "Initialize from RPCError.Code", + arguments: zip( + RPCError.Code.all, + Status.Code.all.dropFirst() // Drop '.ok', there is no '.ok' error code. ) + ) + func initFromRPCErrorCode(errorCode: RPCError.Code, expected: Status.Code) { + #expect(Status.Code(errorCode) == expected) } - } - func testStatusCodeFromInvalidRawValue() { - // Internally represented as a `UInt8`; try all other values. - for rawValue in UInt8(17) ... UInt8.max { - XCTAssertNil(Status.Code(rawValue: Int(rawValue))) + @Test("Initialize from rawValue", arguments: zip(0 ... 16, Status.Code.all)) + func initFromRawValue(rawValue: Int, expected: Status.Code) { + #expect(Status.Code(rawValue: rawValue) == expected) } - // API accepts `Int` so try invalid `Int` values too. - XCTAssertNil(Status.Code(rawValue: -1)) - XCTAssertNil(Status.Code(rawValue: 1000)) - XCTAssertNil(Status.Code(rawValue: .max)) + @Test("Initialize from invalid rawValue", arguments: [-1, 17, 100, .max]) + func initFromInvalidRawValue(rawValue: Int) { + #expect(Status.Code(rawValue: rawValue) == nil) + } } - func testEquatableConformance() { - XCTAssertEqual(Status(code: .ok, message: ""), Status(code: .ok, message: "")) - XCTAssertEqual(Status(code: .ok, message: "message"), Status(code: .ok, message: "message")) - - XCTAssertNotEqual( - Status(code: .ok, message: ""), - Status(code: .ok, message: "message") - ) + @Test("CustomStringConvertible conformance") + func customStringConvertible() { + #expect("\(Status(code: .ok, message: ""))" == #"ok: """#) + #expect("\(Status(code: .dataLoss, message: "oh no"))" == #"dataLoss: "oh no""#) + } - XCTAssertNotEqual( - Status(code: .ok, message: "message"), - Status(code: .internalError, message: "message") - ) + @Test("Equatable conformance") + func equatable() { + let ok = Status(code: .ok, message: "") + let okWithMessage = Status(code: .ok, message: "message") + let internalError = Status(code: .internalError, message: "") - XCTAssertNotEqual( - Status(code: .ok, message: "message"), - Status(code: .ok, message: "different message") - ) + #expect(ok == ok) + #expect(ok != okWithMessage) + #expect(ok != internalError) } - func testFitsInExistentialContainer() { - XCTAssertLessThanOrEqual(MemoryLayout.size, 24) + @Test("Fits in existential container") + func fitsInExistentialContainer() { + #expect(MemoryLayout.size <= 24) } }