Update VirtualSocketServerTest to use a fake clock.

Since this is a test for a fake network, it's only natural that it uses
a fake clock as well. This makes the tests much faster, less flaky, and
lets them be moved out of  "webrtc_nonparallel_tests", since they no
longer have a dependency on any "real" thing (sockets, or time) and
can be run in parallel as easily as any other tests.

As part of this CL, added the fake clock as an argument to
VirtualSocketServer's and TestClient's constructors, since these classes
have methods that wait synchronously for something to occur, and if the
test is using a fake clock, they need to advance it in order to make
progress.

Lastly, added a DCHECK in Thread::ProcessMessages. If called with a
nonzero time while a fake clock is used, it will get stuck in an
infinite loop; a DCHECK is easier to notice than an infinite loop.

BUG=webrtc:7727, webrtc:2409

Review-Url: https://codereview.webrtc.org/2927413002
Cr-Commit-Position: refs/heads/master@{#18544}
This commit is contained in:
deadbeef
2017-06-12 14:30:28 -07:00
committed by Commit Bot
parent 36b1a5fcec
commit 22e0814d51
9 changed files with 101 additions and 37 deletions

View File

@ -532,9 +532,6 @@ rtc_static_library("rtc_base") {
"optionsfile.h",
"rollingaccumulator.h",
"sslroots.h",
"testbase64.h",
"testclient.cc",
"testclient.h",
"transformadapter.cc",
"transformadapter.h",
"window.h",
@ -707,6 +704,8 @@ rtc_source_set("rtc_base_tests_utils") {
"sigslottester.h",
"sigslottester.h.pump",
"testbase64.h",
"testclient.cc",
"testclient.h",
"testechoserver.h",
"testutils.h",
"timedelta.h",
@ -774,7 +773,6 @@ if (rtc_include_tests) {
"socket_unittest.cc",
"socket_unittest.h",
"socketaddress_unittest.cc",
"virtualsocket_unittest.cc",
]
deps = [
":rtc_base",
@ -843,6 +841,7 @@ if (rtc_include_tests) {
"thread_checker_unittest.cc",
"timestampaligner_unittest.cc",
"timeutils_unittest.cc",
"virtualsocket_unittest.cc",
]
deps = [
":rtc_base",

View File

@ -9,6 +9,8 @@
*/
#include "webrtc/base/testclient.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/ptr_util.h"
#include "webrtc/base/thread.h"
#include "webrtc/base/timeutils.h"
@ -20,7 +22,13 @@ namespace rtc {
// NextPacket.
TestClient::TestClient(std::unique_ptr<AsyncPacketSocket> socket)
: socket_(std::move(socket)), prev_packet_timestamp_(-1) {
: TestClient(std::move(socket), nullptr) {}
TestClient::TestClient(std::unique_ptr<AsyncPacketSocket> socket,
FakeClock* fake_clock)
: fake_clock_(fake_clock),
socket_(std::move(socket)),
prev_packet_timestamp_(-1) {
socket_->SignalReadPacket.connect(this, &TestClient::OnPacket);
socket_->SignalReadyToSend.connect(this, &TestClient::OnReadyToSend);
}
@ -31,7 +39,7 @@ bool TestClient::CheckConnState(AsyncPacketSocket::State state) {
// Wait for our timeout value until the socket reaches the desired state.
int64_t end = TimeAfter(kTimeoutMs);
while (socket_->GetState() != state && TimeUntil(end) > 0) {
Thread::Current()->ProcessMessages(1);
AdvanceTime(1);
}
return (socket_->GetState() == state);
}
@ -67,7 +75,7 @@ std::unique_ptr<TestClient::Packet> TestClient::NextPacket(int timeout_ms) {
break;
}
}
Thread::Current()->ProcessMessages(1);
AdvanceTime(1);
}
// Return the first packet placed in the queue.
@ -108,6 +116,16 @@ bool TestClient::CheckTimestamp(int64_t packet_timestamp) {
return res;
}
void TestClient::AdvanceTime(int ms) {
// If the test is using a fake clock, we must advance the fake clock to
// advance time. Otherwise, ProcessMessages will work.
if (fake_clock_) {
SIMULATED_WAIT(false, ms, *fake_clock_);
} else {
Thread::Current()->ProcessMessages(1);
}
}
bool TestClient::CheckNoPacket() {
return NextPacket(kNoPacketTimeoutMs) == nullptr;
}

View File

@ -16,6 +16,7 @@
#include "webrtc/base/asyncudpsocket.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/fakeclock.h"
namespace rtc {
@ -44,6 +45,10 @@ class TestClient : public sigslot::has_slots<> {
// Creates a client that will send and receive with the given socket and
// will post itself messages with the given thread.
explicit TestClient(std::unique_ptr<AsyncPacketSocket> socket);
// Create a test client that will use a fake clock. NextPacket needs to wait
// for a packet to be received, and thus it needs to advance the fake clock
// if the test is using one, rather than just sleeping.
TestClient(std::unique_ptr<AsyncPacketSocket> socket, FakeClock* fake_clock);
~TestClient() override;
SocketAddress address() const { return socket_->GetLocalAddress(); }
@ -93,7 +98,9 @@ class TestClient : public sigslot::has_slots<> {
const PacketTime& packet_time);
void OnReadyToSend(AsyncPacketSocket* socket);
bool CheckTimestamp(int64_t packet_timestamp);
void AdvanceTime(int ms);
FakeClock* fake_clock_ = nullptr;
CriticalSection crit_;
std::unique_ptr<AsyncPacketSocket> socket_;
std::vector<std::unique_ptr<Packet>> packets_;

View File

@ -471,6 +471,11 @@ void Thread::Clear(MessageHandler* phandler,
// Note that these methods have a separate implementation for mac and ios
// defined in webrtc/base/thread_darwin.mm.
bool Thread::ProcessMessages(int cmsLoop) {
// Using ProcessMessages with a custom clock for testing and a time greater
// than 0 doesn't work, since it's not guaranteed to advance the custom
// clock's time, and may get stuck in an infinite loop.
RTC_DCHECK(GetClockForTesting() == nullptr || cmsLoop == 0 ||
cmsLoop == kForever);
int64_t msEnd = (kForever == cmsLoop) ? 0 : TimeAfter(cmsLoop);
int cmsNext = cmsLoop;

View File

@ -39,6 +39,10 @@ ClockInterface* SetClockForTesting(ClockInterface* clock) {
return prev;
}
ClockInterface* GetClockForTesting() {
return g_clock;
}
int64_t SystemTimeNanos() {
int64_t ticks;
#if defined(WEBRTC_MAC)

View File

@ -53,6 +53,9 @@ class ClockInterface {
// that uses it, eliminating the need for a global variable and this function.
ClockInterface* SetClockForTesting(ClockInterface* clock);
// Returns previously set clock, or nullptr if no custom clock is being used.
ClockInterface* GetClockForTesting();
// Returns the actual system time, even if a clock is set for testing.
// Useful for timeouts while using a test clock, or for logging.
int64_t SystemTimeNanos();

View File

@ -17,6 +17,7 @@
#include <memory>
#include "webrtc/base/arraysize.h"
#include "webrtc/base/fakeclock.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/ptr_util.h"
@ -143,10 +144,12 @@ struct Receiver : public MessageHandler, public sigslot::has_slots<> {
uint32_t samples;
};
// Note: This test uses a fake clock in addition to a virtual network.
class VirtualSocketServerTest : public testing::Test {
public:
VirtualSocketServerTest()
: thread_(&ss_),
: ss_(&fake_clock_),
thread_(&ss_),
kIPv4AnyAddress(IPAddress(INADDR_ANY), 0),
kIPv6AnyAddress(IPAddress(in6addr_any), 0) {}
@ -181,7 +184,8 @@ class VirtualSocketServerTest : public testing::Test {
socket->Bind(EmptySocketAddressWithFamily(default_route.family()));
SocketAddress client1_any_addr = socket->GetLocalAddress();
EXPECT_TRUE(client1_any_addr.IsAnyIP());
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket));
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket),
&fake_clock_);
// Create client2 bound to the default route.
AsyncSocket* socket2 =
@ -189,7 +193,8 @@ class VirtualSocketServerTest : public testing::Test {
socket2->Bind(SocketAddress(default_route, 0));
SocketAddress client2_addr = socket2->GetLocalAddress();
EXPECT_FALSE(client2_addr.IsAnyIP());
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2));
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2),
&fake_clock_);
// Client1 sends to client2, client2 should see the default route as
// client1's address.
@ -212,10 +217,12 @@ class VirtualSocketServerTest : public testing::Test {
// Make sure VSS didn't switch families on us.
EXPECT_EQ(server_addr.family(), initial_addr.family());
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket));
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket),
&fake_clock_);
AsyncSocket* socket2 =
ss_.CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM);
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2));
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2),
&fake_clock_);
SocketAddress client2_addr;
EXPECT_EQ(3, client2->SendTo("foo", 3, server_addr));
@ -229,7 +236,7 @@ class VirtualSocketServerTest : public testing::Test {
SocketAddress empty = EmptySocketAddressWithFamily(initial_addr.family());
for (int i = 0; i < 10; i++) {
client2 = MakeUnique<TestClient>(
WrapUnique(AsyncUDPSocket::Create(&ss_, empty)));
WrapUnique(AsyncUDPSocket::Create(&ss_, empty)), &fake_clock_);
SocketAddress next_client2_addr;
EXPECT_EQ(3, client2->SendTo("foo", 3, server_addr));
@ -684,12 +691,15 @@ class VirtualSocketServerTest : public testing::Test {
Sender sender(pthMain, send_socket, 80 * 1024);
Receiver receiver(pthMain, recv_socket, bandwidth);
pthMain->ProcessMessages(5000);
// Allow the sender to run for 5 (simulated) seconds, then be stopped for 5
// seconds.
SIMULATED_WAIT(false, 5000, fake_clock_);
sender.done = true;
pthMain->ProcessMessages(5000);
SIMULATED_WAIT(false, 5000, fake_clock_);
ASSERT_TRUE(receiver.count >= 5 * 3 * bandwidth / 4);
ASSERT_TRUE(receiver.count <= 6 * bandwidth); // queue could drain for 1s
// Ensure the observed bandwidth fell within a reasonable margin of error.
EXPECT_TRUE(receiver.count >= 5 * 3 * bandwidth / 4);
EXPECT_TRUE(receiver.count <= 6 * bandwidth); // queue could drain for 1s
ss_.set_bandwidth(0);
}
@ -725,7 +735,9 @@ class VirtualSocketServerTest : public testing::Test {
Sender sender(pthMain, send_socket, 100 * 2 * 1024);
Receiver receiver(pthMain, recv_socket, 0);
pthMain->ProcessMessages(10000);
// Simulate 10 seconds of packets being sent, then check the observed delay
// distribution.
SIMULATED_WAIT(false, 10000, fake_clock_);
sender.done = receiver.done = true;
ss_.ProcessMessagesUntilIdle();
@ -807,11 +819,13 @@ class VirtualSocketServerTest : public testing::Test {
AsyncSocket* socket = ss_.CreateAsyncSocket(SOCK_DGRAM);
socket->Bind(server_addr);
SocketAddress bound_server_addr = socket->GetLocalAddress();
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket));
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket),
&fake_clock_);
AsyncSocket* socket2 = ss_.CreateAsyncSocket(SOCK_DGRAM);
socket2->Bind(client_addr);
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2));
auto client2 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket2),
&fake_clock_);
SocketAddress client2_addr;
if (shouldSucceed) {
@ -828,6 +842,7 @@ class VirtualSocketServerTest : public testing::Test {
}
protected:
rtc::ScopedFakeClock fake_clock_;
VirtualSocketServer ss_;
AutoSocketServerThread thread_;
const SocketAddress kIPv4AnyAddress;
@ -912,20 +927,11 @@ TEST_F(VirtualSocketServerTest, bandwidth_v6) {
BandwidthTest(kIPv6AnyAddress);
}
// Disabled on iOS simulator since it's a test that relies on being able to
// process packets fast enough in real time, which isn't the case in the
// simulator.
#if defined(TARGET_IPHONE_SIMULATOR)
#define MAYBE_delay_v4 DISABLED_delay_v4
#else
#define MAYBE_delay_v4 delay_v4
#endif
TEST_F(VirtualSocketServerTest, MAYBE_delay_v4) {
TEST_F(VirtualSocketServerTest, delay_v4) {
DelayTest(kIPv4AnyAddress);
}
// See: https://code.google.com/p/webrtc/issues/detail?id=2409
TEST_F(VirtualSocketServerTest, DISABLED_delay_v6) {
TEST_F(VirtualSocketServerTest, delay_v6) {
DelayTest(kIPv6AnyAddress);
}
@ -1040,7 +1046,8 @@ TEST_F(VirtualSocketServerTest, SetSendingBlockedWithUdpSocket) {
WrapUnique(ss_.CreateAsyncSocket(kIPv4AnyAddress.family(), SOCK_DGRAM));
socket1->Bind(kIPv4AnyAddress);
socket2->Bind(kIPv4AnyAddress);
auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket1));
auto client1 =
MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket1), &fake_clock_);
ss_.SetSendingBlocked(true);
EXPECT_EQ(-1, client1->SendTo("foo", 3, socket2->GetLocalAddress()));

View File

@ -19,6 +19,7 @@
#include <vector>
#include "webrtc/base/checks.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/physicalsocketserver.h"
#include "webrtc/base/socketaddresspair.h"
@ -528,8 +529,11 @@ void VirtualSocket::OnSocketServerReadyToSend() {
}
}
VirtualSocketServer::VirtualSocketServer()
: wakeup_(/*manual_reset=*/false, /*initially_signaled=*/false),
VirtualSocketServer::VirtualSocketServer() : VirtualSocketServer(nullptr) {}
VirtualSocketServer::VirtualSocketServer(FakeClock* fake_clock)
: fake_clock_(fake_clock),
wakeup_(/*manual_reset=*/false, /*initially_signaled=*/false),
msg_queue_(nullptr),
stop_on_idle_(false),
next_ipv4_(kInitialNextIPv4),
@ -642,9 +646,17 @@ bool VirtualSocketServer::ProcessMessagesUntilIdle() {
RTC_DCHECK(msg_queue_ == Thread::Current());
stop_on_idle_ = true;
while (!msg_queue_->empty()) {
Message msg;
if (msg_queue_->Get(&msg, Thread::kForever)) {
msg_queue_->Dispatch(&msg);
if (fake_clock_) {
// If using a fake clock, advance it in millisecond increments until the
// queue is empty (the SIMULATED_WAIT macro ensures the queue processes
// all possible messages each time it's called).
SIMULATED_WAIT(false, 1, *fake_clock_);
} else {
// Otherwise, run a normal message loop.
Message msg;
if (msg_queue_->Get(&msg, Thread::kForever)) {
msg_queue_->Dispatch(&msg);
}
}
}
stop_on_idle_ = false;

View File

@ -17,6 +17,7 @@
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/event.h"
#include "webrtc/base/fakeclock.h"
#include "webrtc/base/messagequeue.h"
#include "webrtc/base/socketserver.h"
@ -33,6 +34,10 @@ class SocketAddressPair;
class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
public:
VirtualSocketServer();
// This constructor needs to be used if the test uses a fake clock and
// ProcessMessagesUntilIdle, since ProcessMessagesUntilIdle needs a way of
// advancing time.
explicit VirtualSocketServer(FakeClock* fake_clock);
~VirtualSocketServer() override;
// The default route indicates which local address to use when a socket is
@ -242,6 +247,10 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
typedef std::map<SocketAddress, VirtualSocket*> AddressMap;
typedef std::map<SocketAddressPair, VirtualSocket*> ConnectionMap;
// May be null if the test doesn't use a fake clock, or it does but doesn't
// use ProcessMessagesUntilIdle.
FakeClock* fake_clock_ = nullptr;
// Used to implement Wait/WakeUp.
Event wakeup_;
MessageQueue* msg_queue_;