Merge 385d2cc into 8c83cfc

Author: lfkellogg

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/ApkUpdaterTest.java

@@ -15,7 +15,7 @@
 package com.google.firebase.appdistribution.impl;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.firebase.appdistribution.impl.TestUtils.awaitProgressEvents;
+import static com.google.firebase.appdistribution.impl.TestUtils.awaitCondition;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
@@ -42,6 +42,7 @@
 import com.google.firebase.appdistribution.UpdateProgress;
 import com.google.firebase.appdistribution.UpdateStatus;
 import com.google.firebase.appdistribution.UpdateTask;
+import com.google.firebase.concurrent.FirebaseExecutors;
 import com.google.firebase.concurrent.TestOnlyExecutors;
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -179,19 +180,18 @@ public void updateApk_whenCannotReadInputStream_setsDownloadFailure() throws Exc
 
   @Test
   public void updateApk_whenSuccessfullyUpdated_notificationsSetCorrectly()
-      throws FirebaseAppDistributionException, ExecutionException, InterruptedException,
-          IOException {
+      throws FirebaseAppDistributionException, InterruptedException, IOException {
     doReturn(new ByteArrayInputStream(TEST_FILE.getBytes()))
         .when(mockHttpsUrlConnection)
         .getInputStream();
     // Block thread actually making the request on a latch, which gives us time to add listeners to
     // the returned UpdateTask in time to get all the progress updates
-    CountDownLatch mockConnectionLatch = new CountDownLatch(1);
+    CountDownLatch countDownLatch = new CountDownLatch(1);
     when(mockHttpsUrlConnectionFactory.openConnection(TEST_URL))
         .thenAnswer(
             invocation -> {
               try {
-                mockConnectionLatch.await();
+                countDownLatch.await();
               } catch (InterruptedException e) {
                 throw new AssertionError("Interrupted while waiting in mock");
               }
@@ -200,11 +200,15 @@ public void updateApk_whenSuccessfullyUpdated_notificationsSetCorrectly()
     doNothing().when(apkUpdater).validateJarFile(any(), anyLong(), anyBoolean(), anyLong());
     when(mockApkInstaller.installApk(any(), any())).thenReturn(Tasks.forResult(null));
 
+    // Start the update
     UpdateTask updateTask = apkUpdater.updateApk(TEST_RELEASE, true);
-    List<UpdateProgress> events =
-        awaitProgressEvents(updateTask, 3, mockConnectionLatch::countDown);
 
-    assertThat(events).hasSize(3);
+    // Listen for progress events
+    List<UpdateProgress> events = new ArrayList<>();
+    updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), events::add);
+    countDownLatch.countDown();
+    awaitCondition(() -> events.size() == 3);
+
     assertThat(events.get(0).getUpdateStatus()).isEqualTo(UpdateStatus.PENDING);
     assertThat(events.get(1).getUpdateStatus()).isEqualTo(UpdateStatus.DOWNLOADING);
     assertThat(events.get(1).getApkBytesDownloaded()).isEqualTo(TEST_FILE.length());

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java

@@ -33,7 +33,7 @@
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.RELEASE_NAME_KEY;
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.SCREENSHOT_URI_KEY;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitAsyncOperations;
-import static com.google.firebase.appdistribution.impl.TestUtils.awaitProgressEvents;
+import static com.google.firebase.appdistribution.impl.TestUtils.awaitCondition;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTaskFailure;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTermination;
@@ -86,6 +86,7 @@
 import com.google.firebase.installations.InstallationTokenResult;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -364,11 +365,29 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh
   @Test
   public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown()
       throws InterruptedException {
-    when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null));
-
+    // Block thread making the request on a latch, which gives us time to add listeners to the
+    // returned UpdateTask in time to get all the progress updates
+    CountDownLatch countDownLatch = new CountDownLatch(1);
+    when(mockNewReleaseFetcher.checkForNewRelease())
+        .thenAnswer(
+            invocation -> {
+              try {
+                countDownLatch.await();
+              } catch (InterruptedException e) {
+                throw new AssertionError("Interrupted while waiting in mock");
+              }
+              return Tasks.forResult(null);
+            });
+
+    // Start the update
     UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
 
-    List<UpdateProgress> progressEvents = awaitProgressEvents(task, 1);
+    // Listen for progress events
+    List<UpdateProgress> progressEvents = new ArrayList<>();
+    task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add);
+    countDownLatch.countDown();
+    awaitCondition(() -> progressEvents.size() == 1);
+
     assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus());
     assertNull(ShadowAlertDialog.getLatestAlertDialog());
   }

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java

@@ -28,25 +28,25 @@
 import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.appdistribution.FirebaseAppDistributionException;
 import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
-import com.google.firebase.appdistribution.UpdateProgress;
-import com.google.firebase.appdistribution.UpdateTask;
 import com.google.firebase.appdistribution.impl.FirebaseAppDistributionLifecycleNotifier.ActivityConsumer;
 import com.google.firebase.appdistribution.impl.FirebaseAppDistributionLifecycleNotifier.ActivityFunction;
-import com.google.firebase.concurrent.FirebaseExecutors;
-import com.google.firebase.concurrent.TestOnlyExecutors;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.function.BooleanSupplier;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mockito.stubbing.Answer;
 
 final class TestUtils {
+
+  private static final int AWAIT_TERMINATION_TIMEOUT_MS = 100;
+  private static final int AWAIT_CONDITION_TIMEOUT_MS = 500;
+  private static final int SLEEP_MS = 50;
+
   private TestUtils() {}
 
   static void awaitTaskFailure(Task task, Status status, String messageSubstring) {
@@ -89,7 +89,7 @@ static <T> T awaitTask(Task<T> task)
   }
 
   static void awaitTermination(ExecutorService executorService) throws InterruptedException {
-    executorService.awaitTermination(100, TimeUnit.MILLISECONDS);
+    executorService.awaitTermination(AWAIT_TERMINATION_TIMEOUT_MS, TimeUnit.MILLISECONDS);
   }
 
   static void awaitAsyncOperations(ExecutorService executorService) throws InterruptedException {
@@ -101,41 +101,19 @@ static void awaitAsyncOperations(ExecutorService executorService) throws Interru
     shadowOf(getMainLooper()).idle();
   }
 
-  /** Await a specified number of progress events on an {@link UpdateTask}. */
-  static List<UpdateProgress> awaitProgressEvents(UpdateTask updateTask, int count)
-      throws InterruptedException {
-    return awaitProgressEvents(updateTask, count, /* setupFunction= */ () -> {});
+  static void awaitCondition(BooleanSupplier condition) throws InterruptedException {
+    long start = System.currentTimeMillis();
+    while (elapsedTime(start) < AWAIT_CONDITION_TIMEOUT_MS) {
+      if (condition.getAsBoolean()) {
+        return;
+      }
+      Thread.sleep(SLEEP_MS);
+    }
+    throw new AssertionError("Timed out waiting for condition");
   }
 
-  /**
-   * Await a specified number of progress events on an {@link UpdateTask}, executing a {@link
-   * Runnable} after adding the listeners.
-   *
-   * @param updateTask the update task
-   * @param count the number of progress events to await
-   * @param setupFunction a function to run after adding the listeners to the update task, but
-   *     before we await the progress events. This is useful for kicking off delayed processes that
-   *     would otherwise start emitting progress events that need to be captured.
-   */
-  static List<UpdateProgress> awaitProgressEvents(
-      UpdateTask updateTask, int count, Runnable setupFunction) throws InterruptedException {
-    List<UpdateProgress> progressEvents = new ArrayList<>();
-    updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add);
-    setupFunction.run();
-    ExecutorService executor = TestOnlyExecutors.blocking();
-    executor.execute(
-        () -> {
-          while (progressEvents.size() < count) {
-            try {
-              Thread.sleep(50);
-            } catch (InterruptedException e) {
-              throw new RuntimeException("Interrupted while waiting for progress events", e);
-            }
-          }
-        });
-    executor.awaitTermination(500, TimeUnit.MILLISECONDS);
-    assertThat(progressEvents).hasSize(count);
-    return progressEvents;
+  private static long elapsedTime(long start) {
+    return System.currentTimeMillis() - start;
   }
 
   /**