Unit tests for interop tests #1823
Conversation
Motivation: We need unit tests that run the interop tests with the in process transport. Modifications: Created a test target and implemented the tests for each interop test, that catch AssertionFailure errors when the interop tests fail. Result: We will run the interop tests with the in process transport.
Package.swift
Outdated
| ] | ||
| ) | ||
|
|
||
| static let interoperabilityUnitTests: Target = .testTarget( |
There was a problem hiding this comment.
nit, name is a little misleading: these aren't unit tests of the interoperability tests, we're adding the interop tests as unit tests.
I think "InProcessInteroperabilityTests" is a bit clearer.
| @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) | ||
| struct EmptyUnary: InteroperabilityTest { | ||
| func run(client: GRPCClient) async throws { | ||
| @Sendable func run(client: GRPCClient) async throws { |
There was a problem hiding this comment.
Rather than making the function @Sendable we should make InteroperabilityTest be Sendable
| import GRPCInProcessTransport | ||
| import XCTest | ||
|
|
||
| @testable import InteroperabilityTests |
There was a problem hiding this comment.
Rather than exposing a @testable import we can rely on the InteroperabilityTestCase enum and makeTest.
There was a problem hiding this comment.
We also need to import TestService from the InteroperabilityTests which is internal - should I make it public? or should I add a public enum for it as well?
There was a problem hiding this comment.
Hmm, no @testable is fine. But do make the switch to use makeTest.
| let emptyUnaryTestCase = EmptyUnary() | ||
| do { | ||
| try await runInProcessTransport(interopTest: emptyUnaryTestCase.run) | ||
| } catch let error as AssertionFailure { |
There was a problem hiding this comment.
We can push this logic into runInProcessTransport(interopTest:) and avoid repeating ourselves.
| let emptyUnaryTestCase = InteroperabilityTestCase.emptyUnary.makeTest() | ||
| try await runInProcessTransport(interopTest: emptyUnaryTestCase.run) |
There was a problem hiding this comment.
We can simplify this yet further by change the param to runInProcessTransport to be InteroperabilityTestCase and calling makeTest and run in there.
| let emptyUnaryTestCase = InteroperabilityTestCase.emptyUnary.makeTest() | ||
| try await runInProcessTransport(interopTest: emptyUnaryTestCase.run) |
There was a problem hiding this comment.
Also: use explicit self (and in all the other tests)
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Couple of nits, looks good otherwise though
|
|
||
| @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) | ||
| public protocol InteroperabilityTest { | ||
| public protocol InteroperabilityTest: Sendable { |
There was a problem hiding this comment.
Now that the tests have been refactored to construct the interop test in place this isn't a requirement anymore.
| } | ||
|
|
||
| func testEmtyUnary() async throws { | ||
| try await self.runInProcessTransport(interopTestCase: InteroperabilityTestCase.emptyUnary) |
There was a problem hiding this comment.
nit: prefer using type inference here
| try await self.runInProcessTransport(interopTestCase: InteroperabilityTestCase.emptyUnary) | |
| try await self.runInProcessTransport(interopTestCase: .emptyUnary) |
Tests/InProcessInteroperabilityTests/InProcessInteroperabilityTests.swift
Show resolved
Hide resolved
2 participants
Motivation:
We need unit tests that run the interop tests with the in process transport.
Modifications:
Created a test target and implemented the tests for each interop test, that catch AssertionFailure errors when the interop tests fail.
Result:
We will run the interop tests with the in process transport.
TODO:
The CustomMetadata test fails - I will add the metadata handling in the TestService in a different PR