From 6f82535f45697fe75cbc90b8ed8756eefa82eef3 Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Thu, 11 Aug 2016 15:38:28 -0700 Subject: [PATCH] Enabling IPv6 socket recv timestamp test, and making less flaky. The test worked by sleeping a certain time, then checking that the difference between recv timestamps before and after the sleep was within some margin of the requested sleep time. However, this means that imprecision of SleepMs makes the test flaky. This source of flakiness can be removed by comparing to the actual time slept instead of the requested time. Also making the margin larger, to further reduce the likelihood of flakiness. R=pthatcher@webrtc.org, stefan@webrtc.org Review URL: https://codereview.webrtc.org/2111043004 . Cr-Commit-Position: refs/heads/master@{#13733} --- webrtc/base/physicalsocketserver_unittest.cc | 12 +++------ webrtc/base/socket.h | 1 + webrtc/base/socket_unittest.cc | 28 ++++++++++++++------ webrtc/base/socket_unittest.h | 3 ++- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/webrtc/base/physicalsocketserver_unittest.cc b/webrtc/base/physicalsocketserver_unittest.cc index 6de8a90ea8..37d412dc1f 100644 --- a/webrtc/base/physicalsocketserver_unittest.cc +++ b/webrtc/base/physicalsocketserver_unittest.cc @@ -404,18 +404,14 @@ TEST_F(PhysicalSocketTest, TestGetSetOptionsIPv6) { #if defined(WEBRTC_POSIX) +// We don't get recv timestamps on Mac. #if !defined(WEBRTC_MAC) TEST_F(PhysicalSocketTest, TestSocketRecvTimestampIPv4) { - SocketTest::TestSocketRecvTimestamp(); + SocketTest::TestSocketRecvTimestampIPv4(); } -#if defined(WEBRTC_LINUX) -#define MAYBE_TestSocketRecvTimestampIPv6 DISABLED_TestSocketRecvTimestampIPv6 -#else -#define MAYBE_TestSocketRecvTimestampIPv6 TestSocketRecvTimestampIPv6 -#endif -TEST_F(PhysicalSocketTest, MAYBE_TestSocketRecvTimestampIPv6) { - SocketTest::TestSocketRecvTimestamp(); +TEST_F(PhysicalSocketTest, TestSocketRecvTimestampIPv6) { + SocketTest::TestSocketRecvTimestampIPv6(); } #endif diff --git a/webrtc/base/socket.h b/webrtc/base/socket.h index 7db9459de7..73dfdbb808 100644 --- a/webrtc/base/socket.h +++ b/webrtc/base/socket.h @@ -151,6 +151,7 @@ class Socket { virtual int Connect(const SocketAddress& addr) = 0; virtual int Send(const void *pv, size_t cb) = 0; virtual int SendTo(const void *pv, size_t cb, const SocketAddress& addr) = 0; + // |timestamp| is in units of microseconds. virtual int Recv(void* pv, size_t cb, int64_t* timestamp) = 0; virtual int RecvFrom(void* pv, size_t cb, diff --git a/webrtc/base/socket_unittest.cc b/webrtc/base/socket_unittest.cc index c206220a97..5dc52511e4 100644 --- a/webrtc/base/socket_unittest.cc +++ b/webrtc/base/socket_unittest.cc @@ -184,10 +184,15 @@ void SocketTest::TestGetSetOptionsIPv6() { GetSetOptionsInternal(kIPv6Loopback); } -void SocketTest::TestSocketRecvTimestamp() { +void SocketTest::TestSocketRecvTimestampIPv4() { SocketRecvTimestamp(kIPv4Loopback); } +void SocketTest::TestSocketRecvTimestampIPv6() { + MAYBE_SKIP_IPV6; + SocketRecvTimestamp(kIPv6Loopback); +} + // For unbound sockets, GetLocalAddress / GetRemoteAddress return AF_UNSPEC // values on Windows, but an empty address of the same family on Linux/MacOS X. bool IsUnspecOrEmptyIP(const IPAddress& address) { @@ -1033,19 +1038,26 @@ void SocketTest::SocketRecvTimestamp(const IPAddress& loopback) { EXPECT_EQ(0, socket->Bind(SocketAddress(loopback, 0))); SocketAddress address = socket->GetLocalAddress(); + uint64_t send_time_1 = TimeMicros(); socket->SendTo("foo", 3, address); - int64_t timestamp; + int64_t recv_timestamp_1; char buffer[3]; - socket->RecvFrom(buffer, 3, nullptr, ×tamp); - EXPECT_GT(timestamp, -1); - int64_t prev_timestamp = timestamp; + socket->RecvFrom(buffer, 3, nullptr, &recv_timestamp_1); + EXPECT_GT(recv_timestamp_1, -1); - const int64_t kTimeBetweenPacketsMs = 10; + const int64_t kTimeBetweenPacketsMs = 100; Thread::SleepMs(kTimeBetweenPacketsMs); + uint64_t send_time_2 = TimeMicros(); socket->SendTo("bar", 3, address); - socket->RecvFrom(buffer, 3, nullptr, ×tamp); - EXPECT_NEAR(timestamp, prev_timestamp + kTimeBetweenPacketsMs * 1000, 2000); + int64_t recv_timestamp_2; + socket->RecvFrom(buffer, 3, nullptr, &recv_timestamp_2); + + int64_t system_time_diff = send_time_2 - send_time_1; + int64_t recv_timestamp_diff = recv_timestamp_2 - recv_timestamp_1; + // Compare against the system time at the point of sending, because + // SleepMs may not sleep for exactly the requested time. + EXPECT_NEAR(system_time_diff, recv_timestamp_diff, 10000); } } // namespace rtc diff --git a/webrtc/base/socket_unittest.h b/webrtc/base/socket_unittest.h index 82b7ea5aaf..8172edd036 100644 --- a/webrtc/base/socket_unittest.h +++ b/webrtc/base/socket_unittest.h @@ -57,7 +57,8 @@ class SocketTest : public testing::Test { void TestUdpReadyToSendIPv6(); void TestGetSetOptionsIPv4(); void TestGetSetOptionsIPv6(); - void TestSocketRecvTimestamp(); + void TestSocketRecvTimestampIPv4(); + void TestSocketRecvTimestampIPv6(); static const int kTimeout = 5000; // ms const IPAddress kIPv4Loopback;