MCS connection scaling interop tests for Java#12651
Conversation
| return; | ||
| } | ||
| if (new String(request.getPayload().getBody().toByteArray(), UTF_8) | ||
| .equals(MCS_CS.description())) { |
There was a problem hiding this comment.
Please no. There's two regular approaches we use for tweaking the server's behavior:
- A field in the request proto (e.g.,
fill_username) - Header metadata. I don't think we actually use this approach in the vanilla interop today, but we do do it for psm interop
(fill_username and the like are because of mistakes in the past where grpc-java verified the result message exactly. That means you can't add new fields to have them be ignored. We did want to verify the result was what we expected, but it would have probably been better to just verify the payload. But the behavior hasn't been changed, so we still need these knobs that enable result fields.)
There was a problem hiding this comment.
Added new fields in request and response for setting the client socket address.
…ting." This reverts commit 3719011.
|
I realized I forgot to push the commits addressing review comments last week. |
| .asRuntimeException()); | ||
| return; | ||
| } | ||
| if (whetherSendClientSocketAddressInResponse(request)) { |
There was a problem hiding this comment.
So we just ignore most of the request? It looks like this should go through toChunkQueue() to actually send the response.
There was a problem hiding this comment.
The other parts of the request are about chunking behavior which doesn't apply for this test. I don't need to use toChunkQueue which is used to keep track of what response sizes to send in successive chunks, nor dispatcher.enqueue which handles scheduled sending of the chunks.
There was a problem hiding this comment.
There is no "this test" on the server-side. All features are supported simultaneously. We should plumb it through the Chunk/toResponse().
There was a problem hiding this comment.
I might as well have said: The other parts of the request are about chunking behavior which doesn't apply when the request message indicates that client socket address needs to be sent in the response.
It is just service code branching based on how the particular request needs to be handled.
| @SuppressWarnings("AddressSelection") | ||
| @VisibleForTesting | ||
| void start() throws Exception { | ||
| System.out.println("TestServiceServer.start called with addressType " + addressType); |
There was a problem hiding this comment.
Revert "Add temp debug stmts for server start."?
| .asRuntimeException()); | ||
| return; | ||
| } | ||
| if (whetherSendClientSocketAddressInResponse(request)) { |
There was a problem hiding this comment.
There is no "this test" on the server-side. All features are supported simultaneously. We should plumb it through the Chunk/toResponse().
…dropping intermediary commit with debug statements). # Conflicts: # interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java
kannanjgithub
force-pushed
the
mcs-interop-tests
branch
from
809ae0f to
12f35cf
Compare
# Conflicts: # interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java
2 participants
No description provided.