Interop Tests implementation #1821
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: The test cases test certain functionalities of the new GRPC server and client. Modifications: - created the InteroperabilityTest protocol with the run method - created the InteroperabilityTestCase enum that will be used by main to run certain tests according to the input - implemented the interop tests for the features we support (no caching or compression tests) - added new assertions to the test framework Result: We will be able to implement the main() function that runs the interop tests using in process transport.
glbrntt
requested changes
Mar 6, 2024
Collaborator
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Looks good for the most part, but I think there are a couple of bugs which need fixing first.
Package.swift
Outdated
| name: "InteroperabilityTests", | ||
| dependencies: [ | ||
| .grpcCore, | ||
| .grpcInProcessTransport, |
Collaborator
There was a problem hiding this comment.
We don't need to provide a transport here
Comment on lines
54
to
58
| public func assertFailure( | ||
| _ message: String = "Failure.", | ||
| file: String = #fileID, | ||
| line: Int = #line | ||
| ) throws { |
Collaborator
There was a problem hiding this comment.
Not sure we should bother with this, we can just throw AssertionFailure instead
Comment on lines
99
to
125
| /// The set of server features required to run this test. | ||
| public var requiredServerFeatures: Set<ServerFeature> { | ||
| switch self { | ||
| case .emptyUnary: | ||
| return [.emptyCall] | ||
| case .largeUnary: | ||
| return [.unaryCall] | ||
| case .clientStreaming: | ||
| return [.streamingInputCall] | ||
| case .serverStreaming: | ||
| return [.streamingOutputCall] | ||
| case .pingPong: | ||
| return [.fullDuplexCall] | ||
| case .emptyStream: | ||
| return [.fullDuplexCall] | ||
| case .customMetadata: | ||
| return [.unaryCall, .fullDuplexCall, .echoMetadata] | ||
| case .statusCodeAndMessage: | ||
| return [.unaryCall, .fullDuplexCall, .echoStatus] | ||
| case .specialStatusMessage: | ||
| return [.unaryCall, .echoStatus] | ||
| case .unimplementedMethod: | ||
| return [] | ||
| case .unimplementedService: | ||
| return [] | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
I don't think we ever used this in the previous set of interop tests, we can just remove this and the server features.
| @@ -0,0 +1,714 @@ | |||
| import Dispatch | |||
Collaborator
There was a problem hiding this comment.
This should come after the license header
| * limitations under the License. | ||
| */ | ||
| import GRPCCore | ||
| import GRPCInProcessTransport |
Collaborator
There was a problem hiding this comment.
Don't think we ever use this
Comment on lines
643
to
644
| try await testServiceClient.unaryCall(request: ClientRequest.Single(message: message)) { | ||
| response in |
Collaborator
There was a problem hiding this comment.
Suggested change
| try await testServiceClient.unaryCall(request: ClientRequest.Single(message: message)) { | |
| response in | |
| try await testServiceClient.unaryCall( | |
| request: ClientRequest.Single(message: message) | |
| ) { response in |
Comment on lines
677
to
678
| ) { | ||
| response in |
Collaborator
There was a problem hiding this comment.
Suggested change
| ) { | |
| response in | |
| ) { response in |
| case .success(_): | ||
| try assertFailure("The result should be an error.") | ||
| case .failure(let error): | ||
| try assertEqual(error.code, RPCError.Code.unimplemented) |
Collaborator
There was a problem hiding this comment.
Suggested change
| try assertEqual(error.code, RPCError.Code.unimplemented) | |
| try assertEqual(error.code, .unimplemented) |
Comment on lines
710
to
711
| ) { | ||
| response in |
Collaborator
There was a problem hiding this comment.
Suggested change
| ) { | |
| response in | |
| ) { response in |
| case .success(_): | ||
| try assertFailure("The result should be an error.") | ||
| case .failure(let error): | ||
| try assertEqual(error.code, RPCError.Code.unimplemented) |
Collaborator
There was a problem hiding this comment.
Suggested change
| try assertEqual(error.code, RPCError.Code.unimplemented) | |
| try assertEqual(error.code, .unimplemented) |
glbrntt
requested changes
Mar 6, 2024
Collaborator
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Couple of nits but looks good otherwise!
Comment on lines
211
to
212
| file: #fileID, | ||
| line: #line |
Collaborator
There was a problem hiding this comment.
Oh, no need for this, just add a public init to AssertionFailure to default the fileID and line:
public init(message: String, file: String = #fileID, line: Int = #line) {
// ...
}| for try await id in stream { | ||
| for try await id in ids.stream { | ||
| var message = Grpc_Testing_StreamingOutputCallRequest() | ||
| let sizes = [(31_415, 27_182), (9, 8), (2_653, 1_828), (58_979, 45_904)] |
Collaborator
There was a problem hiding this comment.
We should create this array outside of the loop to avoid creating it multiple times
Comment on lines
324
to
325
| file: #fileID, | ||
| line: #line |
Comment on lines
487
to
488
| file: #fileID, | ||
| line: #line |
Comment on lines
553
to
554
| file: #fileID, | ||
| line: #line |
Comment on lines
575
to
576
| file: #fileID, | ||
| line: #line |
Comment on lines
628
to
629
| file: #fileID, | ||
| line: #line |
Comment on lines
666
to
667
| file: #fileID, | ||
| line: #line |
Comment on lines
702
to
703
| file: #fileID, | ||
| line: #line |
glbrntt
approved these changes
Mar 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
The test cases test certain functionalities of the new GRPC server and client.
Modifications:
Result:
We will be able to implement the main() function that runs the interop tests using in process transport.