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

@@ -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;

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.

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());
     }
   }
 

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<StunMessage> res;
-
   SendRaw1(bad, static_cast<int>(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<int>(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.

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<int>(strlen(bad)));
 
-  StunMessage* msg = Receive();
-  ASSERT_TRUE(msg == NULL);
+  ASSERT_TRUE(ReceiveFails());
 }