Don't wrap the main thread when running death tests.

Also re-enable the TestAnnotationsOnWrongQueueDebug test and rename
the test suite to SequenceCheckerDeathTest so that it gets executed
before other tests.

Bug: webrtc:11577
Change-Id: I3b8037644e4b9139755ccecb17e42b09327e4996
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175346
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31290}
This commit is contained in:
Tommi
2020-05-17 14:33:40 +02:00
committed by Commit Bot
parent d0d55515c4
commit ec3ba734e9
4 changed files with 58 additions and 25 deletions

View File

@ -83,19 +83,21 @@ rtc::scoped_refptr<MockRtpReceiverInternal> CreateMockRtpReceiver(
class TrackMediaInfoMapTest : public ::testing::Test { class TrackMediaInfoMapTest : public ::testing::Test {
public: public:
TrackMediaInfoMapTest() TrackMediaInfoMapTest() : TrackMediaInfoMapTest(true) {}
explicit TrackMediaInfoMapTest(bool use_current_thread)
: voice_media_info_(new cricket::VoiceMediaInfo()), : voice_media_info_(new cricket::VoiceMediaInfo()),
video_media_info_(new cricket::VideoMediaInfo()), video_media_info_(new cricket::VideoMediaInfo()),
local_audio_track_(AudioTrack::Create("LocalAudioTrack", nullptr)), local_audio_track_(AudioTrack::Create("LocalAudioTrack", nullptr)),
remote_audio_track_(AudioTrack::Create("RemoteAudioTrack", nullptr)), remote_audio_track_(AudioTrack::Create("RemoteAudioTrack", nullptr)),
local_video_track_( local_video_track_(VideoTrack::Create(
VideoTrack::Create("LocalVideoTrack", "LocalVideoTrack",
FakeVideoTrackSource::Create(false), FakeVideoTrackSource::Create(false),
rtc::Thread::Current())), use_current_thread ? rtc::Thread::Current() : nullptr)),
remote_video_track_( remote_video_track_(VideoTrack::Create(
VideoTrack::Create("RemoteVideoTrack", "RemoteVideoTrack",
FakeVideoTrackSource::Create(false), FakeVideoTrackSource::Create(false),
rtc::Thread::Current())) {} use_current_thread ? rtc::Thread::Current() : nullptr)) {}
~TrackMediaInfoMapTest() { ~TrackMediaInfoMapTest() {
// If we have a map the ownership has been passed to the map, only delete if // If we have a map the ownership has been passed to the map, only delete if
@ -417,7 +419,10 @@ TEST_F(TrackMediaInfoMapTest, GetAttachmentIdByTrack) {
// base/test/gtest_util.h. // base/test/gtest_util.h.
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
class TrackMediaInfoMapDeathTest : public TrackMediaInfoMapTest {}; class TrackMediaInfoMapDeathTest : public TrackMediaInfoMapTest {
public:
TrackMediaInfoMapDeathTest() : TrackMediaInfoMapTest(false) {}
};
TEST_F(TrackMediaInfoMapDeathTest, MultipleOneSsrcReceiversPerTrack) { TEST_F(TrackMediaInfoMapDeathTest, MultipleOneSsrcReceiversPerTrack) {
AddRtpReceiverWithSsrcs({1}, remote_audio_track_); AddRtpReceiverWithSsrcs({1}, remote_audio_track_);

View File

@ -158,8 +158,12 @@ void TestAnnotationsOnWrongQueue() {
} }
#if RTC_DCHECK_IS_ON #if RTC_DCHECK_IS_ON
// TODO(bugs.webrtc.org/11577): Fix flakiness. // Note: Ending the test suite name with 'DeathTest' is important as it causes
TEST(SequenceCheckerTest, DISABLED_TestAnnotationsOnWrongQueueDebug) { // gtest to order this test before any other non-death-tests, to avoid potential
// global process state pollution such as shared worker threads being started
// (e.g. a side effect of calling InitCocoaMultiThreading() on Mac causes one or
// two additional threads to be created).
TEST(SequenceCheckerDeathTest, TestAnnotationsOnWrongQueueDebug) {
ASSERT_DEATH({ TestAnnotationsOnWrongQueue(); }, ""); ASSERT_DEATH({ TestAnnotationsOnWrongQueue(); }, "");
} }
#else #else

View File

@ -397,6 +397,7 @@ if (rtc_include_tests) {
"//third_party/abseil-cpp/absl/flags:flag", "//third_party/abseil-cpp/absl/flags:flag",
"//third_party/abseil-cpp/absl/flags:parse", "//third_party/abseil-cpp/absl/flags:parse",
"//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/memory",
"//third_party/abseil-cpp/absl/strings:strings",
"//third_party/abseil-cpp/absl/types:optional", "//third_party/abseil-cpp/absl/types:optional",
] ]
} }

View File

@ -17,6 +17,7 @@
#include "absl/flags/flag.h" #include "absl/flags/flag.h"
#include "absl/flags/parse.h" #include "absl/flags/parse.h"
#include "absl/memory/memory.h" #include "absl/memory/memory.h"
#include "absl/strings/match.h"
#include "absl/types/optional.h" #include "absl/types/optional.h"
#include "rtc_base/checks.h" #include "rtc_base/checks.h"
#include "rtc_base/event_tracer.h" #include "rtc_base/event_tracer.h"
@ -109,27 +110,49 @@ class TestMainImpl : public TestMain {
TestListener() = default; TestListener() = default;
private: private:
bool IsDeathTest(const char* test_case_name, const char* test_name) {
// Workaround to avoid wrapping the main thread when we run death tests.
// The approach we take for detecting death tests is essentially the same
// as gtest does internally. Gtest does this:
//
// static const char kDeathTestCaseFilter[] = "*DeathTest:*DeathTest/*";
// ::testing::internal::UnitTestOptions::MatchesFilter(
// test_case_name, kDeathTestCaseFilter);
//
// Our approach is a little more straight forward.
if (absl::EndsWith(test_case_name, "DeathTest"))
return true;
return absl::EndsWith(test_name, "DeathTest");
}
void OnTestStart(const ::testing::TestInfo& test_info) override { void OnTestStart(const ::testing::TestInfo& test_info) override {
if (!IsDeathTest(test_info.test_suite_name(), test_info.name())) {
// Ensure that main thread gets wrapped as an rtc::Thread. // Ensure that main thread gets wrapped as an rtc::Thread.
// TODO(bugs.webrtc.org/9714): It might be better to avoid wrapping the // TODO(bugs.webrtc.org/9714): It might be better to avoid wrapping the
// main thread, or leave it to individual tests that need it. But as long // main thread, or leave it to individual tests that need it. But as
// as we have automatic thread wrapping, we need this to avoid that some // long as we have automatic thread wrapping, we need this to avoid that
// other random thread (which one depending on which tests are run) gets // some other random thread (which one depending on which tests are run)
// automatically wrapped. // gets automatically wrapped.
thread_ = rtc::Thread::CreateWithSocketServer(); thread_ = rtc::Thread::CreateWithSocketServer();
thread_->WrapCurrent(); thread_->WrapCurrent();
RTC_DCHECK_EQ(rtc::Thread::Current(), thread_.get()); RTC_DCHECK_EQ(rtc::Thread::Current(), thread_.get());
} else {
RTC_LOG(LS_INFO) << "No thread auto wrap for death test.";
}
} }
void OnTestEnd(const ::testing::TestInfo& test_info) override { void OnTestEnd(const ::testing::TestInfo& test_info) override {
// Terminate the message loop. Note that if the test failed to clean // Terminate the message loop. Note that if the test failed to clean
// up pending messages, this may execute part of the test. Ideally we // up pending messages, this may execute part of the test. Ideally we
// should print a warning message here, or even fail the test if it leaks. // should print a warning message here, or even fail the test if it leaks.
if (thread_) {
thread_->Quit(); // Signal quit. thread_->Quit(); // Signal quit.
thread_->Run(); // Flush + process Quit signal. thread_->Run(); // Flush + process Quit signal.
thread_->UnwrapCurrent(); thread_->UnwrapCurrent();
thread_ = nullptr; thread_ = nullptr;
} }
}
std::unique_ptr<rtc::Thread> thread_; std::unique_ptr<rtc::Thread> thread_;
}; };