aboutsummaryrefslogtreecommitdiffhomepage
path: root/Firestore/core/test
diff options
context:
space:
mode:
authorGravatar Konstantin Varlamov <var-const@users.noreply.github.com>2018-05-10 19:18:22 -0400
committerGravatar GitHub <noreply@github.com>2018-05-10 19:18:22 -0400
commit8cde81689dcfc74c9b59e088bbe00f95ca4bf68d (patch)
tree94c8bb8874ab756e79ffa9c63d335d3ffc513fec /Firestore/core/test
parentc7667229e2ab35d14fa7855789ee0584a2f8e817 (diff)
Firestore C++: quick fix for flaky tests in AsyncQueue and Executor (#1245)
Diffstat (limited to 'Firestore/core/test')
-rw-r--r--Firestore/core/test/firebase/firestore/util/async_queue_test.cc8
-rw-r--r--Firestore/core/test/firebase/firestore/util/executor_std_test.cc8
-rw-r--r--Firestore/core/test/firebase/firestore/util/executor_test.cc8
3 files changed, 18 insertions, 6 deletions
diff --git a/Firestore/core/test/firebase/firestore/util/async_queue_test.cc b/Firestore/core/test/firebase/firestore/util/async_queue_test.cc
index bcee2e3..4247584 100644
--- a/Firestore/core/test/firebase/firestore/util/async_queue_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/async_queue_test.cc
@@ -83,16 +83,20 @@ TEST_P(AsyncQueueTest, VerifyIsCurrentQueueWorksWithOperationInProgress) {
queue.EnqueueBlocking([&] { EXPECT_NO_THROW(queue.VerifyIsCurrentQueue()); });
}
+// TODO(varconst): this test is inherently flaky because it can't be guaranteed
+// that the enqueued asynchronous operation didn't finish before the code has
+// a chance to even enqueue the next operation. Delays are chosen so that the
+// test is unlikely to fail in practice. Need to revisit this.
TEST_P(AsyncQueueTest, CanScheduleOperationsInTheFuture) {
std::string steps;
queue.Enqueue([&steps] { steps += '1'; });
queue.Enqueue([&] {
- queue.EnqueueAfterDelay(AsyncQueue::Milliseconds(5), kTimerId1, [&] {
+ queue.EnqueueAfterDelay(AsyncQueue::Milliseconds(20), kTimerId1, [&] {
steps += '4';
signal_finished();
});
- queue.EnqueueAfterDelay(AsyncQueue::Milliseconds(1), kTimerId2,
+ queue.EnqueueAfterDelay(AsyncQueue::Milliseconds(10), kTimerId2,
[&steps] { steps += '3'; });
queue.EnqueueRelaxed([&steps] { steps += '2'; });
});
diff --git a/Firestore/core/test/firebase/firestore/util/executor_std_test.cc b/Firestore/core/test/firebase/firestore/util/executor_std_test.cc
index 43cad60..59c3c32 100644
--- a/Firestore/core/test/firebase/firestore/util/executor_std_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/executor_std_test.cc
@@ -70,7 +70,6 @@ TEST_F(ScheduleTest, PopIfDue_Delayed) {
schedule.Push(2, start_time + chr::milliseconds(3));
schedule.Push(3, start_time + chr::milliseconds(1));
- EXPECT_FALSE(schedule.PopIfDue().has_value());
std::this_thread::sleep_for(chr::milliseconds(5));
EXPECT_EQ(schedule.PopIfDue().value(), 3);
@@ -212,11 +211,16 @@ TEST_F(ScheduleTest, PopBlockingIsNotAffectedByIrrelevantRemovals) {
EXPECT_EQ(schedule.PopBlocking(), 1);
});
- while (schedule.empty()) {
+ // Wait (with timeout) for both values to appear in the schedule.
+ while (schedule.size() != 2) {
+ if (now() - start_time >= kTimeout) {
+ Abort();
+ }
std::this_thread::sleep_for(chr::milliseconds(1));
}
const auto maybe_removed =
schedule.RemoveIf([](const int v) { return v == 2; });
+ ASSERT_TRUE(maybe_removed.has_value());
EXPECT_EQ(maybe_removed.value(), 2);
ABORT_ON_TIMEOUT(future);
}
diff --git a/Firestore/core/test/firebase/firestore/util/executor_test.cc b/Firestore/core/test/firebase/firestore/util/executor_test.cc
index e277786..99bddce 100644
--- a/Firestore/core/test/firebase/firestore/util/executor_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/executor_test.cc
@@ -66,15 +66,19 @@ TEST_P(ExecutorTest, DestructorDoesNotBlockIfThereArePendingTasks) {
ABORT_ON_TIMEOUT(future);
}
+// TODO(varconst): this test is inherently flaky because it can't be guaranteed
+// that the enqueued asynchronous operation didn't finish before the code has
+// a chance to even enqueue the next operation. Delays are chosen so that the
+// test is unlikely to fail in practice. Need to revisit this.
TEST_P(ExecutorTest, CanScheduleOperationsInTheFuture) {
std::string steps;
executor->Execute([&steps] { steps += '1'; });
- Schedule(executor.get(), Executor::Milliseconds(5), [&] {
+ Schedule(executor.get(), Executor::Milliseconds(20), [&] {
steps += '4';
signal_finished();
});
- Schedule(executor.get(), Executor::Milliseconds(1),
+ Schedule(executor.get(), Executor::Milliseconds(10),
[&steps] { steps += '3'; });
executor->Execute([&steps] { steps += '2'; });