Run quartc_peer_test cases until bandwidth targets are met. Rather than running for an arbitrary time (eg. 10 seconds) to let bandwidth estimates ramp up, run the tests in quartc_peer_test.cc until the available bandwidth reported to each peer matches the simulated link's bandwidth (with a much longer timeout of 60 seconds, in case it never happens). This should make the test much less brittle, since the simulator run-time will now adjust automatically to small changes in bandwidth estimation behavior. gfe-relnote: n/a (Quartc tests only) PiperOrigin-RevId: 275285919 Change-Id: I6636391f5adc176a06f92004aab924d50e971eb5
diff --git a/quic/quartc/test/quartc_peer.cc b/quic/quartc/test/quartc_peer.cc index 9c2748d..885e6b1 100644 --- a/quic/quartc/test/quartc_peer.cc +++ b/quic/quartc/test/quartc_peer.cc
@@ -23,7 +23,8 @@ buffer_allocator_(buffer_allocator), enabled_(false), session_(nullptr), - configs_(configs) {} + configs_(configs), + last_available_(QuicBandwidth::Zero()) {} QuartcPeer::~QuartcPeer() { session_->CloseConnection("~QuartcPeer()"); @@ -87,6 +88,7 @@ // estimate, or it may explicitly subtract overhead before surfacing its // estimate. QuicBandwidth available = std::min(bandwidth_estimate, pacing_rate); + last_available_ = available; for (auto& source : data_sources_) { available = source->AllocateBandwidth(available); }
diff --git a/quic/quartc/test/quartc_peer.h b/quic/quartc/test/quartc_peer.h index 828b1e2..4906fd0 100644 --- a/quic/quartc/test/quartc_peer.h +++ b/quic/quartc/test/quartc_peer.h
@@ -70,6 +70,8 @@ // produced by that source. IdToSequenceNumberMap GetLastSequenceNumbers() const; + QuicBandwidth last_available_bandwidth() { return last_available_; } + // QuartcEndpoint::Delegate overrides. void OnSessionCreated(QuartcSession* session) override; @@ -116,6 +118,9 @@ // Messages received by this peer from the remote peer. Stored in the order // they are received. std::vector<ReceivedMessage> received_messages_; + + // Last available bandwidth. + QuicBandwidth last_available_; }; } // namespace test
diff --git a/quic/quartc/test/quartc_peer_test.cc b/quic/quartc/test/quartc_peer_test.cc index a9fa721..04e317b 100644 --- a/quic/quartc/test/quartc_peer_test.cc +++ b/quic/quartc/test/quartc_peer_test.cc
@@ -67,6 +67,18 @@ client_endpoint_->Connect(&client_transport_); } + void RampUpBandwidth() { + // Run long enough for the bandwidth estimate to ramp up. + simulator_.RunUntilOrTimeout( + [this] { + return client_peer_->last_available_bandwidth() == + client_server_link_.bandwidth() && + server_peer_->last_available_bandwidth() == + client_server_link_.bandwidth(); + }, + QuicTime::Delta::FromSeconds(60)); + } + SimpleRandom rng_; simulator::Simulator simulator_; simulator::SimulatedQuartcPacketTransport client_transport_; @@ -131,9 +143,7 @@ CreatePeers({config}); Connect(); - - // Run long enough for the bandwidth estimate to ramp up. - simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); + RampUpBandwidth(); // The peers generate frames that fit in one packet. EXPECT_LT(client_peer_->received_messages().back().frame.size, @@ -150,9 +160,7 @@ CreatePeers({config}); Connect(); - - // Run long enough for the bandwidth estimate to ramp up. - simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); + RampUpBandwidth(); // The peers generate frames that fit in one packet. EXPECT_LT(client_peer_->received_messages().back().frame.size, @@ -171,9 +179,7 @@ CreatePeers({config}); Connect(); - - // Run long enough for the bandwidth estimate to ramp up. - simulator_.RunFor(QuicTime::Delta::FromSeconds(12)); + RampUpBandwidth(); EXPECT_EQ(client_peer_->received_messages().back().frame.size, 100u); EXPECT_EQ(server_peer_->received_messages().back().frame.size, 100u); @@ -186,9 +192,7 @@ CreatePeers({config}); Connect(); - - // Run long enough for the bandwidth estimate to ramp up. - simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); + RampUpBandwidth(); // Max frame sizes smaller than the header are ignored, and the frame size is // limited by packet size. @@ -274,10 +278,7 @@ CreatePeers({config_1, config_2, config_3}); Connect(); - - // Run for long enough that bandwidth ramps up and meets the requirements of - // all three sources. - simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); + RampUpBandwidth(); // The last message from each source should be the size allowed by that // source's maximum bandwidth and frame interval. @@ -327,9 +328,7 @@ CreatePeers({config_1, config_2, config_3}); Connect(); - - // Run for long enough that bandwidth ramps up to link capacity. - simulator_.RunFor(QuicTime::Delta::FromSeconds(12)); + RampUpBandwidth(); const std::vector<ReceivedMessage>& client_messages = client_peer_->received_messages(); @@ -377,6 +376,7 @@ CreatePeers({config}); Connect(); + // Note: this time is completely arbitrary, to allow messages to be sent. simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); // After these calls, we should observe no new messages. @@ -388,15 +388,22 @@ std::map<int32_t, int64_t> last_sent_by_server = server_peer_->GetLastSequenceNumbers(); + // Note: this time is completely arbitrary, to allow time for the peers to + // generate new messages after being disabled. The point of the test is that + // they should not do that. simulator_.RunFor(QuicTime::Delta::FromSeconds(10)); - ASSERT_FALSE(client_peer_->received_messages().empty()); - EXPECT_EQ(client_peer_->received_messages().back().frame.sequence_number, - last_sent_by_server[1]); - - ASSERT_FALSE(server_peer_->received_messages().empty()); - EXPECT_EQ(server_peer_->received_messages().back().frame.sequence_number, - last_sent_by_client[1]); + // Messages sent prior to disabling the peers are eventually received. + EXPECT_TRUE(simulator_.RunUntilOrTimeout( + [this, last_sent_by_client, last_sent_by_server]() mutable -> bool { + return !client_peer_->received_messages().empty() && + client_peer_->received_messages().back().frame.sequence_number == + last_sent_by_server[1] && + !server_peer_->received_messages().empty() && + server_peer_->received_messages().back().frame.sequence_number == + last_sent_by_client[1]; + }, + QuicTime::Delta::FromSeconds(60))); } } // namespace