From 8ac10e0e0667a4fe35191deebb5fa9786bf4226c Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 3 Jan 2013 22:59:50 -0800 Subject: Clean up DownloadManager threading tests. Change runUntilStatus() methods to polling with timeout instead of requiring internal knowledge about threading. Fix notification tests, and move opening of InputStream until after handling headers to avoid FNFE. Always reset facade to defaults before each test. Change-Id: I6b2d6cfc4e685d2090c1133b1b2e89ae12760f8b --- .../AbstractDownloadProviderFunctionalTest.java | 6 +-- .../providers/downloads/AbstractPublicApiTest.java | 48 ++++++++++++++-------- .../downloads/DownloadProviderFunctionalTest.java | 31 ++++++++------ .../providers/downloads/FakeSystemFacade.java | 46 +++++++++------------ .../downloads/PublicApiFunctionalTest.java | 23 ++++------- .../android/providers/downloads/ThreadingTest.java | 9 +--- 6 files changed, 76 insertions(+), 87 deletions(-) (limited to 'tests/src') diff --git a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java index a65693fa..48eb0b47 100644 --- a/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/AbstractDownloadProviderFunctionalTest.java @@ -161,6 +161,7 @@ public abstract class AbstractDownloadProviderFunctionalTest extends setContext(mTestContext); setupService(); getService().mSystemFacade = mSystemFacade; + mSystemFacade.setUp(); assertTrue(isDatabaseEmpty()); // ensure we're not messing with real data mServer = new MockWebServer(); mServer.play(); @@ -246,11 +247,6 @@ public abstract class AbstractDownloadProviderFunctionalTest extends return mServer.getUrl(path).toString(); } - public void runService() throws Exception { - startService(null); - mSystemFacade.runAllThreads(); - } - protected String readStream(InputStream inputStream) throws IOException { BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); try { diff --git a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java index cda607aa..12c04f6a 100644 --- a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java +++ b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java @@ -16,17 +16,22 @@ package com.android.providers.downloads; +import static android.app.DownloadManager.STATUS_FAILED; +import static android.app.DownloadManager.STATUS_SUCCESSFUL; +import static android.text.format.DateUtils.SECOND_IN_MILLIS; + import android.app.DownloadManager; import android.database.Cursor; import android.net.Uri; import android.os.ParcelFileDescriptor; -import android.provider.Downloads; +import android.os.SystemClock; import android.util.Log; import java.io.FileInputStream; import java.io.InputStream; import java.net.MalformedURLException; import java.net.UnknownHostException; +import java.util.concurrent.TimeoutException; /** * Code common to tests that use the download manager public API. @@ -94,9 +99,28 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc } } - void runUntilStatus(int status) throws Exception { - runService(); - assertEquals(status, getStatus()); + void runUntilStatus(int status) throws TimeoutException { + startService(null); + waitForStatus(status); + } + + void waitForStatus(int expected) throws TimeoutException { + int actual = -1; + + final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS); + while (SystemClock.elapsedRealtime() < timeout) { + actual = getStatus(); + if (actual == STATUS_SUCCESSFUL || actual == STATUS_FAILED) { + assertEquals(expected, actual); + return; + } else if (actual == expected) { + return; + } + + SystemClock.sleep(100); + } + + throw new TimeoutException("Expected status " + expected + "; only reached " + actual); } // max time to wait before giving up on the current download operation. @@ -105,22 +129,10 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadProviderFunc // download thread private static final int TIME_TO_SLEEP = 1000; - int runUntilDone() throws InterruptedException { - int sleepCounter = MAX_TIME_TO_WAIT_FOR_OPERATION * 1000 / TIME_TO_SLEEP; - for (int i = 0; i < sleepCounter; i++) { - int status = getStatusIfExists(); - if (status == -1 || Downloads.Impl.isStatusCompleted(getStatus())) { - // row doesn't exist or the download is done - return status; - } - // download not done yet. sleep a while and try again - Thread.sleep(TIME_TO_SLEEP); - } - return 0; // failed - } - // waits until progress_so_far is >= (progress)% boolean runUntilProgress(int progress) throws InterruptedException { + startService(null); + int sleepCounter = MAX_TIME_TO_WAIT_FOR_OPERATION * 1000 / TIME_TO_SLEEP; int numBytesReceivedSoFar = 0; int totalBytes = 0; diff --git a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java index 23d300f8..d5249378 100644 --- a/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/DownloadProviderFunctionalTest.java @@ -16,14 +16,16 @@ package com.android.providers.downloads; +import static android.text.format.DateUtils.SECOND_IN_MILLIS; + import android.content.ContentValues; import android.database.Cursor; import android.net.ConnectivityManager; import android.net.Uri; import android.os.Environment; +import android.os.SystemClock; import android.provider.Downloads; import android.test.suitebuilder.annotation.LargeTest; -import android.util.Log; import com.google.mockwebserver.MockWebServer; import com.google.mockwebserver.RecordedRequest; @@ -31,6 +33,7 @@ import com.google.mockwebserver.RecordedRequest; import java.io.InputStream; import java.net.MalformedURLException; import java.net.UnknownHostException; +import java.util.concurrent.TimeoutException; /** * This test exercises the entire download manager working together -- it requests downloads through @@ -109,20 +112,22 @@ public class DownloadProviderFunctionalTest extends AbstractDownloadProviderFunc } } - private void runUntilStatus(Uri downloadUri, int status) throws Exception { - runService(); - boolean done = false; - while (!done) { - int rslt = getDownloadStatus(downloadUri); - if (rslt == Downloads.Impl.STATUS_RUNNING || rslt == Downloads.Impl.STATUS_PENDING) { - Log.i(TAG, "status is: " + rslt + ", for: " + downloadUri); - DownloadHandler.getInstance().waitUntilDownloadsTerminate(); - Thread.sleep(100); - } else { - done = true; + private void runUntilStatus(Uri downloadUri, int expected) throws Exception { + startService(null); + + int actual = -1; + + final long timeout = SystemClock.elapsedRealtime() + (15 * SECOND_IN_MILLIS); + while (SystemClock.elapsedRealtime() < timeout) { + actual = getDownloadStatus(downloadUri); + if (expected == actual) { + return; } + + SystemClock.sleep(100); } - assertEquals(status, getDownloadStatus(downloadUri)); + + throw new TimeoutException("Expected status " + expected + "; only reached " + actual); } protected int getDownloadStatus(Uri downloadUri) { diff --git a/tests/src/com/android/providers/downloads/FakeSystemFacade.java b/tests/src/com/android/providers/downloads/FakeSystemFacade.java index 481b5cba..d54c1224 100644 --- a/tests/src/com/android/providers/downloads/FakeSystemFacade.java +++ b/tests/src/com/android/providers/downloads/FakeSystemFacade.java @@ -7,10 +7,7 @@ import android.net.NetworkInfo; import android.net.NetworkInfo.DetailedState; import java.util.ArrayList; -import java.util.LinkedList; import java.util.List; -import java.util.Queue; - public class FakeSystemFacade implements SystemFacade { long mTimeMillis = 0; Integer mActiveNetworkType = ConnectivityManager.TYPE_WIFI; @@ -19,20 +16,32 @@ public class FakeSystemFacade implements SystemFacade { Long mMaxBytesOverMobile = null; Long mRecommendedMaxBytesOverMobile = null; List mBroadcastsSent = new ArrayList(); - Queue mStartedThreads = new LinkedList(); - private boolean returnActualTime = false; + private boolean mReturnActualTime = false; + + public void setUp() { + mTimeMillis = 0; + mActiveNetworkType = ConnectivityManager.TYPE_WIFI; + mIsRoaming = false; + mIsMetered = false; + mMaxBytesOverMobile = null; + mRecommendedMaxBytesOverMobile = null; + mBroadcastsSent.clear(); + mReturnActualTime = false; + } void incrementTimeMillis(long delta) { mTimeMillis += delta; } + @Override public long currentTimeMillis() { - if (returnActualTime) { + if (mReturnActualTime) { return System.currentTimeMillis(); } return mTimeMillis; } + @Override public NetworkInfo getActiveNetworkInfo(int uid) { if (mActiveNetworkType == null) { return null; @@ -48,14 +57,17 @@ public class FakeSystemFacade implements SystemFacade { return mIsMetered; } + @Override public boolean isNetworkRoaming() { return mIsRoaming; } + @Override public Long getMaxBytesOverMobile() { return mMaxBytesOverMobile ; } + @Override public Long getRecommendedMaxBytesOverMobile() { return mRecommendedMaxBytesOverMobile ; } @@ -70,27 +82,7 @@ public class FakeSystemFacade implements SystemFacade { return true; } - public boolean startThreadsWithoutWaiting = false; - public void setStartThreadsWithoutWaiting(boolean flag) { - this.startThreadsWithoutWaiting = flag; - } - - @Override - public void startThread(Thread thread) { - if (startThreadsWithoutWaiting) { - thread.start(); - } else { - mStartedThreads.add(thread); - } - } - - public void runAllThreads() { - while (!mStartedThreads.isEmpty()) { - mStartedThreads.poll().run(); - } - } - public void setReturnActualTime(boolean flag) { - returnActualTime = flag; + mReturnActualTime = flag; } } diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java index 1e6b7053..4159beb6 100644 --- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java +++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java @@ -47,7 +47,6 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.util.List; - @LargeTest public class PublicApiFunctionalTest extends AbstractPublicApiTest { private static final String REDIRECTED_PATH = "/other_path"; @@ -76,7 +75,6 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { } else { mTestDirectory.mkdir(); } - mSystemFacade.setStartThreadsWithoutWaiting(false); } @Override @@ -412,22 +410,18 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { } public void testCancel() throws Exception { - mSystemFacade.setStartThreadsWithoutWaiting(true); // return 'real time' from FakeSystemFacade so that DownloadThread will report progress mSystemFacade.setReturnActualTime(true); enqueueResponse(buildContinuingResponse()); Download download = enqueueRequest(getRequest()); - startService(null); // give the download time to get started and progress to 1% completion // before cancelling it. boolean rslt = download.runUntilProgress(1); assertTrue(rslt); mManager.remove(download.mId); - startService(null); - int status = download.runUntilDone(); + // make sure the row is gone from the database - assertEquals(-1, status); - mSystemFacade.setReturnActualTime(false); + download.waitForStatus(-1); } public void testDownloadCompleteBroadcast() throws Exception { @@ -512,9 +506,9 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { public void testContentObserver() throws Exception { enqueueResponse(buildEmptyResponse(HTTP_OK)); - enqueueRequest(getRequest()); mResolver.resetNotified(); - runService(); + final Download download = enqueueRequest(getRequest()); + download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); assertTrue(mResolver.mNotifyWasCalled); } @@ -524,10 +518,9 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { final Download download = enqueueRequest( getRequest().setNotificationVisibility(DownloadManager.Request.VISIBILITY_HIDDEN)); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, never()).notify(anyString(), anyInt(), isA(Notification.class)); - // TODO: verify that it never cancels } public void testNotificationVisible() throws Exception { @@ -536,11 +529,10 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { // only shows in-progress notifications final Download download = enqueueRequest(getRequest()); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); // TODO: verify different notif types with tags + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, atLeastOnce()).notify(anyString(), anyInt(), isA(Notification.class)); - verify(mNotifManager, times(1)).cancel(anyString(), anyInt()); } public void testNotificationVisibleComplete() throws Exception { @@ -549,11 +541,10 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest { final Download download = enqueueRequest(getRequest().setNotificationVisibility( DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED)); download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL); - runService(); // TODO: verify different notif types with tags + verify(mNotifManager, times(1)).cancelAll(); verify(mNotifManager, atLeastOnce()).notify(anyString(), anyInt(), isA(Notification.class)); - verify(mNotifManager, times(1)).cancel(anyString(), anyInt()); } public void testRetryAfter() throws Exception { diff --git a/tests/src/com/android/providers/downloads/ThreadingTest.java b/tests/src/com/android/providers/downloads/ThreadingTest.java index 8605c76b..e73bae2a 100644 --- a/tests/src/com/android/providers/downloads/ThreadingTest.java +++ b/tests/src/com/android/providers/downloads/ThreadingTest.java @@ -24,15 +24,8 @@ import android.test.suitebuilder.annotation.LargeTest; */ @LargeTest public class ThreadingTest extends AbstractPublicApiTest { - private static class FakeSystemFacadeWithThreading extends FakeSystemFacade { - @Override - public void startThread(Thread thread) { - thread.start(); - } - } - public ThreadingTest() { - super(new FakeSystemFacadeWithThreading()); + super(new FakeSystemFacade()); } @Override -- cgit v1.2.3