From ec499beaf53409bbdc67dcda48cfc29ade2afa32 Mon Sep 17 00:00:00 2001 From: "jlmiller@webrtc.org" Date: Sat, 7 Feb 2015 22:37:59 +0000 Subject: [PATCH] Increase testclient timeout from 1 to 5 seconds BUG=4182 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/38839004 Cr-Commit-Position: refs/heads/master@{#8285} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8285 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/base/testclient.cc | 14 +++++++------- webrtc/base/testclient.h | 10 +++++++--- webrtc/base/virtualsocket_unittest.cc | 4 ++-- webrtc/p2p/base/relayserver_unittest.cc | 25 +++++++++++++++---------- webrtc/p2p/base/stunserver_unittest.cc | 9 ++++++--- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/webrtc/base/testclient.cc b/webrtc/base/testclient.cc index 32670e21a9..8483c4e8f4 100644 --- a/webrtc/base/testclient.cc +++ b/webrtc/base/testclient.cc @@ -34,7 +34,7 @@ TestClient::~TestClient() { bool TestClient::CheckConnState(AsyncPacketSocket::State state) { // Wait for our timeout value until the socket reaches the desired state. - uint32 end = TimeAfter(kTimeout); + uint32 end = TimeAfter(kTimeoutMs); while (socket_->GetState() != state && TimeUntil(end) > 0) Thread::Current()->ProcessMessages(1); return (socket_->GetState() == state); @@ -51,10 +51,10 @@ int TestClient::SendTo(const char* buf, size_t size, return socket_->SendTo(buf, size, dest, options); } -TestClient::Packet* TestClient::NextPacket() { +TestClient::Packet* TestClient::NextPacket(int timeout_ms) { // If no packets are currently available, we go into a get/dispatch loop for - // at most 1 second. If, during the loop, a packet arrives, then we can stop - // early and return it. + // at most timeout_ms. If, during the loop, a packet arrives, then we can + // stop early and return it. // Note that the case where no packet arrives is important. We often want to // test that a packet does not arrive. @@ -63,7 +63,7 @@ TestClient::Packet* TestClient::NextPacket() { // Pumping another thread's queue could lead to messages being dispatched from // the wrong thread to non-thread-safe objects. - uint32 end = TimeAfter(kTimeout); + uint32 end = TimeAfter(timeout_ms); while (TimeUntil(end) > 0) { { CritScope cs(&crit_); @@ -88,7 +88,7 @@ TestClient::Packet* TestClient::NextPacket() { bool TestClient::CheckNextPacket(const char* buf, size_t size, SocketAddress* addr) { bool res = false; - Packet* packet = NextPacket(); + Packet* packet = NextPacket(kTimeoutMs); if (packet) { res = (packet->size == size && memcmp(packet->buf, buf, size) == 0); if (addr) @@ -100,7 +100,7 @@ bool TestClient::CheckNextPacket(const char* buf, size_t size, bool TestClient::CheckNoPacket() { bool res; - Packet* packet = NextPacket(); + Packet* packet = NextPacket(kNoPacketTimeoutMs); res = (packet == NULL); delete packet; return res; diff --git a/webrtc/base/testclient.h b/webrtc/base/testclient.h index d56f948b04..52058e30b3 100644 --- a/webrtc/base/testclient.h +++ b/webrtc/base/testclient.h @@ -32,6 +32,9 @@ class TestClient : public sigslot::has_slots<> { size_t size; }; + // Default timeout for NextPacket reads. + static const int kTimeoutMs = 5000; + // Creates a client that will send and receive with the given socket and // will post itself messages with the given thread. explicit TestClient(AsyncPacketSocket* socket); @@ -55,9 +58,9 @@ class TestClient : public sigslot::has_slots<> { int SendTo(const char* buf, size_t size, const SocketAddress& dest); // Returns the next packet received by the client or 0 if none is received - // within a reasonable amount of time. The caller must delete the packet + // within the specified timeout. The caller must delete the packet // when done with it. - Packet* NextPacket(); + Packet* NextPacket(int timeout_ms); // Checks that the next packet has the given contents. Returns the remote // address that the packet was sent from. @@ -72,7 +75,8 @@ class TestClient : public sigslot::has_slots<> { bool ready_to_send() const; private: - static const int kTimeout = 1000; + // Timeout for reads when no packet is expected. + static const int kNoPacketTimeoutMs = 1000; // Workaround for the fact that AsyncPacketSocket::GetConnState doesn't exist. Socket::ConnState GetState(); // Slot for packets read on the socket. diff --git a/webrtc/base/virtualsocket_unittest.cc b/webrtc/base/virtualsocket_unittest.cc index d9d4de1c4d..55613a7771 100644 --- a/webrtc/base/virtualsocket_unittest.cc +++ b/webrtc/base/virtualsocket_unittest.cc @@ -736,7 +736,7 @@ class VirtualSocketServerTest : public testing::Test { // Test cross-family datagram sending between a client bound to client_addr // and a server bound to server_addr. shouldSucceed indicates if sending is - // expected to succed or not. + // expected to succeed or not. void CrossFamilyDatagramTest(const SocketAddress& client_addr, const SocketAddress& server_addr, bool shouldSucceed) { @@ -759,7 +759,7 @@ class VirtualSocketServerTest : public testing::Test { EXPECT_EQ(client1_addr, bound_server_addr); } else { EXPECT_EQ(-1, client2->SendTo("foo", 3, bound_server_addr)); - EXPECT_FALSE(client1->CheckNextPacket("foo", 3, 0)); + EXPECT_TRUE(client1->CheckNoPacket()); } } diff --git a/webrtc/p2p/base/relayserver_unittest.cc b/webrtc/p2p/base/relayserver_unittest.cc index afc8a403b4..4f1164acc6 100644 --- a/webrtc/p2p/base/relayserver_unittest.cc +++ b/webrtc/p2p/base/relayserver_unittest.cc @@ -96,6 +96,13 @@ class RelayServerTest : public testing::Test { client->SendTo(data, len, addr); } + bool Receive1Fails() { + return client1_.get()->CheckNoPacket(); + } + bool Receive2Fails() { + return client2_.get()->CheckNoPacket(); + } + StunMessage* Receive1() { return Receive(client1_.get()); } @@ -110,7 +117,8 @@ class RelayServerTest : public testing::Test { } StunMessage* Receive(rtc::TestClient* client) { StunMessage* msg = NULL; - rtc::TestClient::Packet* packet = client->NextPacket(); + rtc::TestClient::Packet* packet = + client->NextPacket(rtc::TestClient::kTimeoutMs); if (packet) { rtc::ByteBuffer buf(packet->buf, packet->size); msg = new RelayMessage(); @@ -121,7 +129,8 @@ class RelayServerTest : public testing::Test { } std::string ReceiveRaw(rtc::TestClient* client) { std::string raw; - rtc::TestClient::Packet* packet = client->NextPacket(); + rtc::TestClient::Packet* packet = + client->NextPacket(rtc::TestClient::kTimeoutMs); if (packet) { raw = std::string(packet->buf, packet->size); delete packet; @@ -174,12 +183,8 @@ class RelayServerTest : public testing::Test { // Send a complete nonsense message and verify that it is eaten. TEST_F(RelayServerTest, TestBadRequest) { - rtc::scoped_ptr res; - SendRaw1(bad, static_cast(strlen(bad))); - res.reset(Receive1()); - - ASSERT_TRUE(!res); + ASSERT_TRUE(Receive1Fails()); } // Send an allocate request without a username and verify it is rejected. @@ -310,7 +315,7 @@ TEST_F(RelayServerTest, TestRemoteBind) { EXPECT_EQ(client2_addr.ipaddr(), src_addr->ipaddr()); EXPECT_EQ(client2_addr.port(), src_addr->port()); - EXPECT_TRUE(Receive2() == NULL); + EXPECT_TRUE(Receive2Fails()); } // Send a complete nonsense message to the established connection and verify @@ -320,8 +325,8 @@ TEST_F(RelayServerTest, TestRemoteBadRequest) { Bind(); SendRaw1(bad, static_cast(strlen(bad))); - EXPECT_TRUE(Receive1() == NULL); - EXPECT_TRUE(Receive2() == NULL); + EXPECT_TRUE(Receive1Fails()); + EXPECT_TRUE(Receive2Fails()); } // Send a send request without a username and verify it is rejected. diff --git a/webrtc/p2p/base/stunserver_unittest.cc b/webrtc/p2p/base/stunserver_unittest.cc index 7266eae862..d405979064 100644 --- a/webrtc/p2p/base/stunserver_unittest.cc +++ b/webrtc/p2p/base/stunserver_unittest.cc @@ -46,9 +46,13 @@ class StunServerTest : public testing::Test { void Send(const char* buf, int len) { client_->SendTo(buf, len, server_addr); } + bool ReceiveFails() { + return(client_->CheckNoPacket()); + } StunMessage* Receive() { StunMessage* msg = NULL; - rtc::TestClient::Packet* packet = client_->NextPacket(); + rtc::TestClient::Packet* packet = + client_->NextPacket(rtc::TestClient::kTimeoutMs); if (packet) { rtc::ByteBuffer buf(packet->buf, packet->size); msg = new StunMessage(); @@ -104,6 +108,5 @@ TEST_F(StunServerTest, TestBad) { "look anything like a normal stun message"; Send(bad, static_cast(strlen(bad))); - StunMessage* msg = Receive(); - ASSERT_TRUE(msg == NULL); + ASSERT_TRUE(ReceiveFails()); }