Deflake some integration tests including storage and FIS. (#295)

This test adds a convenience method RunFlakyBlock to firebase_test_framework that allows you to run a block multiple times, with exponential backoff until it succeeds (or fails a few times). This allows you to perform tests that have a chance of failure, such as uploading/downloading large files.

Author: jonsimantov

installations/integration_test/src/integration_test.cc

@@ -41,6 +41,7 @@
 namespace firebase_testapp_automated {
 
 using app_framework::LogDebug;
+using app_framework::LogError;
 
 using app_framework::ProcessEvents;
 using firebase_test_framework::FirebaseTest;
@@ -143,34 +144,90 @@ TEST_F(FirebaseInstallationsTest, TestCanGetId) {
 }
 
 TEST_F(FirebaseInstallationsTest, TestGettingIdTwiceMatches) {
-  firebase::Future<std::string> id = installations_->GetId();
-  WaitForCompletion(id, "GetId");
-  EXPECT_NE(*id.result(), "");
-  std::string first_id = *id.result();
-  id = installations_->GetId();
-  WaitForCompletion(id, "GetId 2");
-  EXPECT_EQ(*id.result(), first_id);
+  if (!RunFlakyBlock(
+          [](firebase::installations::Installations* installations) {
+            firebase::Future<std::string> id = installations->GetId();
+            WaitForCompletionAnyResult(id, "GetId");
+            if (id.error() != 0) {
+              LogError("GetId returned error %d: %s", id.error(),
+                       id.error_message());
+              return false;
+            }
+            if (*id.result() == "") {
+              LogError("GetId returned blank");
+              return false;
+            }
+            std::string first_id = *id.result();
+            id = installations->GetId();
+            WaitForCompletionAnyResult(id, "GetId 2");
+            if (id.error() != 0) {
+              LogError("GetId 2 returned error %d: %s", id.error(),
+                       id.error_message());
+              return false;
+            }
+            if (*id.result() != first_id) {
+              LogError(
+                  "GetId 2 returned non-matching ID: first(%s) vs second(%s)",
+                  first_id.c_str(), id.result()->c_str());
+              return false;
+            }
+            return true;
+          },
+          installations_)) {
+    FAIL() << "Test failed, check error log for details.";
+  }
 }
 
 TEST_F(FirebaseInstallationsTest, TestDeleteGivesNewIdNextTime) {
-  firebase::Future<std::string> id = installations_->GetId();
-  WaitForCompletion(id, "GetId");
-  EXPECT_NE(*id.result(), "");
-  std::string first_id = *id.result();
-
-  firebase::Future<void> del = installations_->Delete();
-  WaitForCompletion(del, "Delete");
-
-  // Ensure that we now get a different installations id.
-  id = installations_->GetId();
-  WaitForCompletion(id, "GetId 2");
-  EXPECT_NE(*id.result(), "");
+  if (!RunFlakyBlock(
+          [](firebase::installations::Installations* installations) {
+            firebase::Future<std::string> id = installations->GetId();
+            WaitForCompletionAnyResult(id, "GetId");
+            if (id.error() != 0) {
+              LogError("GetId returned error %d: %s", id.error(),
+                       id.error_message());
+              return false;
+            }
+            if (*id.result() == "") {
+              LogError("GetId returned blank");
+              return false;
+            }
+            std::string first_id = *id.result();
+
+            firebase::Future<void> del = installations->Delete();
+            WaitForCompletionAnyResult(del, "Delete");
+            if (del.error() != 0) {
+              LogError("Delete returned error %d: %s", id.error(),
+                       id.error_message());
+              return false;
+            }
+
+            // Ensure that we now get a different installations id.
+            id = installations->GetId();
+            WaitForCompletionAnyResult(id, "GetId 2");
+            if (id.error() != 0) {
+              LogError("GetId 2 returned error %d: %s", id.error(),
+                       id.error_message());
+              return false;
+            }
+            if (*id.result() == "") {
+              LogError("GetId 2 returned blank");
+              return false;
+            }
 #if defined(__ANDROID__) || (defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE)
-  // Desktop is a stub and returns the same ID, but on mobile it should
-  // return a new ID.
-  EXPECT_NE(*id.result(), first_id);
+            // Desktop is a stub and returns the same ID, but on mobile it
+            // should return a new ID.
+            if (*id.result() == first_id) {
+              LogError("IDs match (should be different): %s", first_id.c_str());
+              return false;
+            }
 #endif  // defined(__ANDROID__) || (defined(TARGET_OS_IPHONE) &&
         // TARGET_OS_IPHONE)
+            return true;
+          },
+          installations_)) {
+    FAIL() << "Test failed, check error log for details.";
+  }
 }
 
 TEST_F(FirebaseInstallationsTest, TestCanGetToken) {

messaging/integration_test/src/integration_test.cc

@@ -184,6 +184,10 @@ void FirebaseMessagingTest::TearDownTestSuite() {
   LogDebug("Shutdown Firebase App.");
   delete shared_app_;
   shared_app_ = nullptr;
+
+  // On iOS/FTL, most or all of the tests are skipped, so add a delay so the app
+  // doesn't finish too quickly, as this makes test results flaky.
+  ProcessEvents(1000);
 }
 
 FirebaseMessagingTest::FirebaseMessagingTest() {

scripts/gha/summarize_test_results.py

@@ -308,7 +308,8 @@ def main(argv):
         # Don't process this if meet another TEST SUMMARY
         if "TEST SUMMARY" in test_failure_line: break
         # Only get the lines showing paths.
-        if "/firebase-cpp-sdk/" not in test_failure_line: continue
+        if ("/firebase-cpp-sdk/" not in test_failure_line and
+            "\\firebase-cpp-sdk\\" not in test_failure_line): continue
         test_filename = "";
         if "log tail" in test_failure_line:
           test_filename = re.match(r'^(.*) log tail', test_failure_line).group(1)
@@ -322,6 +323,7 @@ def main(argv):
           test_filename = re.match(r'^(.*integration_test\.app)', test_failure_line).group(1)
 
         if test_filename:
+          test_filename = test_filename.replace('\\', '/')
           m2 = re.search(r'/ta/(firebase)?([^/]+)/iti?/', test_filename, re.IGNORECASE)
           if not m2: m2 = re.search(r'/testapps/(firebase)?([^/]+)/integration_test', test_filename, re.IGNORECASE)
           if m2: