Merge branch 'master' into codable-master

Author: wilhuff

Firestore/Example/Firestore.xcodeproj/project.pbxproj

@@ -1478,7 +1478,6 @@
 		B6FB467B208E9A8200554BA2 /* async_queue_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = async_queue_test.cc; sourceTree = "<group>"; };
 		B6FB4680208EA0BE00554BA2 /* async_queue_libdispatch_test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = async_queue_libdispatch_test.mm; sourceTree = "<group>"; };
 		B6FB4681208EA0BE00554BA2 /* async_queue_std_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = async_queue_std_test.cc; sourceTree = "<group>"; };
-		B6FB4686208F9B9100554BA2 /* async_tests_util.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = async_tests_util.h; sourceTree = "<group>"; };
 		B6FB4687208F9B9100554BA2 /* executor_std_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = executor_std_test.cc; sourceTree = "<group>"; };
 		B6FB4688208F9B9100554BA2 /* executor_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = executor_test.cc; sourceTree = "<group>"; };
 		B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = executor_libdispatch_test.mm; sourceTree = "<group>"; };
@@ -1726,7 +1725,6 @@
 				B6FB4681208EA0BE00554BA2 /* async_queue_std_test.cc */,
 				B6FB467B208E9A8200554BA2 /* async_queue_test.cc */,
 				B6FB467A208E9A8200554BA2 /* async_queue_test.h */,
-				B6FB4686208F9B9100554BA2 /* async_tests_util.h */,
 				54740A521FC913E500713A1A /* autoid_test.cc */,
 				AB380D01201BC69F00D97691 /* bits_test.cc */,
 				548DB928200D59F600E00ABC /* comparison_test.cc */,

Firestore/Example/GoogleTest.podspec

@@ -17,7 +17,7 @@
 
 Pod::Spec.new do |s|
   s.name             = 'GoogleTest'
-  s.version          = '1.8.0'
+  s.version          = '1.8.1'
   s.summary          = 'Google Test'
 
   s.description      = <<-DESC

Firestore/core/test/firebase/firestore/remote/exponential_backoff_test.cc

@@ -20,7 +20,6 @@
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
 #include "Firestore/core/src/firebase/firestore/util/executor.h"
 #include "Firestore/core/test/firebase/firestore/testutil/async_testing.h"
-#include "Firestore/core/test/firebase/firestore/util/async_tests_util.h"
 #include "absl/memory/memory.h"
 #include "gtest/gtest.h"
 
@@ -30,13 +29,13 @@ namespace firebase {
 namespace firestore {
 namespace remote {
 
+using testutil::Expectation;
 using util::AsyncQueue;
 using util::Executor;
-using util::TestWithTimeoutMixin;
 using util::TimerId;
 
-class ExponentialBackoffTest : public TestWithTimeoutMixin,
-                               public testing::Test {
+class ExponentialBackoffTest : public testing::Test,
+                               public testutil::AsyncTest {
  public:
   ExponentialBackoffTest()
       : queue{testutil::AsyncQueueForTesting()},
@@ -51,12 +50,13 @@ class ExponentialBackoffTest : public TestWithTimeoutMixin,
 TEST_F(ExponentialBackoffTest, CanScheduleOperations) {
   EXPECT_FALSE(queue->IsScheduled(timer_id));
 
+  Expectation finished;
   queue->EnqueueBlocking([&] {
-    backoff.BackoffAndRun([&] { signal_finished(); });
+    backoff.BackoffAndRun(finished.AsCallback());
     EXPECT_TRUE(queue->IsScheduled(timer_id));
   });
 
-  EXPECT_TRUE(WaitForTestToFinish());
+  Await(finished);
   EXPECT_FALSE(queue->IsScheduled(timer_id));
 }
 
@@ -75,16 +75,17 @@ TEST_F(ExponentialBackoffTest, CanCancelOperations) {
 }
 
 TEST_F(ExponentialBackoffTest, SequentialCallsToBackoffAndRun) {
+  Expectation finished;
   queue->EnqueueBlocking([&] {
     backoff.BackoffAndRun([] {});
     backoff.BackoffAndRun([] {});
-    backoff.BackoffAndRun([&] { signal_finished(); });
+    backoff.BackoffAndRun(finished.AsCallback());
   });
 
   // The chosen value of initial_delay is large enough that it shouldn't be
   // realistically possible for backoff to finish already.
   queue->RunScheduledOperationsUntil(timer_id);
-  EXPECT_TRUE(WaitForTestToFinish());
+  Await(finished);
 }
 
 }  // namespace remote

Firestore/core/test/firebase/firestore/testutil/CMakeLists.txt

@@ -43,6 +43,7 @@ cc_library(
     firebase_firestore_nanopb
 
     ${TESTUTIL_DEPENDS}
+    GTest::GTest
     absl_time
     firebase_firestore_core
     firebase_firestore_model

Firestore/core/test/firebase/firestore/testutil/async_testing.cc

@@ -17,10 +17,12 @@
 #include "Firestore/core/test/firebase/firestore/testutil/async_testing.h"
 
 #include <string>
+#include <utility>
 
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
 #include "Firestore/core/src/firebase/firestore/util/executor.h"
 #include "absl/strings/str_cat.h"
+#include "gtest/gtest.h"
 
 namespace firebase {
 namespace firestore {
@@ -38,6 +40,65 @@ std::shared_ptr<util::AsyncQueue> AsyncQueueForTesting() {
   return std::make_shared<AsyncQueue>(ExecutorForTesting("worker"));
 }
 
+// MARK: - Expectation
+
+Expectation::Expectation()
+    : promise_(std::make_shared<std::promise<void>>()),
+      future_(promise_->get_future().share()) {
+}
+
+void Expectation::Fulfill() {
+  promise_->set_value();
+}
+
+std::function<void()> Expectation::AsCallback() const {
+  // Additional copy required because putting `promise_` in the capture list
+  // is not allowed. Generalized capture in C++14 would allow direct
+  // initialization.
+  auto promise = promise_;
+  return [promise] { promise->set_value(); };
+}
+
+// MARK: - AsyncTest
+
+std::future<void> AsyncTest::Async(std::function<void()> action) const {
+  std::packaged_task<void()> task(std::move(action));
+  auto future = task.get_future();
+
+  std::thread thread(std::move(task));
+  thread.detach();
+  return future;
+}
+
+void AsyncTest::Await(const std::future<void>& future,
+                      std::chrono::milliseconds timeout) const {
+  std::future_status result = future.wait_for(timeout);
+  if (result == std::future_status::ready) {
+    return;
+  }
+
+  ADD_FAILURE() << "Test timed out after " << timeout.count() << " ms";
+}
+
+void AsyncTest::Await(const std::shared_future<void>& future,
+                      std::chrono::milliseconds timeout) const {
+  std::future_status result = future.wait_for(timeout);
+  if (result == std::future_status::ready) {
+    return;
+  }
+
+  ADD_FAILURE() << "Test timed out after " << timeout.count() << " ms";
+}
+
+void AsyncTest::Await(Expectation& expectation,
+                      std::chrono::milliseconds timeout) const {
+  return Await(expectation.get_future(), timeout);
+}
+
+void AsyncTest::SleepFor(int millis) const {
+  std::this_thread::sleep_for(std::chrono::milliseconds(millis));
+}
+
 }  // namespace testutil
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/test/firebase/firestore/testutil/async_testing.h

@@ -17,7 +17,13 @@
 #ifndef FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_ASYNC_TESTING_H_
 #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_ASYNC_TESTING_H_
 
+#include <chrono>  // NOLINT(build/c++11)
+#include <functional>
+#include <future>  // NOLINT(build/c++11)
 #include <memory>
+#include <thread>  // NOLINT(build/c++11)
+
+#include "gtest/gtest.h"
 
 namespace firebase {
 namespace firestore {
@@ -46,6 +52,89 @@ std::unique_ptr<util::Executor> ExecutorForTesting(const char* name);
  */
 std::shared_ptr<util::AsyncQueue> AsyncQueueForTesting();
 
+constexpr auto kTimeout = std::chrono::seconds(5);
+
+/**
+ * An expected outcome of an asynchronous test.
+ */
+class Expectation {
+ public:
+  Expectation();
+
+  /**
+   * Marks this expectation as fulfilled.
+   *
+   * Only a single call to `Fulfill` is allowed for any given `Expectaction`. An
+   * exception is thrown if `Fulfill` is called more than once.
+   */
+  void Fulfill();
+
+  /**
+   * Returns a callback function, that when invoked, fullfills the expectation.
+   *
+   * The returned function has a lifetime that's independent of the Expectation
+   * that created it.
+   */
+  std::function<void()> AsCallback() const;
+
+  /**
+   * Returns a `shared_future` that represents the completion of this
+   * Expectation.
+   */
+  const std::shared_future<void>& get_future() const {
+    return future_;
+  }
+
+ private:
+  std::shared_ptr<std::promise<void>> promise_;
+  std::shared_future<void> future_;
+};
+
+/**
+ * A mixin that supplies utilities for safely writing asynchronous tests.
+ */
+class AsyncTest {
+ public:
+  AsyncTest() = default;
+
+  std::future<void> Async(std::function<void()> action) const;
+
+  /**
+   * Waits for the future to become ready.
+   *
+   * Fails the current test if the timeout occurs.
+   */
+  void Await(const std::future<void>& future,
+             std::chrono::milliseconds timeout = kTimeout) const;
+
+  /**
+   * Waits for the shared future to become ready.
+   *
+   * Fails the current test if the timeout occurs.
+   */
+  void Await(const std::shared_future<void>& future,
+             std::chrono::milliseconds timeout = kTimeout) const;
+
+  /**
+   * Waits for the expectation to become fulfilled.
+   *
+   * Fails the current test if the timeout occurs.
+   */
+  void Await(Expectation& expectation,  // NOLINT(runtime/references)
+             std::chrono::milliseconds timeout = kTimeout) const;
+
+  /**
+   * Sleeps the current thread for the given number of milliseconds.
+   */
+  void SleepFor(int millis) const;
+
+ private:
+  testing::ScopedTrace trace_{
+      "Test case name", 1,
+      testing::Message()
+          << testing::UnitTest::GetInstance()->current_test_info()->name()};
+};
+
 }  // namespace testutil
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/test/firebase/firestore/util/CMakeLists.txt

@@ -28,7 +28,6 @@ cc_test(
     async_queue_std_test.cc
     async_queue_test.cc
     async_queue_test.h
-    async_tests_util.h
     executor_std_test.cc
     executor_test.cc
     executor_test.h
@@ -44,7 +43,6 @@ if(HAVE_LIBDISPATCH)
       async_queue_libdispatch_test.mm
       async_queue_test.cc
       async_queue_test.h
-      async_tests_util.h
       executor_libdispatch_test.mm
       executor_test.cc
       executor_test.h
@@ -189,6 +187,7 @@ cc_library(
     firebase_firestore_auth
     firebase_firestore_core
     firebase_firestore_remote
+    firebase_firestore_testutil
     firebase_firestore_util
     grpc++
 )

Firestore/core/test/firebase/firestore/util/async_queue_libdispatch_test.mm

@@ -25,9 +25,10 @@
 namespace firebase {
 namespace firestore {
 namespace util {
-
 namespace {
 
+using testutil::Expectation;
+
 dispatch_queue_t CreateDispatchQueue() {
   return dispatch_queue_create("AsyncQueueTests", DISPATCH_QUEUE_SERIAL);
 }
@@ -47,8 +48,8 @@ dispatch_queue_t CreateDispatchQueue() {
                         AsyncQueueTest,
                         ::testing::Values(CreateExecutorLibdispatch));
 
-class AsyncQueueTestLibdispatchOnly : public TestWithTimeoutMixin,
-                                      public ::testing::Test {
+class AsyncQueueTestLibdispatchOnly : public ::testing::Test,
+                                      public testutil::AsyncTest {
  public:
   AsyncQueueTestLibdispatchOnly()
       : underlying_queue{CreateDispatchQueue()},
@@ -63,10 +64,10 @@ dispatch_queue_t CreateDispatchQueue() {
 // interacts with raw usage of libdispatch.
 
 TEST_F(AsyncQueueTestLibdispatchOnly, SameQueueIsAllowedForUnownedActions) {
-  internal::DispatchAsync(underlying_queue, [this] {
-    queue.Enqueue([this] { signal_finished(); });
-  });
-  EXPECT_TRUE(WaitForTestToFinish());
+  Expectation ran;
+  internal::DispatchAsync(underlying_queue,
+                          [&] { queue.Enqueue(ran.AsCallback()); });
+  Await(ran);
 }
 
 TEST_F(AsyncQueueTestLibdispatchOnly,