Miscellaneous test-only fixes

  * Avoid dereferencing awaited pointers after Await has failed, preventing
    crashes after test timeouts.
  * Await `CollectionReference::Add`, preventing nondeterminism in tests.

PiperOrigin-RevId: 320449430

Author: wilhuff

firestore/src/tests/firestore_integration_test.cc

@@ -141,8 +141,7 @@ void FirestoreIntegrationTest::WriteDocument(DocumentReference reference,
                                              const MapFieldValue& data) const {
   Future<void> future = reference.Set(data);
   Await(future);
-  EXPECT_EQ(FutureStatus::kFutureStatusComplete, future.status());
-  EXPECT_EQ(0, future.error()) << DescribeFailedFuture(future) << std::endl;
+  FailIfUnsuccessful("WriteDocument", future);
 }
 
 void FirestoreIntegrationTest::WriteDocuments(
@@ -157,28 +156,29 @@ DocumentSnapshot FirestoreIntegrationTest::ReadDocument(
     const DocumentReference& reference) const {
   Future<DocumentSnapshot> future = reference.Get();
   const DocumentSnapshot* result = Await(future);
-  EXPECT_EQ(FutureStatus::kFutureStatusComplete, future.status());
-  EXPECT_EQ(0, future.error()) << DescribeFailedFuture(future) << std::endl;
-  EXPECT_NE(nullptr, result) << DescribeFailedFuture(future) << std::endl;
-  return *result;
+  if (FailIfUnsuccessful("ReadDocument", future)) {
+    return {};
+  } else {
+    return *result;
+  }
 }
 
 QuerySnapshot FirestoreIntegrationTest::ReadDocuments(
     const Query& reference) const {
   Future<QuerySnapshot> future = reference.Get();
   const QuerySnapshot* result = Await(future);
-  EXPECT_EQ(FutureStatus::kFutureStatusComplete, future.status());
-  EXPECT_EQ(0, future.error()) << DescribeFailedFuture(future) << std::endl;
-  EXPECT_NE(nullptr, result) << DescribeFailedFuture(future) << std::endl;
-  return *result;
+  if (FailIfUnsuccessful("ReadDocuments", future)) {
+    return {};
+  } else {
+    return *result;
+  }
 }
 
 void FirestoreIntegrationTest::DeleteDocument(
     DocumentReference reference) const {
   Future<void> future = reference.Delete();
   Await(future);
-  EXPECT_EQ(FutureStatus::kFutureStatusComplete, future.status());
-  EXPECT_EQ(0, future.error()) << DescribeFailedFuture(future) << std::endl;
+  FailIfUnsuccessful("DeleteDocument", future);
 }
 
 std::vector<std::string> FirestoreIntegrationTest::QuerySnapshotToIds(
@@ -210,6 +210,29 @@ void FirestoreIntegrationTest::Await(const Future<void>& future) {
   }
 }
 
+/* static */
+bool FirestoreIntegrationTest::FailIfUnsuccessful(const char* operation,
+                                                  const FutureBase& future) {
+  if (future.status() != FutureStatus::kFutureStatusComplete) {
+    ADD_FAILURE() << operation << " timed out: " << DescribeFailedFuture(future)
+                  << std::endl;
+    return true;
+  } else if (future.error() != Error::kErrorOk) {
+    ADD_FAILURE() << operation << "failed: " << DescribeFailedFuture(future)
+                  << std::endl;
+    return true;
+  } else {
+    return false;
+  }
+}
+
+/* static */
+std::string FirestoreIntegrationTest::DescribeFailedFuture(
+    const FutureBase& future) {
+  return "WARNING: Future failed. Error code " +
+         std::to_string(future.error()) + ", message " + future.error_message();
+}
+
 void FirestoreIntegrationTest::TerminateAndRelease(Firestore* firestore) {
   Await(firestore->Terminate());
   Release(firestore);

firestore/src/tests/firestore_integration_test.h

@@ -252,12 +252,12 @@ class FirestoreIntegrationTest : public testing::Test {
     EXPECT_GT(cycles, 0) << "Waiting listener timed out.";
   }
 
-  template <typename T>
-  static std::string DescribeFailedFuture(const Future<T>& future) {
-    return "WARNING: Future failed. Error code " +
-           std::to_string(future.error()) + ", message " +
-           future.error_message();
-  }
+  // Fails the current test if the given future did not complete or contained an
+  // error. Returns true if the future has failed.
+  static bool FailIfUnsuccessful(const char* operation,
+                                 const FutureBase& future);
+
+  static std::string DescribeFailedFuture(const FutureBase& future);
 
   // Creates a new Firestore instance, without any caching, using a uniquely-
   // generated app_name.

firestore/src/tests/query_network_test.cc

@@ -57,8 +57,10 @@ class QueryNetworkTest : public FirestoreIntegrationTest {
     EXPECT_TRUE(accumulator.AwaitRemoteEvent().empty());
 
     Await(firestore()->DisableNetwork());
-    collection.Add(MapFieldValue{{"foo", FieldValue::ServerTimestamp()}});
+    auto added =
+        collection.Add(MapFieldValue{{"foo", FieldValue::ServerTimestamp()}});
     Await(firestore()->EnableNetwork());
+    Await(added);
 
     QuerySnapshot snapshot = accumulator.AwaitServerEvent();
     EXPECT_FALSE(snapshot.empty());

firestore/src/tests/query_test.cc

@@ -438,7 +438,7 @@ TEST_F(FirestoreIntegrationTest, TestCanQueryByDocumentIdUsingRefs) {
 
 TEST_F(FirestoreIntegrationTest, TestCanQueryWithAndWithoutDocumentKey) {
   CollectionReference collection = Collection();
-  collection.Add({});
+  Await(collection.Add({}));
   QuerySnapshot snapshot1 = ReadDocuments(collection.OrderBy(
       FieldPath::DocumentId(), Query::Direction::kAscending));
   QuerySnapshot snapshot2 = ReadDocuments(collection);