gfe-relnote: various test code cleanup. Not protected. PiperOrigin-RevId: 277363244 Change-Id: I99a46b448b57e75ae807844337f013dbbbf6ea98
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 2c14d12..b22f00d 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -352,10 +352,9 @@ void CheckClosedStreams() { QuicStreamId first_stream_id = QuicUtils::GetFirstBidirectionalStreamId( - connection_->transport_version(), Perspective::IS_CLIENT); - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { - first_stream_id = - QuicUtils::GetCryptoStreamId(connection_->transport_version()); + transport_version(), Perspective::IS_CLIENT); + if (!QuicVersionUsesCryptoFrames(transport_version())) { + first_stream_id = QuicUtils::GetCryptoStreamId(transport_version()); } for (QuicStreamId i = first_stream_id; i < 100; i++) { if (!QuicContainsKey(closed_streams_, i)) { @@ -390,12 +389,11 @@ } QuicStreamId GetNthServerInitiatedBidirectionalId(int n) { - return GetNthServerInitiatedBidirectionalStreamId( - connection_->transport_version(), n); + return GetNthServerInitiatedBidirectionalStreamId(transport_version(), n); } QuicStreamId IdDelta() { - return QuicUtils::StreamIdDelta(connection_->transport_version()); + return QuicUtils::StreamIdDelta(transport_version()); } std::string EncodeSettings(const SettingsFrame& settings) { @@ -473,10 +471,9 @@ TEST_P(QuicSpdySessionTestServer, IsClosedStreamDefault) { // Ensure that no streams are initially closed. QuicStreamId first_stream_id = QuicUtils::GetFirstBidirectionalStreamId( - connection_->transport_version(), Perspective::IS_CLIENT); - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { - first_stream_id = - QuicUtils::GetCryptoStreamId(connection_->transport_version()); + transport_version(), Perspective::IS_CLIENT); + if (!QuicVersionUsesCryptoFrames(transport_version())) { + first_stream_id = QuicUtils::GetCryptoStreamId(transport_version()); } for (QuicStreamId i = first_stream_id; i < 100; i++) { EXPECT_FALSE(session_.IsClosedStream(i)) << "stream id: " << i; @@ -850,7 +847,7 @@ // This tests prioritization of the crypto stream when flow control limits are // reached. When CRYPTO frames are in use, there is no flow control for the // crypto handshake, so this test is irrelevant. - if (QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (QuicVersionUsesCryptoFrames(transport_version())) { return; } session_.set_writev_consumes_all_data(true); @@ -867,7 +864,7 @@ // Blocking (due to buffering of) the Crypto stream is detected. session_.MarkConnectionLevelWriteBlocked( - QuicUtils::GetCryptoStreamId(connection_->transport_version())); + QuicUtils::GetCryptoStreamId(transport_version())); EXPECT_TRUE(session_.HasPendingHandshake()); TestStream* stream4 = session_.CreateOutgoingBidirectionalStream(); @@ -940,9 +937,9 @@ // Mark the crypto and headers streams as write blocked, we expect them to be // allowed to write later. - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (!QuicVersionUsesCryptoFrames(transport_version())) { session_.MarkConnectionLevelWriteBlocked( - QuicUtils::GetCryptoStreamId(connection_->transport_version())); + QuicUtils::GetCryptoStreamId(transport_version())); } // Create a data stream, and although it is write blocked we never expect it @@ -953,18 +950,18 @@ // The crypto and headers streams should be called even though we are // connection flow control blocked. - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (!QuicVersionUsesCryptoFrames(transport_version())) { TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream(); EXPECT_CALL(*crypto_stream, OnCanWrite()); } - if (!VersionUsesHttp3(connection_->transport_version())) { + if (!VersionUsesHttp3(transport_version())) { TestHeadersStream* headers_stream; QuicSpdySessionPeer::SetHeadersStream(&session_, nullptr); headers_stream = new TestHeadersStream(&session_); QuicSpdySessionPeer::SetHeadersStream(&session_, headers_stream); session_.MarkConnectionLevelWriteBlocked( - QuicUtils::GetHeadersStreamId(connection_->transport_version())); + QuicUtils::GetHeadersStreamId(transport_version())); EXPECT_CALL(*headers_stream, OnCanWrite()); } @@ -1127,7 +1124,7 @@ QuicStreamFrame data1(id, false, 0, QuicStringPiece(type, 1)); session_.OnStreamFrame(data1); } else { - id = QuicUtils::GetHeadersStreamId(connection_->transport_version()); + id = QuicUtils::GetHeadersStreamId(transport_version()); } // Send two bytes of payload. @@ -1149,7 +1146,7 @@ QuicStreamFrame data1(id, false, 0, QuicStringPiece(type, 1)); session_.OnStreamFrame(data1); } else { - id = QuicUtils::GetHeadersStreamId(connection_->transport_version()); + id = QuicUtils::GetHeadersStreamId(transport_version()); } // Send two bytes of payload. @@ -1164,9 +1161,8 @@ TEST_P(QuicSpdySessionTestServer, OnStreamFrameInvalidStreamId) { // Send two bytes of payload. - QuicStreamFrame data1( - QuicUtils::GetInvalidStreamId(connection_->transport_version()), true, 0, - QuicStringPiece("HT")); + QuicStreamFrame data1(QuicUtils::GetInvalidStreamId(transport_version()), + true, 0, QuicStringPiece("HT")); EXPECT_CALL(*connection_, CloseConnection( QUIC_INVALID_STREAM_ID, "Received data for an invalid stream", @@ -1176,10 +1172,9 @@ TEST_P(QuicSpdySessionTestServer, OnRstStreamInvalidStreamId) { // Send two bytes of payload. - QuicRstStreamFrame rst1( - kInvalidControlFrameId, - QuicUtils::GetInvalidStreamId(connection_->transport_version()), - QUIC_ERROR_PROCESSING_STREAM, 0); + QuicRstStreamFrame rst1(kInvalidControlFrameId, + QuicUtils::GetInvalidStreamId(transport_version()), + QUIC_ERROR_PROCESSING_STREAM, 0); EXPECT_CALL(*connection_, CloseConnection( QUIC_INVALID_STREAM_ID, "Received data for an invalid stream", @@ -1225,7 +1220,7 @@ TEST_P(QuicSpdySessionTestServer, HandshakeUnblocksFlowControlBlockedCryptoStream) { - if (QuicVersionUsesCryptoFrames(GetParam().transport_version)) { + if (QuicVersionUsesCryptoFrames(transport_version())) { // QUIC version 47 onwards uses CRYPTO frames for the handshake, so this // test doesn't make sense for those versions. return; @@ -1269,8 +1264,7 @@ CryptoHandshakeMessage msg; session_.GetMutableCryptoStream()->OnHandshakeMessage(msg); EXPECT_TRUE(QuicSessionPeer::IsStreamWriteBlocked( - &session_, - QuicUtils::GetCryptoStreamId(connection_->transport_version()))); + &session_, QuicUtils::GetCryptoStreamId(transport_version()))); // Stream is now unblocked and will no longer have buffered data. EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked()); EXPECT_FALSE(session_.IsConnectionFlowControlBlocked()); @@ -1287,7 +1281,7 @@ HandshakeUnblocksFlowControlBlockedHeadersStream) { // This test depends on stream-level flow control for the crypto stream, which // doesn't exist when CRYPTO frames are used. - if (QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (QuicVersionUsesCryptoFrames(transport_version())) { return; } @@ -1348,8 +1342,7 @@ EXPECT_FALSE(session_.IsStreamFlowControlBlocked()); EXPECT_TRUE(headers_stream->HasBufferedData()); EXPECT_TRUE(QuicSessionPeer::IsStreamWriteBlocked( - &session_, - QuicUtils::GetHeadersStreamId(connection_->transport_version()))); + &session_, QuicUtils::GetHeadersStreamId(transport_version()))); } #endif // !defined(OS_IOS) @@ -1573,7 +1566,7 @@ } TEST_P(QuicSpdySessionTestServer, WindowUpdateUnblocksHeadersStream) { - if (VersionUsesHttp3(GetParam().transport_version)) { + if (VersionUsesHttp3(transport_version())) { // The test relies on headers stream, which no longer exists in IETF QUIC. return; } @@ -1627,8 +1620,7 @@ GetNthClientInitiatedBidirectionalId(kMaxStreams); // Create kMaxStreams data streams, and close them all without receiving a // FIN or a RST_STREAM from the client. - const QuicStreamId kNextId = - QuicUtils::StreamIdDelta(connection_->transport_version()); + const QuicStreamId kNextId = QuicUtils::StreamIdDelta(transport_version()); for (QuicStreamId i = kFirstStreamId; i < kFinalStreamId; i += kNextId) { QuicStreamFrame data1(i, false, 0, QuicStringPiece("HT")); session_.OnStreamFrame(data1); @@ -1969,16 +1961,15 @@ // Lost data on cryption stream, streams 2 and 4. EXPECT_CALL(*stream4, HasPendingRetransmission()).WillOnce(Return(true)); - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (!QuicVersionUsesCryptoFrames(transport_version())) { EXPECT_CALL(*crypto_stream, HasPendingRetransmission()) .WillOnce(Return(true)); } EXPECT_CALL(*stream2, HasPendingRetransmission()).WillOnce(Return(true)); session_.OnFrameLost(QuicFrame(frame3)); - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { - QuicStreamFrame frame1( - QuicUtils::GetCryptoStreamId(connection_->transport_version()), false, - 0, 1300); + if (!QuicVersionUsesCryptoFrames(transport_version())) { + QuicStreamFrame frame1(QuicUtils::GetCryptoStreamId(transport_version()), + false, 0, 1300); session_.OnFrameLost(QuicFrame(frame1)); } else { QuicCryptoFrame crypto_frame(ENCRYPTION_INITIAL, 0, 1300); @@ -1995,7 +1986,7 @@ // stream go first. // Do not check congestion window when crypto stream has lost data. EXPECT_CALL(*send_algorithm, CanSend(_)).Times(0); - if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + if (!QuicVersionUsesCryptoFrames(transport_version())) { EXPECT_CALL(*crypto_stream, OnCanWrite()); EXPECT_CALL(*crypto_stream, HasPendingRetransmission()) .WillOnce(Return(false));
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index c080982..26a6acb 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -349,10 +349,7 @@ stream_->OnStreamHeadersPriority( spdy::SpdyStreamPrecedence(kV3HighestPriority)); - const bool version_uses_qpack = - VersionUsesHttp3(GetParam().transport_version); - - if (version_uses_qpack) { + if (UsesHttp3()) { EXPECT_CALL( *connection_, CloseConnection( @@ -365,7 +362,7 @@ stream_->OnStreamHeaderList(false, 1 << 20, headers); - if (!version_uses_qpack) { + if (!UsesHttp3()) { EXPECT_EQ(QUIC_HEADERS_TOO_LARGE, stream_->stream_error()); } } @@ -1026,7 +1023,7 @@ trailers_block["key2"] = "value2"; trailers_block["key3"] = "value3"; SpdyHeaderBlock trailers_block_with_final_offset = trailers_block.Clone(); - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { // :final-offset pseudo-header is only added if trailers are sent // on the headers stream. trailers_block_with_final_offset[kFinalOffsetHeaderKey] = "0"; @@ -1054,7 +1051,7 @@ TEST_P(QuicSpdyStreamTest, ReceivingTrailersWithOffset) { // kFinalOffsetHeaderKey is not used when HEADERS are sent on the // request/response stream. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1101,7 +1098,7 @@ TEST_P(QuicSpdyStreamTest, ReceivingTrailersWithoutOffset) { // kFinalOffsetHeaderKey is not used when HEADERS are sent on the // request/response stream. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1133,7 +1130,7 @@ TEST_P(QuicSpdyStreamTest, ReceivingTrailersWithoutFin) { // In IETF QUIC, there is no such thing as FIN flag on HTTP/3 frames like the // HEADERS frame. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1164,7 +1161,7 @@ // If HEADERS frames are sent on the request/response stream, then the // sequencer will signal an error if any stream data arrives after a FIN, // so QuicSpdyStream does not need to. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1186,7 +1183,7 @@ // If HEADERS frames are sent on the request/response stream, // then the sequencer will block them from reaching QuicSpdyStream // after the stream is closed. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1236,7 +1233,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersSendsAFin) { Initialize(kShouldProcessData); - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { // In this case, TestStream::WriteHeadersImpl() does not prevent writes. EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)) .Times(AtLeast(1)); @@ -1258,7 +1255,7 @@ SetQuicFlag(FLAGS_quic_allow_http3_priority, true); InitializeWithPerspective(kShouldProcessData, Perspective::IS_CLIENT); - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { // In this case, TestStream::WriteHeadersImpl() does not prevent writes. // Six writes include priority for headers, headers frame header, headers // frame, priority of trailers, trailing headers frame header, and trailers. @@ -1289,7 +1286,7 @@ TEST_P(QuicSpdyStreamTest, WritingTrailersFinalOffset) { Initialize(kShouldProcessData); - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { // In this case, TestStream::WriteHeadersImpl() does not prevent writes. EXPECT_CALL(*session_, WritevData(stream_, stream_->id(), _, _, _)) .Times(AtLeast(1)); @@ -1318,7 +1315,7 @@ SpdyHeaderBlock expected_trailers(trailers.Clone()); // :final-offset pseudo-header is only added if trailers are sent // on the headers stream. - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { expected_trailers[kFinalOffsetHeaderKey] = QuicTextUtils::Uint64ToString(body.length() + header_length); } @@ -1360,7 +1357,7 @@ // This test exercises sending trailers on the headers stream while data is // still queued on the response/request stream. In IETF QUIC, data and // trailers are sent on the same stream, so this test does not apply. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1419,7 +1416,7 @@ TEST_P(QuicSpdyStreamTest, HeaderStreamNotiferCorrespondingSpdyStream) { // There is no headers stream if QPACK is used. - if (VersionUsesHttp3(GetParam().transport_version)) { + if (UsesHttp3()) { return; } @@ -1752,7 +1749,7 @@ } TEST_P(QuicSpdyStreamTest, HeadersFrameOnRequestStream) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -1780,7 +1777,7 @@ } TEST_P(QuicSpdyStreamTest, ProcessBodyAfterTrailers) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -1824,7 +1821,7 @@ // normal body. Make sure the http decoder stops processing body after the // connection shuts down. TEST_P(QuicSpdyStreamTest, MalformedHeadersStopHttpDecoder) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -1866,7 +1863,7 @@ } TEST_P(QuicSpdyStreamTest, ImmediateHeaderDecodingWithDynamicTableEntries) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -1921,7 +1918,7 @@ } TEST_P(QuicSpdyStreamTest, BlockedHeaderDecoding) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -1978,7 +1975,7 @@ } TEST_P(QuicSpdyStreamTest, AsyncErrorDecodingHeaders) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2010,7 +2007,7 @@ } TEST_P(QuicSpdyStreamTest, AsyncErrorDecodingTrailers) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2120,7 +2117,7 @@ // Test that stream bytes are consumed (by calling // sequencer()->MarkConsumed()) incrementally, as soon as possible. TEST_P(QuicSpdyStreamIncrementalConsumptionTest, OnlyKnownFrames) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2179,7 +2176,7 @@ } TEST_P(QuicSpdyStreamIncrementalConsumptionTest, UnknownFramesInterleaved) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2285,7 +2282,7 @@ // Close connection if a DATA frame is received before a HEADERS frame. TEST_P(QuicSpdyStreamTest, DataBeforeHeaders) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2307,7 +2304,7 @@ // Close connection if a HEADERS frame is received after the trailing HEADERS. TEST_P(QuicSpdyStreamTest, TrailersAfterTrailers) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2360,7 +2357,7 @@ // Regression test for https://crbug.com/978733. // Close connection if a DATA frame is received after the trailing HEADERS. TEST_P(QuicSpdyStreamTest, DataAfterTrailers) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; } @@ -2410,7 +2407,7 @@ // SETTINGS frames are invalid on bidirectional streams. If one is received, // the connection is closed. No more data should be processed. TEST_P(QuicSpdyStreamTest, StopProcessingIfConnectionClosed) { - if (!VersionUsesHttp3(GetParam().transport_version)) { + if (!UsesHttp3()) { return; }
diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc index 8623152..bb1cf19 100644 --- a/quic/tools/quic_simple_server_session_test.cc +++ b/quic/tools/quic_simple_server_session_test.cc
@@ -288,7 +288,8 @@ INSTANTIATE_TEST_SUITE_P(Tests, QuicSimpleServerSessionTest, - ::testing::ValuesIn(AllSupportedVersions())); + ::testing::ValuesIn(AllSupportedVersions()), + ::testing::PrintToStringParamName()); TEST_P(QuicSimpleServerSessionTest, CloseStreamDueToReset) { // Open a stream, then reset it.