Adds new conformance/client module #182
Merged
Conversation
This file contains 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
jhump
force-pushed
the
jh/conformance-revamp
branch
from
9bd518f
to
9d4849f
Compare
- This contains the new conformance client, for the new conformance tests - The base module has all of the logic and uses the Java Lite runtime (since it is lowest common denominator) - There are google-java and google-javalite modules which have the main functions for running the clients. The google-java module swaps out the javalite runtime for the java runtime. The client module (with shared logic) can convert from javalite generated messages to java generated messages by serializing and deserializing - The adapt package in the client module provides a glimpse of what I think better stream APIs would look like. I'd like to update the actual stream interfaces to more closely match this in the future. For now, the code just adapts exising interfaces to these. - To test the different unary invocation styles, there is an enum for each style and a method in UnaryClient that is an adapter: a single interface for the actual conformance client to use (that looks like the Callback style), but under the hood it will call the suspend, callback, or blocking methods based on the desired invocation style. - The actual client logic is TODO, in various handle* methods of the Client class. This felt like a big enough chunk already, so I figured it would be good to get this reviewed before plowing through the rest.
jhump
force-pushed
the
jh/conformance-revamp
branch
from
6201610
to
e430ea9
Compare
pkwarren
reviewed
Jan 2, 2024
conformance/client/google-java/src/main/kotlin/com/connectrpc/conformance/client/java/Main.kt
Show resolved
Hide resolved
conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt
Outdated
Show resolved
Hide resolved
pkwarren
approved these changes
Jan 3, 2024
pkwarren
approved these changes
Jan 3, 2024
jhump
force-pushed
the
jh/conformance-revamp
branch
from
ede63e1
to
527df44
Compare
pkwarren
approved these changes
Jan 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is purely additive. It does not make any changes to the existing container/crosstest-based conformance test targets. It's just starting to add code to eventually replace all of that.
So the old conformance tests are in these modules:
conformance/commonconformance/google-javaconformance/google-javaliteThe new ones are in these:
conformance/clientconformance/client/google-javaconformance/client/google-javaliteThis doesn't include any actual logic yet. This was a rather substantial PR already, so I figured it would be good to get this approach reviewed before fleshing out the conformance client handler logic (which are all stubbed out in
Client.ktwith TODOs).To avoid the redundant implementations of the current conformance tests, I'm doing the following:
conformance/clientmodule. So the shared logic deals with the lite generated message types, since that's the lowest common denominator. (The full runtime should be able to correctly support lite generate code.)conformance/client/google-javalitemodule doesn't need any add'l generated code.conformance/client/google-javamodule, on the other hand, does. So the shared code will convert from generated lite messages to normal/non-lite messages. This uses abuf.gen.lite.yamlwhich has a different package prefix, so both sets of generated messages can co-exist. The dependencies of the non-lite module exclude the javalite protobuf runtime, swapping in the full runtime instead.This also provides a glimpse of what I'm thinking about for an overhaul of the stream interfaces. In particular, see
BidiStreamClient.BidiStream,ServerStreamClient.ServerStream, andClientStreamClient.ClientStream. Currently, these contain adapters from the existing actual stream interfaces to these newer ones. But I'd like to overhaul the main stream interfaces to look more-or-less like these in the future, so let me know what you think.