Return absl::Status from test helper functions This change refactors several test helper functions to return error instead of asserting in the method, following go/totw/206 PiperOrigin-RevId: 854242806
diff --git a/quiche/binary_http/binary_http_message_test.cc b/quiche/binary_http/binary_http_message_test.cc index d0e0d97..7add275 100644 --- a/quiche/binary_http/binary_http_message_test.cc +++ b/quiche/binary_http/binary_http_message_test.cc
@@ -31,10 +31,15 @@ } template <class T> -void TestPrintTo(const T& resp) { +absl::Status TestPrintTo(const T& resp) { std::ostringstream os; PrintTo(resp, &os); - EXPECT_EQ(os.str(), resp.DebugString()); + if (os.str() != resp.DebugString()) { + return absl::FailedPreconditionError(absl::StrCat( + "PrintTo output to stream does not match DebugString: stream=", + os.str(), ", DebugString=", resp.DebugString())); + } + return absl::OkStatus(); } class RequestMessageSectionTestHandler @@ -53,39 +58,60 @@ RequestMessageSectionTestHandler() = default; absl::Status OnControlData( const BinaryHttpRequest::ControlData& control_data) override { - EXPECT_FALSE(message_data_.control_data_.has_value()); + if (message_data_.control_data_.has_value()) { + return absl::FailedPreconditionError( + "OnControlData called multiple times"); + } message_data_.control_data_ = control_data; return absl::OkStatus(); } absl::Status OnHeader(absl::string_view name, absl::string_view value) override { - EXPECT_FALSE(message_data_.headers_done_); + if (message_data_.headers_done_) { + return absl::FailedPreconditionError( + "OnHeader called after OnHeadersDone"); + } message_data_.headers_.push_back({std::string(name), std::string(value)}); return absl::OkStatus(); } absl::Status OnHeadersDone() override { - EXPECT_FALSE(message_data_.headers_done_); + if (message_data_.headers_done_) { + return absl::FailedPreconditionError( + "OnHeadersDone called multiple times"); + } message_data_.headers_done_ = true; return absl::OkStatus(); } absl::Status OnBodyChunk(absl::string_view body_chunk) override { - EXPECT_FALSE(message_data_.body_chunks_done_); + if (message_data_.body_chunks_done_) { + return absl::FailedPreconditionError( + "OnBodyChunk called after OnBodyChunksDone"); + } message_data_.body_chunks_.push_back(std::string(body_chunk)); return absl::OkStatus(); } absl::Status OnBodyChunksDone() override { - EXPECT_FALSE(message_data_.body_chunks_done_); + if (message_data_.body_chunks_done_) { + return absl::FailedPreconditionError( + "OnBodyChunksDone called multiple times"); + } message_data_.body_chunks_done_ = true; return absl::OkStatus(); } absl::Status OnTrailer(absl::string_view name, absl::string_view value) override { - EXPECT_FALSE(message_data_.trailers_done_); + if (message_data_.trailers_done_) { + return absl::FailedPreconditionError( + "OnTrailer called after OnTrailersDone"); + } message_data_.trailers_.push_back({std::string(name), std::string(value)}); return absl::OkStatus(); } absl::Status OnTrailersDone() override { - EXPECT_FALSE(message_data_.trailers_done_); + if (message_data_.trailers_done_) { + return absl::FailedPreconditionError( + "OnTrailersDone called multiple times"); + } message_data_.trailers_done_ = true; return absl::OkStatus(); } @@ -179,7 +205,7 @@ "libcurl/7.16.3 OpenSSL/0.9.7l " "zlib/1.2.3};Field{host=www.example.com};Field{accept-language=en, " "mi}}Body{}}}")); - TestPrintTo(request); + QUICHE_EXPECT_OK(TestPrintTo(request)); } TEST(BinaryHttpRequest, DecodeGetNoBody) { @@ -214,7 +240,7 @@ {"host", "www.example.com"}, {"accept-language", "en, mi"}}; for (const auto& field : expected_fields) { - TestPrintTo(field); + QUICHE_EXPECT_OK(TestPrintTo(field)); } ASSERT_THAT(request.GetHeaderFields(), ContainerEq(expected_fields)); ASSERT_EQ(request.body(), ""); @@ -225,7 +251,7 @@ "libcurl/7.16.3 OpenSSL/0.9.7l " "zlib/1.2.3};Field{host=www.example.com};Field{accept-language=en, " "mi}}Body{}}}")); - TestPrintTo(request); + QUICHE_EXPECT_OK(TestPrintTo(request)); } TEST(BinaryHttpRequest, EncodeGetNoBodyOrHeaders) { @@ -257,7 +283,7 @@ ASSERT_EQ(*result, expected); EXPECT_THAT(request.DebugString(), StrEq("BinaryHttpRequest{BinaryHttpMessage{Headers{}Body{}}}")); - TestPrintTo(request); + QUICHE_EXPECT_OK(TestPrintTo(request)); } TEST(BinaryHttpRequest, DecodeGetNoBodyOrHeaders) { @@ -280,13 +306,13 @@ FieldsAre("GET", "https", "example.com", "/")); std::vector<BinaryHttpMessage::Field> expected_fields = {}; for (const auto& field : expected_fields) { - TestPrintTo(field); + QUICHE_EXPECT_OK(TestPrintTo(field)); } ASSERT_THAT(request.GetHeaderFields(), ContainerEq(expected_fields)); ASSERT_EQ(request.body(), ""); EXPECT_THAT(request.DebugString(), StrEq("BinaryHttpRequest{BinaryHttpMessage{Headers{}Body{}}}")); - TestPrintTo(request); + QUICHE_EXPECT_OK(TestPrintTo(request)); } } @@ -485,26 +511,43 @@ EXPECT_NE(request, no_body); } -void ExpectRequestMessageSectionHandler( +absl::Status ExpectRequestMessageSectionHandler( const RequestMessageSectionTestHandler::MessageData& message_data) { - EXPECT_TRUE(message_data.control_data_.has_value()); - if (message_data.control_data_.has_value()) { - EXPECT_THAT(*message_data.control_data_, - FieldsAre("POST", "https", "google.com", "/hello")); + if (!message_data.control_data_.has_value()) { + return absl::FailedPreconditionError("control_data missing"); + } + if (message_data.control_data_->method != "POST" || + message_data.control_data_->scheme != "https" || + message_data.control_data_->authority != "google.com" || + message_data.control_data_->path != "/hello") { + return absl::FailedPreconditionError("control_data mismatch"); } std::vector<std::pair<std::string, std::string>> expected_headers = { {"user-agent", "curl/7.16.3 libcurl/7.16.3 OpenSSL/0.9.7l zlib/1.2.3"}, {"accept-language", "en, mi"}}; - EXPECT_TRUE(message_data.headers_done_); - EXPECT_THAT(message_data.headers_, ContainerEq(expected_headers)); + if (!message_data.headers_done_) { + return absl::FailedPreconditionError("headers not done"); + } + if (message_data.headers_ != expected_headers) { + return absl::FailedPreconditionError("headers mismatch"); + } std::vector<std::string> expected_body_chunks = {"chunk1", "chunk2", "chunk3"}; - EXPECT_TRUE(message_data.body_chunks_done_); - EXPECT_THAT(message_data.body_chunks_, ContainerEq(expected_body_chunks)); + if (!message_data.body_chunks_done_) { + return absl::FailedPreconditionError("body chunks not done"); + } + if (message_data.body_chunks_ != expected_body_chunks) { + return absl::FailedPreconditionError("body chunks mismatch"); + } std::vector<std::pair<std::string, std::string>> expected_trailers = { {"trailer1", "value1"}, {"trailer2", "value2"}}; - EXPECT_TRUE(message_data.trailers_done_); - EXPECT_THAT(message_data.trailers_, ContainerEq(expected_trailers)); + if (!message_data.trailers_done_) { + return absl::FailedPreconditionError("trailers not done"); + } + if (message_data.trailers_ != expected_trailers) { + return absl::FailedPreconditionError("trailers mismatch"); + } + return absl::OkStatus(); } TEST(IndeterminateLengthDecoder, FullRequestDecodingSuccess) { @@ -522,7 +565,8 @@ BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); QUICHE_ASSERT_OK(decoder); QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/true)); - ExpectRequestMessageSectionHandler(handler.GetMessageData()); + QUICHE_EXPECT_OK( + ExpectRequestMessageSectionHandler(handler.GetMessageData())); } class MockFailingMessageSectionHandler @@ -674,7 +718,8 @@ // Decode the last byte, send end_stream. QUICHE_EXPECT_OK(decoder->Decode( absl::string_view(&request_bytes[request_bytes.size() - 1], 1), true)); - ExpectRequestMessageSectionHandler(handler.GetMessageData()); + QUICHE_EXPECT_OK( + ExpectRequestMessageSectionHandler(handler.GetMessageData())); } TEST(IndeterminateLengthDecoder, @@ -722,15 +767,24 @@ EXPECT_THAT(status, test::StatusIs(absl::StatusCode::kInvalidArgument)); } -void ExpectTruncatedTrailerSection( +absl::Status ExpectTruncatedTrailerSection( const RequestMessageSectionTestHandler::MessageData& message_data) { - EXPECT_TRUE(message_data.headers_done_); - EXPECT_TRUE(message_data.trailers_done_); + if (!message_data.headers_done_) { + return absl::FailedPreconditionError("headers not done"); + } + if (!message_data.trailers_done_) { + return absl::FailedPreconditionError("trailers not done"); + } std::vector<std::pair<std::string, std::string>> expected_headers = { {"user-agent", "curl/7.16.3 libcurl/7.16.3 OpenSSL/0.9.7l zlib/1.2.3"}, {"accept-language", "en, mi"}}; - EXPECT_THAT(message_data.headers_, ContainerEq(expected_headers)); - EXPECT_THAT(message_data.trailers_, testing::IsEmpty()); + if (message_data.headers_ != expected_headers) { + return absl::FailedPreconditionError("headers mismatch"); + } + if (!message_data.trailers_.empty()) { + return absl::FailedPreconditionError("trailers not empty"); + } + return absl::OkStatus(); } TEST(IndeterminateLengthDecoder, TruncatedBodyAndTrailers) { @@ -749,7 +803,7 @@ auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); EXPECT_THAT(message_data.body_chunks_, testing::IsEmpty()); - ExpectTruncatedTrailerSection(message_data); + QUICHE_EXPECT_OK(ExpectTruncatedTrailerSection(message_data)); } TEST(IndeterminateLengthDecoder, TruncatedBodyAndTrailersSplitEndStream) { @@ -770,7 +824,7 @@ auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); EXPECT_THAT(message_data.body_chunks_, testing::IsEmpty()); - ExpectTruncatedTrailerSection(message_data); + QUICHE_EXPECT_OK(ExpectTruncatedTrailerSection(message_data)); } namespace { @@ -941,7 +995,8 @@ BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); QUICHE_ASSERT_OK(decoder); QUICHE_EXPECT_OK(decoder->Decode(encoded_data, /*end_stream=*/true)); - ExpectRequestMessageSectionHandler(handler.GetMessageData()); + QUICHE_EXPECT_OK( + ExpectRequestMessageSectionHandler(handler.GetMessageData())); } TEST(RequestIndeterminateLengthEncoder, OutOfOrderHeaders) { @@ -1288,7 +1343,7 @@ std::vector<std::string> expected_body_chunks = {"chunk1", "chunk2", "chunk3"}; EXPECT_THAT(message_data.body_chunks_, ContainerEq(expected_body_chunks)); - ExpectTruncatedTrailerSection(message_data); + QUICHE_EXPECT_OK(ExpectTruncatedTrailerSection(message_data)); } TEST(IndeterminateLengthDecoder, TruncatedTrailersSplitEndStream) { @@ -1313,7 +1368,7 @@ std::vector<std::string> expected_body_chunks = {"chunk1", "chunk2", "chunk3"}; EXPECT_THAT(message_data.body_chunks_, ContainerEq(expected_body_chunks)); - ExpectTruncatedTrailerSection(message_data); + QUICHE_EXPECT_OK(ExpectTruncatedTrailerSection(message_data)); } TEST(IndeterminateLengthDecoder, InvalidDecodeAfterEndStream) { @@ -1612,7 +1667,7 @@ "CRLF.\r\n}}InformationalResponse{Field{running=\"sleep " "15\"}};InformationalResponse{Field{link=</style.css>; rel=preload; " "as=style};Field{link=</script.js>; rel=preload; as=script}}}")); - TestPrintTo(response); + QUICHE_EXPECT_OK(TestPrintTo(response)); } TEST(BinaryHttpResponse, DecodeMultiInformationalWithBody) { @@ -1696,7 +1751,7 @@ "CRLF.\r\n}}InformationalResponse{Field{running=\"sleep " "15\"}};InformationalResponse{Field{link=</style.css>; rel=preload; " "as=style};Field{link=</script.js>; rel=preload; as=script}}}")); - TestPrintTo(response); + QUICHE_EXPECT_OK(TestPrintTo(response)); } TEST(BinaryHttpMessage, SwapBody) { @@ -1786,46 +1841,74 @@ } template <typename T> -void TestPadding(T& message) { +absl::Status TestPadding(T& message) { const auto data_so = message.Serialize(); - ASSERT_TRUE(data_so.ok()); + if (!data_so.ok()) { + return data_so.status(); + } auto data = *data_so; - ASSERT_EQ(data.size(), message.EncodedSize()); - + if (data.size() != message.EncodedSize()) { + return absl::FailedPreconditionError("Incorrect size"); + } message.set_num_padding_bytes(10); const auto padded_data_so = message.Serialize(); - ASSERT_TRUE(padded_data_so.ok()); + if (!padded_data_so.ok()) { + return padded_data_so.status(); + } const auto padded_data = *padded_data_so; - ASSERT_EQ(padded_data.size(), message.EncodedSize()); - + if (padded_data.size() != message.EncodedSize()) { + return absl::FailedPreconditionError("Incorrect padded size"); + } // Check padding size output. - ASSERT_EQ(data.size() + 10, padded_data.size()); + if (data.size() + 10 != padded_data.size()) { + return absl::FailedPreconditionError("Padding size mismatch"); + } // Check for valid null byte padding output data.resize(data.size() + 10); - ASSERT_EQ(data, padded_data); - + if (data != padded_data) { + return absl::FailedPreconditionError("Padded data mismatch"); + } // Deserialize padded and not padded, and verify they are the same. const auto deserialized_padded_message_so = T::Create(data); - ASSERT_TRUE(deserialized_padded_message_so.ok()); + if (!deserialized_padded_message_so.ok()) { + return deserialized_padded_message_so.status(); + } const auto deserialized_padded_message = *deserialized_padded_message_so; - ASSERT_EQ(deserialized_padded_message, message); - ASSERT_EQ(deserialized_padded_message.num_padding_bytes(), size_t(10)); - + if (!(deserialized_padded_message == message)) { + return absl::FailedPreconditionError( + "Deserialized padded message != message"); + } + if (deserialized_padded_message.num_padding_bytes() != size_t(10)) { + return absl::FailedPreconditionError( + "Padding bytes mismatch in deserialized"); + } // Invalid padding data[data.size() - 1] = 'a'; const auto bad_so = T::Create(data); - ASSERT_FALSE(bad_so.ok()); - + if (bad_so.ok()) { + return absl::FailedPreconditionError( + "Expected bad status for invalid padding"); + } // Check that padding does not impact equality. data.resize(data.size() - 10); const auto deserialized_message_so = T::Create(data); - ASSERT_TRUE(deserialized_message_so.ok()); + if (!deserialized_message_so.ok()) { + return deserialized_message_so.status(); + } const auto deserialized_message = *deserialized_message_so; - ASSERT_EQ(deserialized_message.num_padding_bytes(), size_t(0)); + if (deserialized_message.num_padding_bytes() != size_t(0)) { + return absl::FailedPreconditionError("Num padding bytes should be 0"); + } // Confirm that the message payloads are equal, but not fully equivalent due // to padding. - ASSERT_THAT(deserialized_message, HasEqPayload(deserialized_padded_message)); - ASSERT_NE(deserialized_message, deserialized_padded_message); + if (!deserialized_message.IsPayloadEqual(deserialized_padded_message)) { + return absl::FailedPreconditionError("IsPayloadEqual failed"); + } + if (deserialized_message == deserialized_padded_message) { + return absl::FailedPreconditionError( + "deserialized_message == deserialized_padded_message"); + } + return absl::OkStatus(); } TEST(BinaryHttpRequest, Padding) { @@ -1841,7 +1924,7 @@ "curl/7.16.3 libcurl/7.16.3 OpenSSL/0.9.7l zlib/1.2.3"}) ->AddHeaderField({"Host", "www.example.com"}) ->AddHeaderField({"Accept-Language", "en, mi"}); - TestPadding(request); + QUICHE_EXPECT_OK(TestPadding(request)); } TEST(BinaryHttpResponse, Padding) { @@ -1854,7 +1937,7 @@ BinaryHttpResponse response(200); response.AddHeaderField({"Server", "Apache"}); response.set_body("Hello, world!\r\n"); - TestPadding(response); + QUICHE_EXPECT_OK(TestPadding(response)); } } // namespace quiche