Skip to content

Improve readability of a test in ApkUpdaterTest #4518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 4, 2023
Merged

Improve readability of a test in ApkUpdaterTest #4518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
75108e0
Rename CountDownLatch and inline more of the setup to improve readabi…
lfkellogg Jan 4, 2023
0133e0a
Use a latch for the other usage of awaitProgressEvents()
lfkellogg Jan 4, 2023
385d2cc
Refactor awaitProgressEvents into awaitCondition
lfkellogg Jan 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions ...ppdistribution/src/test/java/com/google/firebase/appdistribution/impl/ApkUpdaterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");
}
Expand All @@ -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());
Expand Down
27 changes: 23 additions & 4 deletions ...java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
58 changes: 18 additions & 40 deletions ...ase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}

/**
Expand Down