grpc: parameterize integration tests by gRPC client type.#2434
grpc: parameterize integration tests by gRPC client type.#2434htuch merged 1 commit intoenvoyproxy:masterfrom
Conversation
Testing: New paramaterized integration tests. Risk Level: Low Signed-off-by: Harvey Tuch <htuch@google.com>
| case Grpc::ClientType::GoogleGrpc: | ||
| test_server_->waitForCounterGe("grpc.ratelimit.streams_closed_4", 1); | ||
| EXPECT_EQ(1, test_server_->counter("grpc.ratelimit.streams_total")->value()); | ||
| EXPECT_EQ(1, test_server_->counter("grpc.ratelimit.streams_closed_4")->value()); |
There was a problem hiding this comment.
"grpc.ratelimit.streams_closed_4" is a pretty strange stat name. Is that a bug?
There was a problem hiding this comment.
Nup; it's grpc.<stat prefix>.streams_closed_<grpc status code>.
There was a problem hiding this comment.
Where is this implemented? I'm confused. Why don't we have similar stats for Envoy gRPC client?
There was a problem hiding this comment.
We had a discussion about this, where the outcome was that this would make more sense to do in a router related stat for Envoy gRPC (and also do this for data plane gRPC at the same time). It will be implemented in the Google gRPC PR coming up, where with Google gRPC we maintain stats that would normally show up under upstream cluster stats etc.
There was a problem hiding this comment.
Ah OK, my main question was how this is passing, since I didn't see this implemented yet. So this PR doesn't actually enable the Google client tests yet?
There was a problem hiding this comment.
Nevermind, I see the parameter only running Envoy tests now. OK.
Which will be needed when we update to the latest version of bazel: #22295 Signed-off-by: JP Simard <jp@jpsim.com>
Which will be needed when we update to the latest version of bazel: #22295 Signed-off-by: JP Simard <jp@jpsim.com>
Testing: New parameterized integration tests.
Risk Level: Low
Signed-off-by: Harvey Tuch htuch@google.com