Skip to content

Commit f474229

Browse files
numanelahinumanelahi
authored
Fix the flaky grpc_json_reverse_transcoder tests (#38790)
Commit Message: Fix the flaky grpc_json_reverse_transcoder tests Additional Description: There are certain tests that pass in certain environments but fail in others, this PR makes those tests consistent across all environments. Risk Level: None, as this PR only updates the tests. Testing: n/a, as this change itself fixes some of the tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: numanelahi <[email protected]>
1 parent 6235191 commit f474229

File tree

2 files changed

+92
-33
lines changed

2 files changed

+92
-33
lines changed

‎test/extensions/filters/http/grpc_json_reverse_transcoder/filter_test.cc

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ using testing::NiceMock;
1919
using testing::Return;
2020
using testing::ReturnRef;
2121

22+
using Envoy::Protobuf::util::MessageDifferencer;
23+
2224
namespace Envoy {
2325
namespace Extensions {
2426
namespace HttpFilters {
@@ -501,9 +503,7 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, OKResponse) {
501503
{"content-type", "application/grpc"}};
502504
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(req_headers, false));
503505

504-
std::string book_str = "{\"id\":123,\"author\":\"John Doe\",\"title\":\"A Book\"}";
505-
Buffer::OwnedImpl buffer;
506-
buffer.add(book_str);
506+
Buffer::OwnedImpl buffer{"{\"id\":123,\"author\":\"John Doe\",\"title\":\"A Book\"}"};
507507

508508
Http::TestResponseTrailerMapImpl trailers;
509509
EXPECT_CALL(encoder_callbacks_, addEncodedTrailers()).WillOnce(ReturnRef(trailers));
@@ -514,12 +514,20 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, OKResponse) {
514514
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(res_headers, false));
515515

516516
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(buffer, true));
517+
518+
bookstore::Book expected_book;
519+
expected_book.set_id(123);
520+
expected_book.set_author("John Doe");
521+
expected_book.set_title("A Book");
522+
523+
Grpc::Decoder decoder;
524+
std::vector<Grpc::Frame> frames;
525+
std::ignore = decoder.decode(buffer, frames);
526+
517527
bookstore::Book book;
518-
book.set_id(123);
519-
book.set_author("John Doe");
520-
book.set_title("A Book");
521-
auto book_buffer = Grpc::Common::serializeToGrpcFrame(book);
522-
EXPECT_EQ(buffer.toString(), book_buffer.get()->toString());
528+
book.ParseFromString(frames[0].data_->toString());
529+
530+
EXPECT_TRUE(MessageDifferencer::Equals(expected_book, book));
523531
EXPECT_EQ(trailers.getGrpcStatusValue(), "0"); // OK
524532
}
525533

@@ -530,18 +538,23 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, OKResponseWithTrailer) {
530538
{"content-type", "application/grpc"}};
531539
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(req_headers, false));
532540

533-
std::string book_str = "{\"id\":123,\"author\":\"John Doe\",\"title\":\"A Book\"}";
534-
Buffer::OwnedImpl buffer;
535-
buffer.add(book_str);
541+
Buffer::OwnedImpl buffer{"{\"id\":123,\"author\":\"John Doe\",\"title\":\"A Book\"}"};
536542

537543
EXPECT_CALL(encoder_callbacks_, addEncodedData(_, _))
538544
.WillOnce(Invoke([](Buffer::Instance& data, bool) {
545+
bookstore::Book expected_book;
546+
expected_book.set_id(123);
547+
expected_book.set_author("John Doe");
548+
expected_book.set_title("A Book");
549+
550+
Grpc::Decoder decoder;
551+
std::vector<Grpc::Frame> frames;
552+
std::ignore = decoder.decode(data, frames);
553+
539554
bookstore::Book book;
540-
book.set_id(123);
541-
book.set_author("John Doe");
542-
book.set_title("A Book");
543-
auto book_buffer = Grpc::Common::serializeToGrpcFrame(book);
544-
EXPECT_EQ(data.toString(), book_buffer.get()->toString());
555+
book.ParseFromString(frames[0].data_->toString());
556+
557+
EXPECT_TRUE(MessageDifferencer::Equals(expected_book, book));
545558
}));
546559

547560
Http::TestResponseHeaderMapImpl res_headers{{":status", "200"},
@@ -576,11 +589,18 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, OKHttpBodyResponse) {
576589

577590
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(buffer, true));
578591

592+
google::api::HttpBody expected_body;
593+
expected_body.set_content_type("application/json");
594+
expected_body.set_data(book_str);
595+
596+
Grpc::Decoder decoder;
597+
std::vector<Grpc::Frame> frames;
598+
std::ignore = decoder.decode(buffer, frames);
599+
579600
google::api::HttpBody body;
580-
body.set_content_type("application/json");
581-
body.set_data(book_str);
582-
auto body_buffer = Grpc::Common::serializeToGrpcFrame(body);
583-
EXPECT_EQ(buffer.toString(), body_buffer.get()->toString());
601+
body.ParseFromString(frames[0].data_->toString());
602+
603+
EXPECT_TRUE(MessageDifferencer::Equals(expected_body, body));
584604
EXPECT_EQ(trailers.getGrpcStatusValue(), "0");
585605
}
586606

@@ -619,11 +639,18 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, OKHttpBodyResponseWithTrailer) {
619639

620640
EXPECT_CALL(encoder_callbacks_, addEncodedData(_, _))
621641
.WillOnce(Invoke([&book_str](Buffer::Instance& data, bool) {
642+
google::api::HttpBody expected_body;
643+
expected_body.set_content_type("application/json");
644+
expected_body.set_data(book_str);
645+
646+
Grpc::Decoder decoder;
647+
std::vector<Grpc::Frame> frames;
648+
std::ignore = decoder.decode(data, frames);
649+
622650
google::api::HttpBody body;
623-
body.set_content_type("application/json");
624-
body.set_data(book_str);
625-
auto body_buffer = Grpc::Common::serializeToGrpcFrame(body);
626-
EXPECT_EQ(data.toString(), body_buffer.get()->toString());
651+
body.ParseFromString(frames[0].data_->toString());
652+
653+
EXPECT_TRUE(MessageDifferencer::Equals(expected_body, body));
627654
}));
628655

629656
Http::TestResponseHeaderMapImpl res_headers{{":status", "200"},
@@ -715,7 +742,7 @@ TEST_F(GrpcJsonReverseTranscoderFilterTest, MiscEncodingAndDecoding) {
715742
TEST_F(GrpcJsonReverseTranscoderFilterTest, ParseInvalidConfig) {
716743
envoy::extensions::filters::http::grpc_json_reverse_transcoder::v3::GrpcJsonReverseTranscoder
717744
config;
718-
config.set_descriptor_path("test/proto/bookstore.proto");
745+
config.set_descriptor_path(TestEnvironment::runfilesPath("test/proto/bookstore.proto"));
719746
EXPECT_THROW_WITH_MESSAGE(GrpcJsonReverseTranscoderConfig(config, *api_), EnvoyException,
720747
"Unable to parse proto descriptor");
721748
}

‎test/extensions/filters/http/grpc_json_reverse_transcoder/grpc_json_reverse_transcoder_integration_test.cc

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ using absl::StatusCode;
1919
using Envoy::Protobuf::TextFormat;
2020
using Envoy::ProtobufWkt::Empty;
2121

22+
using Envoy::Protobuf::util::MessageDifferencer;
23+
2224
namespace Envoy {
2325
namespace {
2426

@@ -130,8 +132,16 @@ TEST_P(GrpcJsonReverseTranscoderIntegrationTest, SimpleRequest) {
130132
expected_book.set_title("Kids book");
131133
expected_book.set_author("John Doe");
132134

133-
auto serialized_book = Grpc::Common::serializeToGrpcFrame(expected_book);
134-
EXPECT_EQ(response->body(), serialized_book->toString());
135+
Buffer::OwnedImpl transcoded_buffer{response->body()};
136+
137+
Grpc::Decoder decoder;
138+
std::vector<Grpc::Frame> frames;
139+
std::ignore = decoder.decode(transcoded_buffer, frames);
140+
141+
bookstore::Book book;
142+
book.ParseFromString(frames[0].data_->toString());
143+
144+
EXPECT_TRUE(MessageDifferencer::Equals(expected_book, book));
135145

136146
EXPECT_THAT(*response->trailers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "0"));
137147

@@ -199,8 +209,15 @@ TEST_P(GrpcJsonReverseTranscoderIntegrationTest, HttpBodyRequestResponse) {
199209
expected_res.set_content_type(Http::Headers::get().ContentTypeValues.Html);
200210
expected_res.set_data(response_str);
201211

202-
auto serialized_res = Grpc::Common::serializeToGrpcFrame(expected_res);
203-
EXPECT_EQ(response->body(), serialized_res->toString());
212+
Buffer::OwnedImpl transcoded_buffer{response->body()};
213+
Grpc::Decoder decoder;
214+
std::vector<Grpc::Frame> frames;
215+
std::ignore = decoder.decode(transcoded_buffer, frames);
216+
217+
google::api::HttpBody transcoded_res;
218+
transcoded_res.ParseFromString(frames[0].data_->toString());
219+
220+
EXPECT_TRUE(MessageDifferencer::Equals(expected_res, transcoded_res));
204221

205222
EXPECT_THAT(*response->trailers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "0"));
206223

@@ -276,8 +293,15 @@ TEST_P(GrpcJsonReverseTranscoderIntegrationTest, NestedHttpBodyRequest) {
276293
expected_res.set_content_type(Http::Headers::get().ContentTypeValues.Html);
277294
expected_res.set_data(response_str);
278295

279-
auto serialized_res = Grpc::Common::serializeToGrpcFrame(expected_res);
280-
EXPECT_EQ(response->body(), serialized_res->toString());
296+
Buffer::OwnedImpl transcoded_buffer{response->body()};
297+
Grpc::Decoder decoder;
298+
std::vector<Grpc::Frame> frames;
299+
std::ignore = decoder.decode(transcoded_buffer, frames);
300+
301+
google::api::HttpBody transcoded_res;
302+
transcoded_res.ParseFromString(frames[0].data_->toString());
303+
304+
EXPECT_TRUE(MessageDifferencer::Equals(expected_res, transcoded_res));
281305

282306
EXPECT_THAT(*response->trailers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "0"));
283307

@@ -343,8 +367,16 @@ TEST_P(GrpcJsonReverseTranscoderIntegrationTest, RequestWithQueryParams) {
343367
book->set_id(123);
344368
book->set_title("Kids book");
345369
book->set_author("John Doe");
346-
auto serialized_res = Grpc::Common::serializeToGrpcFrame(expected_res);
347-
EXPECT_EQ(response->body(), serialized_res->toString());
370+
371+
Buffer::OwnedImpl transcoded_buffer{response->body()};
372+
Grpc::Decoder decoder;
373+
std::vector<Grpc::Frame> frames;
374+
std::ignore = decoder.decode(transcoded_buffer, frames);
375+
376+
bookstore::ListBooksResponse transcoded_res;
377+
transcoded_res.ParseFromString(frames[0].data_->toString());
378+
379+
EXPECT_TRUE(MessageDifferencer::Equals(expected_res, transcoded_res));
348380

349381
EXPECT_THAT(*response->trailers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "0"));
350382

0 commit comments

Comments
 (0)