Fix flaky test SendRtpRtcpHeaderExtensionsTest.SentPackets*.
Flakiness was caused by a race condition between two atomic integers shared by two threads. Fixed by counting bad packets (those not containing the expected extension) instead of the good packets.
The CL also eliminates another possible flake by introducing a test fixture which doesn't automatically start sending audio packets when constructed.
BUG=3340,3356
R=tina.legrand@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/14499004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@6182 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.cc b/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.cc
index 8cd4894..c688e48 100644
--- a/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.cc
+++ b/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.cc
@@ -10,65 +10,7 @@
#include "webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h"
-#include <string.h>
-
AfterStreamingFixture::AfterStreamingFixture()
- : channel_(voe_base_->CreateChannel()) {
- EXPECT_GE(channel_, 0);
-
- fake_microphone_input_file_ = resource_manager_.long_audio_file_path();
- EXPECT_FALSE(fake_microphone_input_file_.empty());
-
- SetUpLocalPlayback();
+ : BeforeStreamingFixture() {
ResumePlaying();
- RestartFakeMicrophone();
-}
-
-AfterStreamingFixture::~AfterStreamingFixture() {
- voe_file_->StopPlayingFileAsMicrophone(channel_);
- PausePlaying();
-
- EXPECT_EQ(0, voe_network_->DeRegisterExternalTransport(channel_));
- voe_base_->DeleteChannel(channel_);
- delete transport_;
-}
-
-void AfterStreamingFixture::SwitchToManualMicrophone() {
- EXPECT_EQ(0, voe_file_->StopPlayingFileAsMicrophone(channel_));
-
- TEST_LOG("You need to speak manually into the microphone for this test.\n");
- TEST_LOG("Please start speaking now.\n");
- Sleep(1000);
-}
-
-void AfterStreamingFixture::RestartFakeMicrophone() {
- EXPECT_EQ(0, voe_file_->StartPlayingFileAsMicrophone(
- channel_, fake_microphone_input_file_.c_str(), true, true));
-}
-
-void AfterStreamingFixture::PausePlaying() {
- EXPECT_EQ(0, voe_base_->StopSend(channel_));
- EXPECT_EQ(0, voe_base_->StopPlayout(channel_));
- EXPECT_EQ(0, voe_base_->StopReceive(channel_));
-}
-
-void AfterStreamingFixture::ResumePlaying() {
- EXPECT_EQ(0, voe_base_->StartReceive(channel_));
- EXPECT_EQ(0, voe_base_->StartPlayout(channel_));
- EXPECT_EQ(0, voe_base_->StartSend(channel_));
-}
-
-void AfterStreamingFixture::SetUpLocalPlayback() {
- transport_ = new LoopBackTransport(voe_network_);
- EXPECT_EQ(0, voe_network_->RegisterExternalTransport(channel_, *transport_));
-
- webrtc::CodecInst codec;
- codec.channels = 1;
- codec.pacsize = 160;
- codec.plfreq = 8000;
- codec.pltype = 0;
- codec.rate = 64000;
- strcpy(codec.plname, "PCMU");
-
- voe_codec_->SetSendCodec(channel_, codec);
}
diff --git a/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h b/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h
index f9980e1..f81716f 100644
--- a/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h
+++ b/webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h
@@ -11,40 +11,14 @@
#ifndef SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_AFTER_STREAMING_H_
#define SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_AFTER_STREAMING_H_
-#include "webrtc/voice_engine/test/auto_test/fixtures/after_initialization_fixture.h"
-#include "webrtc/voice_engine/test/auto_test/resource_manager.h"
+#include "webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h"
// This fixture will, in addition to the work done by its superclasses,
-// create a channel and start playing a file through the fake microphone
-// to simulate microphone input. The purpose is to make it convenient
-// to write tests that require microphone input.
-class AfterStreamingFixture : public AfterInitializationFixture {
+// start play back on construction.
+class AfterStreamingFixture : public BeforeStreamingFixture {
public:
AfterStreamingFixture();
- virtual ~AfterStreamingFixture();
-
- protected:
- int channel_;
- ResourceManager resource_manager_;
- std::string fake_microphone_input_file_;
-
- // Shuts off the fake microphone for this test.
- void SwitchToManualMicrophone();
-
- // Restarts the fake microphone if it's been shut off earlier.
- void RestartFakeMicrophone();
-
- // Stops all sending and playout.
- void PausePlaying();
-
- // Resumes all sending and playout.
- void ResumePlaying();
-
- private:
- void SetUpLocalPlayback();
-
- LoopBackTransport* transport_;
+ virtual ~AfterStreamingFixture() {}
};
-
#endif // SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_AFTER_STREAMING_H_
diff --git a/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.cc b/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.cc
new file mode 100644
index 0000000..8a7c4c3
--- /dev/null
+++ b/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.cc
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h"
+
+BeforeStreamingFixture::BeforeStreamingFixture()
+ : channel_(voe_base_->CreateChannel()),
+ transport_(NULL) {
+ EXPECT_GE(channel_, 0);
+
+ fake_microphone_input_file_ = resource_manager_.long_audio_file_path();
+ EXPECT_FALSE(fake_microphone_input_file_.empty());
+
+ SetUpLocalPlayback();
+ RestartFakeMicrophone();
+}
+
+BeforeStreamingFixture::~BeforeStreamingFixture() {
+ voe_file_->StopPlayingFileAsMicrophone(channel_);
+ PausePlaying();
+
+ EXPECT_EQ(0, voe_network_->DeRegisterExternalTransport(channel_));
+ voe_base_->DeleteChannel(channel_);
+ delete transport_;
+}
+
+void BeforeStreamingFixture::SwitchToManualMicrophone() {
+ EXPECT_EQ(0, voe_file_->StopPlayingFileAsMicrophone(channel_));
+
+ TEST_LOG("You need to speak manually into the microphone for this test.\n");
+ TEST_LOG("Please start speaking now.\n");
+ Sleep(1000);
+}
+
+void BeforeStreamingFixture::RestartFakeMicrophone() {
+ EXPECT_EQ(0, voe_file_->StartPlayingFileAsMicrophone(
+ channel_, fake_microphone_input_file_.c_str(), true, true));
+}
+
+void BeforeStreamingFixture::PausePlaying() {
+ EXPECT_EQ(0, voe_base_->StopSend(channel_));
+ EXPECT_EQ(0, voe_base_->StopPlayout(channel_));
+ EXPECT_EQ(0, voe_base_->StopReceive(channel_));
+}
+
+void BeforeStreamingFixture::ResumePlaying() {
+ EXPECT_EQ(0, voe_base_->StartReceive(channel_));
+ EXPECT_EQ(0, voe_base_->StartPlayout(channel_));
+ EXPECT_EQ(0, voe_base_->StartSend(channel_));
+}
+
+void BeforeStreamingFixture::SetUpLocalPlayback() {
+ transport_ = new LoopBackTransport(voe_network_);
+ EXPECT_EQ(0, voe_network_->RegisterExternalTransport(channel_, *transport_));
+
+ webrtc::CodecInst codec;
+ codec.channels = 1;
+ codec.pacsize = 160;
+ codec.plfreq = 8000;
+ codec.pltype = 0;
+ codec.rate = 64000;
+#if defined(_MSC_VER) && defined(_WIN32)
+ _snprintf(codec.plname, RTP_PAYLOAD_NAME_SIZE - 1, "PCMU");
+#else
+ snprintf(codec.plname, RTP_PAYLOAD_NAME_SIZE, "PCMU");
+#endif
+ voe_codec_->SetSendCodec(channel_, codec);
+}
diff --git a/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h b/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h
new file mode 100644
index 0000000..6de583c
--- /dev/null
+++ b/webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_BEFORE_STREAMING_H_
+#define SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_BEFORE_STREAMING_H_
+
+#include <string>
+#include "webrtc/voice_engine/test/auto_test/fixtures/after_initialization_fixture.h"
+#include "webrtc/voice_engine/test/auto_test/resource_manager.h"
+
+// This fixture will, in addition to the work done by its superclasses,
+// create a channel and prepare playing a file through the fake microphone
+// to simulate microphone input. The purpose is to make it convenient
+// to write tests that require microphone input.
+class BeforeStreamingFixture : public AfterInitializationFixture {
+ public:
+ BeforeStreamingFixture();
+ virtual ~BeforeStreamingFixture();
+
+ protected:
+ int channel_;
+ ResourceManager resource_manager_;
+ std::string fake_microphone_input_file_;
+
+ // Shuts off the fake microphone for this test.
+ void SwitchToManualMicrophone();
+
+ // Restarts the fake microphone if it's been shut off earlier.
+ void RestartFakeMicrophone();
+
+ // Stops all sending and playout.
+ void PausePlaying();
+
+ // Resumes all sending and playout.
+ void ResumePlaying();
+
+ private:
+ void SetUpLocalPlayback();
+
+ LoopBackTransport* transport_;
+};
+
+
+#endif // SRC_VOICE_ENGINE_MAIN_TEST_AUTO_TEST_STANDARD_BEFORE_STREAMING_H_
diff --git a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc
index eaa7f50..cf33adf 100644
--- a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc
+++ b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc
@@ -13,7 +13,7 @@
#include "webrtc/system_wrappers/interface/atomic32.h"
#include "webrtc/system_wrappers/interface/sleep.h"
#include "webrtc/video_engine/include/vie_network.h"
-#include "webrtc/voice_engine/test/auto_test/fixtures/after_streaming_fixture.h"
+#include "webrtc/voice_engine/test/auto_test/fixtures/before_streaming_fixture.h"
using ::testing::_;
using ::testing::AtLeast;
@@ -23,14 +23,13 @@
class ExtensionVerifyTransport : public webrtc::Transport {
public:
ExtensionVerifyTransport()
- : received_packets_(0),
- ok_packets_(0),
- parser_(webrtc::RtpHeaderParser::Create()),
+ : parser_(webrtc::RtpHeaderParser::Create()),
+ received_packets_(0),
+ bad_packets_(0),
audio_level_id_(-1),
absolute_sender_time_id_(-1) {}
virtual int SendPacket(int channel, const void* data, int len) {
- ++received_packets_;
webrtc::RTPHeader header;
if (parser_->Parse(static_cast<const uint8_t*>(data), len, &header)) {
bool ok = true;
@@ -42,10 +41,14 @@
!header.extension.hasAbsoluteSendTime) {
ok = false;
}
- if (ok) {
- ++ok_packets_;
+ if (!ok) {
+ // bad_packets_ count packets we expected to have an extension but
+ // didn't have one.
+ ++bad_packets_;
}
}
+ // received_packets_ count all packets we receive.
+ ++received_packets_;
return len;
}
@@ -64,25 +67,31 @@
id);
}
- bool WaitForNPackets(int count) {
- while (received_packets_.Value() < count) {
- webrtc::SleepMs(10);
+ bool Wait() {
+ // Wait until we've received to specified number of packets.
+ while (received_packets_.Value() < kPacketsExpected) {
+ webrtc::SleepMs(kSleepIntervalMs);
}
- return (ok_packets_.Value() == count);
+ // Check whether any where 'bad' (didn't contain an extension when they
+ // where supposed to).
+ return bad_packets_.Value() == 0;
}
private:
- webrtc::Atomic32 received_packets_;
- webrtc::Atomic32 ok_packets_;
+ enum {
+ kPacketsExpected = 10,
+ kSleepIntervalMs = 10
+ };
webrtc::scoped_ptr<webrtc::RtpHeaderParser> parser_;
+ webrtc::Atomic32 received_packets_;
+ webrtc::Atomic32 bad_packets_;
int audio_level_id_;
int absolute_sender_time_id_;
};
-class SendRtpRtcpHeaderExtensionsTest : public AfterStreamingFixture {
+class SendRtpRtcpHeaderExtensionsTest : public BeforeStreamingFixture {
protected:
virtual void SetUp() {
- PausePlaying();
EXPECT_EQ(0, voe_network_->DeRegisterExternalTransport(channel_));
EXPECT_EQ(0, voe_network_->RegisterExternalTransport(channel_,
verifying_transport_));
@@ -94,12 +103,25 @@
ExtensionVerifyTransport verifying_transport_;
};
+TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeNoAudioLevel) {
+ verifying_transport_.SetAudioLevelId(0);
+ ResumePlaying();
+ EXPECT_FALSE(verifying_transport_.Wait());
+}
+
TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAudioLevel) {
EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAudioLevelIndicationStatus(channel_, true,
9));
verifying_transport_.SetAudioLevelId(9);
ResumePlaying();
- EXPECT_TRUE(verifying_transport_.WaitForNPackets(10));
+ EXPECT_TRUE(verifying_transport_.Wait());
+}
+
+TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeNoAbsoluteSenderTime)
+{
+ verifying_transport_.SetAbsoluteSenderTimeId(0);
+ ResumePlaying();
+ EXPECT_FALSE(verifying_transport_.Wait());
}
TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAbsoluteSenderTime) {
@@ -107,7 +129,7 @@
11));
verifying_transport_.SetAbsoluteSenderTimeId(11);
ResumePlaying();
- EXPECT_TRUE(verifying_transport_.WaitForNPackets(10));
+ EXPECT_TRUE(verifying_transport_.Wait());
}
TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAllExtensions1) {
@@ -118,7 +140,7 @@
verifying_transport_.SetAudioLevelId(9);
verifying_transport_.SetAbsoluteSenderTimeId(11);
ResumePlaying();
- EXPECT_TRUE(verifying_transport_.WaitForNPackets(10));
+ EXPECT_TRUE(verifying_transport_.Wait());
}
TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAllExtensions2) {
@@ -130,7 +152,7 @@
// Don't register audio level with header parser - unknown extensions should
// be ignored when parsing.
ResumePlaying();
- EXPECT_TRUE(verifying_transport_.WaitForNPackets(10));
+ EXPECT_TRUE(verifying_transport_.Wait());
}
class MockViENetwork : public webrtc::ViENetwork {
@@ -150,10 +172,9 @@
const webrtc::RTPHeader&));
};
-class ReceiveRtpRtcpHeaderExtensionsTest : public AfterStreamingFixture {
+class ReceiveRtpRtcpHeaderExtensionsTest : public BeforeStreamingFixture {
protected:
virtual void SetUp() {
- PausePlaying();
EXPECT_EQ(0,
voe_rtp_rtcp_->SetSendAbsoluteSenderTimeStatus(channel_, true, 11));
EXPECT_EQ(0,
diff --git a/webrtc/voice_engine/voice_engine.gyp b/webrtc/voice_engine/voice_engine.gyp
index a2de46d..8e8b98f 100644
--- a/webrtc/voice_engine/voice_engine.gyp
+++ b/webrtc/voice_engine/voice_engine.gyp
@@ -162,6 +162,8 @@
'test/auto_test/fixtures/after_streaming_fixture.h',
'test/auto_test/fixtures/before_initialization_fixture.cc',
'test/auto_test/fixtures/before_initialization_fixture.h',
+ 'test/auto_test/fixtures/before_streaming_fixture.cc',
+ 'test/auto_test/fixtures/before_streaming_fixture.h',
'test/auto_test/standard/audio_processing_test.cc',
'test/auto_test/standard/codec_before_streaming_test.cc',
'test/auto_test/standard/codec_test.cc',
