From c3f3515f8ea78c0a8a5026639e605a9bdfda4add Mon Sep 17 00:00:00 2001 From: skvlad Date: Fri, 2 Sep 2016 13:23:46 -0700 Subject: [PATCH] Fixed flaky VideoSendStreamTest::SupportsAbsoluteSendTime This test failed on the memcheck bot: https://build.chromium.org/p/client.webrtc/builders/Linux%20Memcheck/builds/6704/steps/video_engine_tests/logs/stdio The test assumed that the absolute send time header extension can never be zero. It's a timestamp truncated to 24 bits, and zero is not a special value - so it can very rarely end up being precisely zero. The fix makes the test wait for at least one packet having a non-zero send time. I've considered changing the test to use a fake clock instead to ensure that not only the value is non-zero, but that it indeed reflects the system timestamp - but that involves changing a very large number of files. Besides, other tests in this file don't verify values for header extensions where zeroes are allowed. NOTRY=true Review-Url: https://codereview.webrtc.org/2307693002 Cr-Commit-Position: refs/heads/master@{#14056} --- webrtc/video/video_send_stream_tests.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 654784cc5f..8129af9ab2 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -136,8 +136,16 @@ TEST_F(VideoSendStreamTest, SupportsAbsoluteSendTime) { EXPECT_FALSE(header.extension.hasTransmissionTimeOffset); EXPECT_TRUE(header.extension.hasAbsoluteSendTime); EXPECT_EQ(header.extension.transmissionTimeOffset, 0); - EXPECT_GT(header.extension.absoluteSendTime, 0u); - observation_complete_.Set(); + if (header.extension.absoluteSendTime != 0) { + // Wait for at least one packet with a non-zero send time. The send time + // is a 16-bit value derived from the system clock, and it is valid + // for a packet to have a zero send time. To tell that from an + // unpopulated value we'll wait for a packet with non-zero send time. + observation_complete_.Set(); + } else { + LOG(LS_WARNING) << "Got a packet with zero absoluteSendTime, waiting" + " for another packet..."; + } return SEND_PACKET; }