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}
diff --git a/pc/track_media_info_map_unittest.cc b/pc/track_media_info_map_unittest.cc
index c487ab8..0cb1e0e 100644
--- a/pc/track_media_info_map_unittest.cc
+++ b/pc/track_media_info_map_unittest.cc
@@ -83,19 +83,21 @@
class TrackMediaInfoMapTest : public ::testing::Test {
public:
- TrackMediaInfoMapTest()
+ TrackMediaInfoMapTest() : TrackMediaInfoMapTest(true) {}
+
+ explicit TrackMediaInfoMapTest(bool use_current_thread)
: voice_media_info_(new cricket::VoiceMediaInfo()),
video_media_info_(new cricket::VideoMediaInfo()),
local_audio_track_(AudioTrack::Create("LocalAudioTrack", nullptr)),
remote_audio_track_(AudioTrack::Create("RemoteAudioTrack", nullptr)),
- local_video_track_(
- VideoTrack::Create("LocalVideoTrack",
- FakeVideoTrackSource::Create(false),
- rtc::Thread::Current())),
- remote_video_track_(
- VideoTrack::Create("RemoteVideoTrack",
- FakeVideoTrackSource::Create(false),
- rtc::Thread::Current())) {}
+ local_video_track_(VideoTrack::Create(
+ "LocalVideoTrack",
+ FakeVideoTrackSource::Create(false),
+ use_current_thread ? rtc::Thread::Current() : nullptr)),
+ remote_video_track_(VideoTrack::Create(
+ "RemoteVideoTrack",
+ FakeVideoTrackSource::Create(false),
+ use_current_thread ? rtc::Thread::Current() : nullptr)) {}
~TrackMediaInfoMapTest() {
// If we have a map the ownership has been passed to the map, only delete if
@@ -417,7 +419,10 @@
// base/test/gtest_util.h.
#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) {
AddRtpReceiverWithSsrcs({1}, remote_audio_track_);
diff --git a/rtc_base/synchronization/sequence_checker_unittest.cc b/rtc_base/synchronization/sequence_checker_unittest.cc
index 3ba7bc7..a173a82 100644
--- a/rtc_base/synchronization/sequence_checker_unittest.cc
+++ b/rtc_base/synchronization/sequence_checker_unittest.cc
@@ -158,8 +158,12 @@
}
#if RTC_DCHECK_IS_ON
-// TODO(bugs.webrtc.org/11577): Fix flakiness.
-TEST(SequenceCheckerTest, DISABLED_TestAnnotationsOnWrongQueueDebug) {
+// Note: Ending the test suite name with 'DeathTest' is important as it causes
+// 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(); }, "");
}
#else
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 34da889..3f91f4a 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -397,6 +397,7 @@
"//third_party/abseil-cpp/absl/flags:flag",
"//third_party/abseil-cpp/absl/flags:parse",
"//third_party/abseil-cpp/absl/memory",
+ "//third_party/abseil-cpp/absl/strings:strings",
"//third_party/abseil-cpp/absl/types:optional",
]
}
diff --git a/test/test_main_lib.cc b/test/test_main_lib.cc
index 1fd6230..f5e0234 100644
--- a/test/test_main_lib.cc
+++ b/test/test_main_lib.cc
@@ -17,6 +17,7 @@
#include "absl/flags/flag.h"
#include "absl/flags/parse.h"
#include "absl/memory/memory.h"
+#include "absl/strings/match.h"
#include "absl/types/optional.h"
#include "rtc_base/checks.h"
#include "rtc_base/event_tracer.h"
@@ -109,26 +110,48 @@
TestListener() = default;
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 {
- // Ensure that main thread gets wrapped as an rtc::Thread.
- // 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
- // as we have automatic thread wrapping, we need this to avoid that some
- // other random thread (which one depending on which tests are run) gets
- // automatically wrapped.
- thread_ = rtc::Thread::CreateWithSocketServer();
- thread_->WrapCurrent();
- RTC_DCHECK_EQ(rtc::Thread::Current(), thread_.get());
+ if (!IsDeathTest(test_info.test_suite_name(), test_info.name())) {
+ // Ensure that main thread gets wrapped as an rtc::Thread.
+ // 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 as we have automatic thread wrapping, we need this to avoid that
+ // some other random thread (which one depending on which tests are run)
+ // gets automatically wrapped.
+ thread_ = rtc::Thread::CreateWithSocketServer();
+ thread_->WrapCurrent();
+ 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 {
// Terminate the message loop. Note that if the test failed to clean
// 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.
- thread_->Quit(); // Signal quit.
- thread_->Run(); // Flush + process Quit signal.
- thread_->UnwrapCurrent();
- thread_ = nullptr;
+ if (thread_) {
+ thread_->Quit(); // Signal quit.
+ thread_->Run(); // Flush + process Quit signal.
+ thread_->UnwrapCurrent();
+ thread_ = nullptr;
+ }
}
std::unique_ptr<rtc::Thread> thread_;
