From 2d494861ec61672af9fdde943cfd42cc69a5acda Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 28 Apr 2020 18:00:50 +0100 Subject: [PATCH] Cleanup and document TestDownloadManagerListener PiperOrigin-RevId: 308843488 --- .../offline/DownloadManagerTest.java | 55 ++++---- .../dash/offline/DownloadManagerDashTest.java | 36 ++--- .../dash/offline/DownloadServiceDashTest.java | 12 +- .../testutil/TestDownloadManagerListener.java | 125 +++++++++--------- 4 files changed, 105 insertions(+), 123 deletions(-) diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java index 9ea1445785..2d698ffc0d 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java @@ -100,7 +100,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertDownloaderCount(ID1, 1); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertThat(downloadManager.getCurrentDownloads()).isEmpty(); } @@ -117,7 +117,7 @@ public class DownloadManagerTest { assertRemoved(ID1); assertDownloaderCount(ID1, 2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertThat(downloadManager.getCurrentDownloads()).isEmpty(); } @@ -134,7 +134,7 @@ public class DownloadManagerTest { downloader.assertStartCount(MIN_RETRY_COUNT + 1); assertFailed(ID1); - downloadManagerListener.blockUntilTasksComplete(); + downloadManagerListener.blockUntilIdle(); assertCurrentDownloadCount(0); } @@ -153,7 +153,7 @@ public class DownloadManagerTest { downloader.assertStartCount(MIN_RETRY_COUNT + 1); assertCompleted(ID1); - downloadManagerListener.blockUntilTasksComplete(); + downloadManagerListener.blockUntilIdle(); assertCurrentDownloadCount(0); } @@ -174,7 +174,7 @@ public class DownloadManagerTest { downloader.assertStartCount(tooManyRetries + 1); assertCompleted(ID1); - downloadManagerListener.blockUntilTasksComplete(); + downloadManagerListener.blockUntilIdle(); } @Test @@ -192,7 +192,7 @@ public class DownloadManagerTest { downloader2.unblock(); downloader2.assertCompleted(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -212,7 +212,7 @@ public class DownloadManagerTest { downloader2.unblock(); downloader2.assertCompleted(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -230,7 +230,7 @@ public class DownloadManagerTest { assertRemoved(ID1); assertDownloaderCount(ID1, 2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -238,13 +238,13 @@ public class DownloadManagerTest { // Finish one download and keep one running. postDownloadRequest(ID1); getDownloader(ID1, 0).unblock(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); postDownloadRequest(ID2); postRemoveAllRequest(); getDownloader(ID1, 1).unblock(); getDownloader(ID2, 1).unblock(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertRemoved(ID1); assertRemoved(ID2); @@ -270,7 +270,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertDownloaderCount(ID1, 2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -287,7 +287,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertCompleted(ID2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -307,7 +307,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertCompleted(ID2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -327,7 +327,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertRemoved(ID2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -351,7 +351,7 @@ public class DownloadManagerTest { assertCompleted(ID1); assertCompleted(ID2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -412,7 +412,7 @@ public class DownloadManagerTest { downloader5.assertStarted(); downloader5.unblock(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -431,7 +431,7 @@ public class DownloadManagerTest { downloader.assertStarted(); downloader.unblock(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -450,7 +450,7 @@ public class DownloadManagerTest { downloader.unblock(); assertRemoving(ID1); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -476,7 +476,7 @@ public class DownloadManagerTest { downloader2.assertStarted(); downloader2.unblock(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); } @Test @@ -563,8 +563,7 @@ public class DownloadManagerTest { downloadManager.setMinRetryCount(MIN_RETRY_COUNT); downloadManager.setRequirements(new Requirements(0)); downloadManager.resumeDownloads(); - downloadManagerListener = - new TestDownloadManagerListener(downloadManager, dummyMainThread); + downloadManagerListener = new TestDownloadManagerListener(downloadManager); }); downloadManagerListener.blockUntilInitialized(); } catch (Throwable throwable) { @@ -632,31 +631,31 @@ public class DownloadManagerTest { } private void assertDownloading(String id) { - downloadManagerListener.assertState(id, Download.STATE_DOWNLOADING, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_DOWNLOADING); } private void assertCompleted(String id) { - downloadManagerListener.assertState(id, Download.STATE_COMPLETED, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_COMPLETED); } private void assertRemoving(String id) { - downloadManagerListener.assertState(id, Download.STATE_REMOVING, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_REMOVING); } private void assertFailed(String id) { - downloadManagerListener.assertState(id, Download.STATE_FAILED, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_FAILED); } private void assertQueued(String id) { - downloadManagerListener.assertState(id, Download.STATE_QUEUED, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_QUEUED); } private void assertStopped(String id) { - downloadManagerListener.assertState(id, Download.STATE_STOPPED, TIMEOUT_MS); + downloadManagerListener.assertState(id, Download.STATE_STOPPED); } private void assertRemoved(String id) { - downloadManagerListener.assertRemoved(id, TIMEOUT_MS); + downloadManagerListener.assertRemoved(id); } private void assertCurrentDownloadCount(int expectedCount) { diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java index 616a743865..b16d5727b1 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java @@ -147,14 +147,14 @@ public class DownloadManagerDashTest { dummyMainThread.runOnMainThread(this::createDownloadManager); // Block on the test thread. - blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCachedData(cache, fakeDataSet); } @Test public void handleDownloadRequest_downloadSuccess() throws Throwable { handleDownloadRequest(fakeStreamKey1, fakeStreamKey2); - blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCachedData(cache, new RequestSet(fakeDataSet).useBoundedDataSpecFor("audio_init_data")); } @@ -162,7 +162,7 @@ public class DownloadManagerDashTest { public void handleDownloadRequest_withRequest_downloadSuccess() throws Throwable { handleDownloadRequest(fakeStreamKey1); handleDownloadRequest(fakeStreamKey2); - blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCachedData(cache, new RequestSet(fakeDataSet).useBoundedDataSpecFor("audio_init_data")); } @@ -173,23 +173,17 @@ public class DownloadManagerDashTest { .appendReadAction(() -> handleDownloadRequest(fakeStreamKey2)) .appendReadData(TestUtil.buildTestData(5)) .endData(); - handleDownloadRequest(fakeStreamKey1); - - blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCachedData(cache, new RequestSet(fakeDataSet).useBoundedDataSpecFor("audio_init_data")); } @Test public void handleRemoveAction_blockUntilTaskCompleted_noDownloadedData() throws Throwable { handleDownloadRequest(fakeStreamKey1); - - blockUntilTasksCompleteAndThrowAnyDownloadError(); - + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); handleRemoveAction(); - - blockUntilTasksCompleteAndThrowAnyDownloadError(); - + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCacheEmpty(cache); } @@ -197,9 +191,7 @@ public class DownloadManagerDashTest { public void handleRemoveAction_beforeDownloadFinish_noDownloadedData() throws Throwable { handleDownloadRequest(fakeStreamKey1); handleRemoveAction(); - - blockUntilTasksCompleteAndThrowAnyDownloadError(); - + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCacheEmpty(cache); } @@ -211,23 +203,15 @@ public class DownloadManagerDashTest { .appendReadAction(downloadInProgressLatch::countDown) .appendReadData(TestUtil.buildTestData(5)) .endData(); - handleDownloadRequest(fakeStreamKey1); - assertThat(downloadInProgressLatch.await(ASSERT_TRUE_TIMEOUT_MS, TimeUnit.MILLISECONDS)) .isTrue(); handleRemoveAction(); - - blockUntilTasksCompleteAndThrowAnyDownloadError(); - + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCacheEmpty(cache); } - private void blockUntilTasksCompleteAndThrowAnyDownloadError() throws Throwable { - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); - } - private void handleDownloadRequest(StreamKey... keys) { DownloadRequest request = getDownloadRequest(keys); runOnMainThread(() -> downloadManager.addDownload(request)); @@ -262,9 +246,7 @@ public class DownloadManagerDashTest { new DownloadManager( ApplicationProvider.getApplicationContext(), downloadIndex, downloaderFactory); downloadManager.setRequirements(new Requirements(0)); - - downloadManagerListener = - new TestDownloadManagerListener(downloadManager, dummyMainThread); + downloadManagerListener = new TestDownloadManagerListener(downloadManager); downloadManager.resumeDownloads(); }); } diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java index 42b0a2e7c2..0073b3bfaf 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java @@ -121,8 +121,7 @@ public class DownloadServiceDashTest { final DownloadManager dashDownloadManager = new DownloadManager( ApplicationProvider.getApplicationContext(), downloadIndex, downloaderFactory); - downloadManagerListener = - new TestDownloadManagerListener(dashDownloadManager, dummyMainThread); + downloadManagerListener = new TestDownloadManagerListener(dashDownloadManager); dashDownloadManager.resumeDownloads(); dashDownloadService = @@ -160,7 +159,7 @@ public class DownloadServiceDashTest { downloadKeys(fakeStreamKey1); downloadKeys(fakeStreamKey2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCachedData(cache, fakeDataSet); } @@ -170,11 +169,11 @@ public class DownloadServiceDashTest { public void removeAction() throws Throwable { downloadKeys(fakeStreamKey1, fakeStreamKey2); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); removeAll(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCacheEmpty(cache); } @@ -187,7 +186,7 @@ public class DownloadServiceDashTest { removeAll(); - downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError(); + downloadManagerListener.blockUntilIdleAndThrowAnyFailure(); assertCacheEmpty(cache); } @@ -221,5 +220,4 @@ public class DownloadServiceDashTest { dashDownloadService.onStartCommand(startIntent, 0, 0); }); } - } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java index 1d5e9dc779..56e6b7cacc 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java @@ -18,55 +18,87 @@ package com.google.android.exoplayer2.testutil; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import android.os.Handler; import androidx.annotation.Nullable; import com.google.android.exoplayer2.offline.Download; -import com.google.android.exoplayer2.offline.Download.State; import com.google.android.exoplayer2.offline.DownloadManager; import com.google.android.exoplayer2.util.ConditionVariable; import java.util.HashMap; -import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; -/** A {@link DownloadManager.Listener} for testing. */ +/** + * Allows tests to block for, and assert properties of, calls from a {@link DownloadManager} to its + * {@link DownloadManager.Listener}. + */ public final class TestDownloadManagerListener implements DownloadManager.Listener { private static final int TIMEOUT_MS = 10_000; private static final int STATE_REMOVED = -1; private final DownloadManager downloadManager; - private final DummyMainThread dummyMainThread; - private final HashMap> downloadStates; + private final HashMap> downloadStates; private final ConditionVariable initializedCondition; private final ConditionVariable idleCondition; @Download.FailureReason private int failureReason; - public TestDownloadManagerListener( - DownloadManager downloadManager, DummyMainThread dummyMainThread) { + public TestDownloadManagerListener(DownloadManager downloadManager) { this.downloadManager = downloadManager; - this.dummyMainThread = dummyMainThread; downloadStates = new HashMap<>(); initializedCondition = TestUtil.createRobolectricConditionVariable(); idleCondition = TestUtil.createRobolectricConditionVariable(); downloadManager.addListener(this); } - @Nullable - public Integer pollStateChange(String taskId, long timeoutMs) throws InterruptedException { - return getStateQueue(taskId).poll(timeoutMs, TimeUnit.MILLISECONDS); + /** Blocks until the manager is initialized. */ + public void blockUntilInitialized() throws InterruptedException { + assertThat(initializedCondition.block(TIMEOUT_MS)).isTrue(); } + /** Blocks until the manager is idle. */ + public void blockUntilIdle() throws InterruptedException { + idleCondition.close(); + // If the manager is already idle the condition will be opened by the code immediately below. + // Else it will be opened by onIdle(). + ConditionVariable checkedOnMainThread = TestUtil.createRobolectricConditionVariable(); + new Handler(downloadManager.getApplicationLooper()) + .post( + () -> { + if (downloadManager.isIdle()) { + idleCondition.open(); + checkedOnMainThread.open(); + } + }); + checkedOnMainThread.block(TIMEOUT_MS); + assertThat(idleCondition.block(TIMEOUT_MS)).isTrue(); + } + + /** Blocks until the manager is idle and throws if any of the downloads failed. */ + public void blockUntilIdleAndThrowAnyFailure() throws Exception { + blockUntilIdle(); + if (failureReason != Download.FAILURE_REASON_NONE) { + throw new Exception("Failure reason: " + failureReason); + } + } + + /** Asserts that the specified download transitions to the specified state. */ + public void assertState(String id, @Download.State int state) { + assertStateInternal(id, state); + } + + /** Asserts that the specified download is removed. */ + public void assertRemoved(String id) { + assertStateInternal(id, STATE_REMOVED); + } + + // DownloadManager.Listener implementation. + @Override public void onInitialized(DownloadManager downloadManager) { initializedCondition.open(); } - public void blockUntilInitialized() throws InterruptedException { - if (!downloadManager.isInitialized()) { - assertThat(initializedCondition.block(TIMEOUT_MS)).isTrue(); - } - } - @Override public void onDownloadChanged(DownloadManager downloadManager, Download download) { if (download.state == Download.STATE_FAILED) { @@ -81,57 +113,17 @@ public final class TestDownloadManagerListener implements DownloadManager.Listen } @Override - public synchronized void onIdle(DownloadManager downloadManager) { + public void onIdle(DownloadManager downloadManager) { idleCondition.open(); } - /** - * Blocks until all remove and download tasks are complete and throws an exception if there was an - * error. - */ - public void blockUntilTasksCompleteAndThrowAnyDownloadError() throws Throwable { - blockUntilTasksComplete(); - if (failureReason != Download.FAILURE_REASON_NONE) { - throw new Exception("Failure reason: " + failureReason); - } - } + // Internal logic. - /** Blocks until all remove and download tasks are complete. Task errors are ignored. */ - public void blockUntilTasksComplete() throws InterruptedException { - idleCondition.close(); - dummyMainThread.runOnMainThread( - () -> { - if (downloadManager.isIdle()) { - idleCondition.open(); - } - }); - assertThat(idleCondition.block(TIMEOUT_MS)).isTrue(); - } - - private ArrayBlockingQueue getStateQueue(String taskId) { - synchronized (downloadStates) { - @Nullable ArrayBlockingQueue stateQueue = downloadStates.get(taskId); - if (stateQueue == null) { - stateQueue = new ArrayBlockingQueue<>(10); - downloadStates.put(taskId, stateQueue); - } - return stateQueue; - } - } - - public void assertRemoved(String taskId, int timeoutMs) { - assertStateInternal(taskId, STATE_REMOVED, timeoutMs); - } - - public void assertState(String taskId, @State int expectedState, int timeoutMs) { - assertStateInternal(taskId, expectedState, timeoutMs); - } - - private void assertStateInternal(String taskId, int expectedState, int timeoutMs) { + private void assertStateInternal(String id, int expectedState) { while (true) { @Nullable Integer state = null; try { - state = pollStateChange(taskId, timeoutMs); + state = getStateQueue(id).poll(TIMEOUT_MS, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { fail("Interrupted: " + e.getMessage()); } @@ -144,4 +136,15 @@ public final class TestDownloadManagerListener implements DownloadManager.Listen } } } + + private LinkedBlockingQueue getStateQueue(String id) { + synchronized (downloadStates) { + @Nullable LinkedBlockingQueue stateQueue = downloadStates.get(id); + if (stateQueue == null) { + stateQueue = new LinkedBlockingQueue<>(); + downloadStates.put(id, stateQueue); + } + return stateQueue; + } + } }