From 9db32157cd9f492ec1851ea18b241a0d1a94a08b Mon Sep 17 00:00:00 2001
From: Francois Rousseau <frousseau@google.com>
Date: Mon, 13 May 2019 21:52:35 +0000
Subject: [PATCH] [crash][feedback] switch to RunLoopUntil() in unit tests
* apparently RunLoopWithTimeoutUntil() is bad because the default timeout
isn't guaranteed to be enough on some bots so instead we rely on the
infra timeout they give to each test
* we don't check the return value RunLoopUntil() in anticipation of https://fuchsia-review.googlesource.com/c/fuchsia/+/280471
TESTED=`fx run-test feedback_agent_tests`
TESTED=`fx run-test crashpad_agent_tests`
Change-Id: I491252ef6a3e3f51f418ef5e57d3f426411e54f6
---
.../tests/crashpad_agent_unittest.cc | 15 +++++----------
.../tests/data_provider_unittest.cc | 16 ++++++----------
.../tests/log_listener_unittest.cc | 12 ++----------
3 files changed, 13 insertions(+), 30 deletions(-)
diff --git a/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc b/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
index 895b9cba181..1455650f15c 100644
--- a/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
+++ b/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
@@ -185,8 +185,7 @@ class CrashpadAgentTest : public gtest::RealLoopFixture {
out_result = std::move(result);
has_out_result = true;
});
- FXL_CHECK(RunLoopWithTimeoutOrUntil(
- [&has_out_result] { return has_out_result; }));
+ RunLoopUntil([&has_out_result] { return has_out_result; });
return out_result;
}
@@ -277,8 +276,7 @@ TEST_F(CrashpadAgentTest, OnNativeException_C_Basic) {
out_result = std::move(result);
has_out_result = true;
});
- ASSERT_TRUE(
- RunLoopWithTimeoutOrUntil([&has_out_result] { return has_out_result; }));
+ RunLoopUntil([&has_out_result] { return has_out_result; });
EXPECT_TRUE(out_result.is_response());
CheckAttachments();
@@ -315,8 +313,7 @@ TEST_F(CrashpadAgentTest, OnManagedRuntimeException_Dart_Basic) {
out_result = std::move(result);
has_out_result = true;
});
- ASSERT_TRUE(
- RunLoopWithTimeoutOrUntil([&has_out_result] { return has_out_result; }));
+ RunLoopUntil([&has_out_result] { return has_out_result; });
EXPECT_TRUE(out_result.is_response());
CheckAttachments({"DartError"});
@@ -337,8 +334,7 @@ TEST_F(CrashpadAgentTest, OnManagedRuntimeException_UnknownLanguage_Basic) {
out_result = std::move(result);
has_out_result = true;
});
- ASSERT_TRUE(
- RunLoopWithTimeoutOrUntil([&has_out_result] { return has_out_result; }));
+ RunLoopUntil([&has_out_result] { return has_out_result; });
EXPECT_TRUE(out_result.is_response());
CheckAttachments({"data"});
@@ -356,8 +352,7 @@ TEST_F(CrashpadAgentTest, OnKernelPanicCrashLog_Basic) {
out_result = std::move(result);
has_out_result = true;
});
- ASSERT_TRUE(
- RunLoopWithTimeoutOrUntil([&has_out_result] { return has_out_result; }));
+ RunLoopUntil([&has_out_result] { return has_out_result; });
EXPECT_TRUE(out_result.is_response());
CheckAttachments({"kernel_panic_crash_log"});
diff --git a/src/developer/feedback_agent/tests/data_provider_unittest.cc b/src/developer/feedback_agent/tests/data_provider_unittest.cc
index 86ac2a72d56..f7c21af469a 100644
--- a/src/developer/feedback_agent/tests/data_provider_unittest.cc
+++ b/src/developer/feedback_agent/tests/data_provider_unittest.cc
@@ -204,8 +204,7 @@ TEST_F(DataProviderImplTest, GetScreenshot_SucceedOnScenicReturningSuccess) {
feedback_response.screenshot = std::move(screenshot);
has_feedback_response = true;
});
- ASSERT_TRUE(RunLoopWithTimeoutOrUntil(
- [&has_feedback_response] { return has_feedback_response; }));
+ RunLoopUntil([&has_feedback_response] { return has_feedback_response; });
EXPECT_TRUE(get_scenic_responses().empty());
@@ -240,8 +239,7 @@ TEST_F(DataProviderImplTest, GetScreenshot_FailOnScenicReturningFailure) {
feedback_response.screenshot = std::move(screenshot);
has_feedback_response = true;
});
- ASSERT_TRUE(RunLoopWithTimeoutOrUntil(
- [&has_feedback_response] { return has_feedback_response; }));
+ RunLoopUntil([&has_feedback_response] { return has_feedback_response; });
EXPECT_TRUE(get_scenic_responses().empty());
@@ -262,8 +260,7 @@ TEST_F(DataProviderImplTest,
feedback_response.screenshot = std::move(screenshot);
has_feedback_response = true;
});
- ASSERT_TRUE(RunLoopWithTimeoutOrUntil(
- [&has_feedback_response] { return has_feedback_response; }));
+ RunLoopUntil([&has_feedback_response] { return has_feedback_response; });
EXPECT_TRUE(get_scenic_responses().empty());
@@ -294,9 +291,9 @@ TEST_F(DataProviderImplTest, GetScreenshot_ParallelRequests) {
feedback_responses.push_back({std::move(screenshot)});
});
}
- ASSERT_TRUE(RunLoopWithTimeoutOrUntil([&feedback_responses, num_calls] {
+ RunLoopUntil([&feedback_responses, num_calls] {
return feedback_responses.size() == num_calls;
- }));
+ });
EXPECT_TRUE(get_scenic_responses().empty());
@@ -343,8 +340,7 @@ TEST_F(DataProviderImplTest, GetData_SmokeTest) {
feedback_result = std::move(result);
has_feedback_result = true;
});
- ASSERT_TRUE(RunLoopWithTimeoutOrUntil(
- [&has_feedback_result] { return has_feedback_result; }));
+ RunLoopUntil([&has_feedback_result] { return has_feedback_result; });
ASSERT_TRUE(feedback_result.is_response());
// As we control the system log attachment, we can expect it to be present and
diff --git a/src/developer/feedback_agent/tests/log_listener_unittest.cc b/src/developer/feedback_agent/tests/log_listener_unittest.cc
index 23fdcd1de79..1f3fce53c03 100644
--- a/src/developer/feedback_agent/tests/log_listener_unittest.cc
+++ b/src/developer/feedback_agent/tests/log_listener_unittest.cc
@@ -28,8 +28,6 @@ namespace fuchsia {
namespace feedback {
namespace {
-const zx::duration kLoopTimeout = zx::sec(1);
-
template <typename ResultListenerT>
bool DoStringBufferMatch(const fuchsia::mem::Buffer& actual,
const std::string& expected,
@@ -78,11 +76,7 @@ class CollectSystemLogTest : public gtest::RealLoopFixture {
}
fit::result<fuchsia::mem::Buffer> CollectSystemLog(
- zx::duration timeout = zx::msec(900)) {
- FXL_CHECK(timeout < kLoopTimeout)
- << "The timeout for the system log collection must be strictly smaller "
- "than the timeout for the test";
-
+ zx::duration timeout = zx::sec(1)) {
fit::result<fuchsia::mem::Buffer> result;
executor_.schedule_task(
fuchsia::feedback::CollectSystemLog(
@@ -90,9 +84,7 @@ class CollectSystemLogTest : public gtest::RealLoopFixture {
.then([&result](fit::result<fuchsia::mem::Buffer>& res) {
result = std::move(res);
}));
- FXL_CHECK(
- RunLoopWithTimeoutOrUntil([&result] { return !!result; }, kLoopTimeout))
- << "Test timed out";
+ RunLoopUntil([&result] { return !!result; });
return result;
}
--
GitLab