From e8b00a1a748c20b0364d76115fb0a9bc4a3fd30e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 25 Aug 2020 17:11:20 +0200 Subject: [PATCH] Fix OperationsChainTest.OnChainEmptyCallback flake. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test was added in https://webrtc-review.googlesource.com/c/src/+/180620. On my machine it passes about 98% of the time. The test is meant to count the number of times the callback that the operations chain is empty has been invoked, and does this by ensuring the last operation to make the chain empty has completed before expecting that the counter has increased. The race happns when the operation has completed but the callback that the chain is empty has not happened yet. This CL fixes that by using EXPECT_EQ_WAIT instead. TBR=hta@webrtc.org Bug: chromium:1060083 Change-Id: I2ebfac3e635ef895d6602f7360e5ec6006fc1d0a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182541 Reviewed-by: Henrik Boström Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#31992} --- rtc_base/BUILD.gn | 1 + rtc_base/operations_chain_unittest.cc | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 4eaedfd066..a39a9b6be0 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -1276,6 +1276,7 @@ if (rtc_include_tests) { sources = [ "operations_chain_unittest.cc" ] deps = [ + ":gunit_helpers", ":rtc_base", ":rtc_base_approved", ":rtc_event", diff --git a/rtc_base/operations_chain_unittest.cc b/rtc_base/operations_chain_unittest.cc index 988ad346af..5f183e42cb 100644 --- a/rtc_base/operations_chain_unittest.cc +++ b/rtc_base/operations_chain_unittest.cc @@ -18,6 +18,7 @@ #include "rtc_base/bind.h" #include "rtc_base/event.h" +#include "rtc_base/gunit.h" #include "rtc_base/thread.h" #include "test/gmock.h" #include "test/gtest.h" @@ -26,6 +27,12 @@ namespace rtc { using ::testing::ElementsAre; +namespace { + +constexpr int kDefaultTimeout = 3000; + +} // namespace + class OperationTracker { public: OperationTracker() : background_thread_(Thread::Create()) { @@ -409,7 +416,7 @@ TEST(OperationsChainTest, OnChainEmptyCallback) { // Completing the operation empties the chain, invoking the callback. unblock_async_operation_event0.Set(); async_operation_completed_event0->Wait(Event::kForever); - EXPECT_EQ(1u, on_empty_callback_counter); + EXPECT_TRUE_WAIT(1u == on_empty_callback_counter, kDefaultTimeout); // Chain multiple events. Event unblock_async_operation_event1; @@ -421,16 +428,16 @@ TEST(OperationsChainTest, OnChainEmptyCallback) { operation_tracker_proxy.PostAsynchronousOperation( &unblock_async_operation_event2); // Again, the callback is not invoked until the operation has completed. - EXPECT_EQ(1u, on_empty_callback_counter); + EXPECT_TRUE_WAIT(1u == on_empty_callback_counter, kDefaultTimeout); // Upon completing the first event, the chain is still not empty, so the // callback must not be invoked yet. unblock_async_operation_event1.Set(); async_operation_completed_event1->Wait(Event::kForever); - EXPECT_EQ(1u, on_empty_callback_counter); + EXPECT_TRUE_WAIT(1u == on_empty_callback_counter, kDefaultTimeout); // Completing the last evenet empties the chain, invoking the callback. unblock_async_operation_event2.Set(); async_operation_completed_event2->Wait(Event::kForever); - EXPECT_EQ(2u, on_empty_callback_counter); + EXPECT_TRUE_WAIT(2u == on_empty_callback_counter, kDefaultTimeout); } TEST(OperationsChainTest,