Serialize threading for download manager testing.
Steve Howard [Mon, 26 Jul 2010 22:25:06 +0000 (15:25 -0700)]
The download manager uses threading in a simple way -- it launches two
threads, UpdateThread and DownloadThread, and both are "fire and
forget".  This is fortunate for testing, since it means we can
eliminate multithreading and simply run each thread in order, and
everything still works.

This change does just that, abstracting Thread.start() behind
SystemFacade and making FakeSystemFacade put new threads into a queue
and then run through them serially.  This simplifies much of the test
code and makes it all much more predictable.

I've simplified the test code as much as possible here and moved a few
more tests over to PublicApiFunctionalTest, leaving only a minimum in
DownloadManagerFunctionalTest, which will eventually be deleted
altogether.  I've also improved testing in some areas -- for example,
we can now test that running notifications get cancelled after the
download completes in a robust way.

There is one test case that checks for race conditions and requires
multithreading.  I've moved this into a new ThreadingTest class, which
uses a custom FakeSystemFacade that allows multithreading.  I've
extracted AbstractPublicApiTest for the newly shared code.

Change-Id: Ic1d5c76bfa9913fe053174c3d8b516790ca8b25f

src/com/android/providers/downloads/DownloadInfo.java
src/com/android/providers/downloads/DownloadService.java
src/com/android/providers/downloads/RealSystemFacade.java
src/com/android/providers/downloads/SystemFacade.java
tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/AbstractPublicApiTest.java [new file with mode: 0644]
tests/src/com/android/providers/downloads/DownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/FakeSystemFacade.java
tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
tests/src/com/android/providers/downloads/ThreadingTest.java [new file with mode: 0644]

index 29c2d49..ee3ca54 100644 (file)
@@ -369,6 +369,6 @@ public class DownloadInfo {
         }
         DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this);
         mHasActiveThread = true;
-        downloader.start();
+        mSystemFacade.startThread(downloader);
     }
 }
index a617595..c9443fd 100644 (file)
@@ -258,7 +258,7 @@ public class DownloadService extends Service {
             mPendingUpdate = true;
             if (mUpdateThread == null) {
                 mUpdateThread = new UpdateThread();
-                mUpdateThread.start();
+                mSystemFacade.startThread(mUpdateThread);
             }
         }
     }
index 1d9e64a..adf0107 100644 (file)
@@ -87,4 +87,9 @@ class RealSystemFacade implements SystemFacade {
     public void cancelAllNotifications() {
         mNotificationManager.cancelAll();
     }
+
+    @Override
+    public void startThread(Thread thread) {
+        thread.start();
+    }
 }
index e41644a..3f8ff26 100644 (file)
@@ -53,4 +53,9 @@ interface SystemFacade {
      * Cancel all system notifications.
      */
     public void cancelAllNotifications();
+
+    /**
+     * Start a thread.
+     */
+    public void startThread(Thread thread);
 }
index a401a5b..9e7ef0f 100644 (file)
@@ -48,23 +48,18 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
 
     protected static final String LOG_TAG = "DownloadManagerFunctionalTest";
     private static final String PROVIDER_AUTHORITY = "downloads";
-    protected static final long REQUEST_TIMEOUT_MILLIS = 10 * 1000;
     protected static final long RETRY_DELAY_MILLIS = 61 * 1000;
     protected static final String FILE_CONTENT = "hello world";
     protected static final int HTTP_OK = 200;
     protected static final int HTTP_PARTIAL_CONTENT = 206;
     protected static final int HTTP_NOT_FOUND = 404;
     protected static final int HTTP_SERVICE_UNAVAILABLE = 503;
+
     protected MockWebServer mServer;
     protected MockContentResolverWithNotify mResolver;
     protected TestContext mTestContext;
     protected FakeSystemFacade mSystemFacade;
 
-    static interface StatusReader {
-        public int getStatus();
-        public boolean isComplete(int status);
-    }
-
     static class MockContentResolverWithNotify extends MockContentResolver {
         public boolean mNotifyWasCalled = false;
 
@@ -140,15 +135,15 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
         }
     }
 
-    public AbstractDownloadManagerFunctionalTest() {
+    public AbstractDownloadManagerFunctionalTest(FakeSystemFacade systemFacade) {
         super(DownloadService.class);
+        mSystemFacade = systemFacade;
     }
 
     @Override
     protected void setUp() throws Exception {
         super.setUp();
 
-        mSystemFacade = new FakeSystemFacade();
         Context realContext = getContext();
         mTestContext = new TestContext(realContext);
         setupProviderAndResolver();
@@ -165,28 +160,10 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
 
     @Override
     protected void tearDown() throws Exception {
-        waitForThreads();
         cleanUpDownloads();
         super.tearDown();
     }
 
-    private void waitForThreads() throws InterruptedException {
-        DownloadService service = getService();
-        if (service == null) {
-            return;
-        }
-
-        long startTimeMillis = System.currentTimeMillis();
-        while (service.mUpdateThread != null
-                && System.currentTimeMillis() < startTimeMillis + 1000) {
-            Thread.sleep(50);
-        }
-
-        // We can't explicitly wait for DownloadThreads, so just throw this last sleep in.  Ugly,
-        // but necessary to avoid unbearable flakiness until I can find a better solution.
-        Thread.sleep(50);
-    }
-
     private boolean isDatabaseEmpty() {
         Cursor cursor = mResolver.query(Downloads.CONTENT_URI, null, null, null, null);
         try {
@@ -230,16 +207,10 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
      * Enqueue a response from the MockWebServer.
      */
     MockResponse enqueueResponse(int status, String body) {
-        return enqueueResponse(status, body, true);
-    }
-
-    MockResponse enqueueResponse(int status, String body, boolean includeContentType) {
         MockResponse response = new MockResponse()
                                 .setResponseCode(status)
                                 .setBody(body);
-        if (includeContentType) {
-            response.addHeader("Content-type", "text/plain");
-        }
+        response.addHeader("Content-type", "text/plain");
         mServer.enqueue(response);
         return response;
     }
@@ -249,11 +220,11 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     }
 
     /**
-     * Wait for a request to come to the MockWebServer and return it.
+     * Fetch the last request received by the MockWebServer.
      */
-    RecordedRequest takeRequest() throws InterruptedException {
-        RecordedRequest request = mServer.takeRequestWithTimeout(REQUEST_TIMEOUT_MILLIS);
-        assertNotNull("Timed out waiting for request", request);
+    protected RecordedRequest takeRequest() throws InterruptedException {
+        RecordedRequest request = mServer.takeRequestWithTimeout(0);
+        assertNotNull("Expected request was not made", request);
         return request;
     }
 
@@ -261,58 +232,10 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
         return mServer.getUrl(path).toString();
     }
 
-    /**
-     * Run the service and wait for a request and for the download to reach the given status.
-     * @return the request received
-     */
-    protected RecordedRequest runUntilStatus(StatusReader reader, int status) throws Exception {
+    public void runService() throws Exception {
         startService(null);
-        RecordedRequest request = takeRequest();
-        waitForDownloadToStop(reader, status);
-        return request;
-    }
-
-    /**
-     * Wait for a download to given a given status, with a timeout.  Fails if the download reaches
-     * any other final status.
-     */
-    protected void waitForDownloadToStop(StatusReader reader, int expectedStatus)
-            throws Exception {
-        long startTimeMillis = System.currentTimeMillis();
-        long endTimeMillis = startTimeMillis + REQUEST_TIMEOUT_MILLIS;
-        int status = reader.getStatus();
-        while (status != expectedStatus) {
-            if (reader.isComplete(status)) {
-                fail("Download completed with unexpected status: " + status);
-            }
-            waitForChange(endTimeMillis);
-            if (startTimeMillis > endTimeMillis) {
-                fail("Download timed out with status " + status);
-            }
-            mServer.checkForExceptions();
-            status = reader.getStatus();
-        }
-
-        long delta = System.currentTimeMillis() - startTimeMillis;
-        Log.d(LOG_TAG, "Status " + status + " reached after " + delta + "ms");
-    }
-
-    /**
-     * Wait until mResolver gets notifyChange() called, or endTimeMillis is reached.
-     */
-    private void waitForChange(long endTimeMillis) {
-        synchronized(mResolver) {
-            long now = System.currentTimeMillis();
-            while (!mResolver.mNotifyWasCalled && now < endTimeMillis) {
-                try {
-                    mResolver.wait(endTimeMillis - now);
-                } catch (InterruptedException exc) {
-                    // no problem
-                }
-                now = System.currentTimeMillis();
-            }
-            mResolver.resetNotified();
-        }
+        mSystemFacade.runAllThreads();
+        mServer.checkForExceptions();
     }
 
     protected String readStream(InputStream inputStream) throws IOException {
diff --git a/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java b/tests/src/com/android/providers/downloads/AbstractPublicApiTest.java
new file mode 100644 (file)
index 0000000..db6b246
--- /dev/null
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.downloads;
+
+import android.database.Cursor;
+import android.net.DownloadManager;
+import android.net.Uri;
+import android.os.ParcelFileDescriptor;
+
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+
+/**
+ * Code common to tests that use the download manager public API.
+ */
+public abstract class AbstractPublicApiTest extends AbstractDownloadManagerFunctionalTest {
+
+    class Download {
+        final long mId;
+
+        private Download(long downloadId) {
+            this.mId = downloadId;
+        }
+
+        public int getStatus() {
+            return (int) getLongField(DownloadManager.COLUMN_STATUS);
+        }
+
+        String getStringField(String field) {
+            Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
+            try {
+                assertEquals(1, cursor.getCount());
+                cursor.moveToFirst();
+                return cursor.getString(cursor.getColumnIndexOrThrow(field));
+            } finally {
+                cursor.close();
+            }
+        }
+
+        long getLongField(String field) {
+            Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
+            try {
+                assertEquals(1, cursor.getCount());
+                cursor.moveToFirst();
+                return cursor.getLong(cursor.getColumnIndexOrThrow(field));
+            } finally {
+                cursor.close();
+            }
+        }
+
+        String getContents() throws Exception {
+            ParcelFileDescriptor downloadedFile = mManager.openDownloadedFile(mId);
+            assertTrue("Invalid file descriptor: " + downloadedFile,
+                       downloadedFile.getFileDescriptor().valid());
+            InputStream stream = new FileInputStream(downloadedFile.getFileDescriptor());
+            try {
+                return readStream(stream);
+            } finally {
+                stream.close();
+            }
+        }
+
+        void runUntilStatus(int status) throws Exception {
+            runService();
+            assertEquals(status, getStatus());
+        }
+    }
+
+    protected static final String PACKAGE_NAME = "my.package.name";
+    protected static final String REQUEST_PATH = "/path";
+
+    protected DownloadManager mManager;
+
+    public AbstractPublicApiTest(FakeSystemFacade systemFacade) {
+        super(systemFacade);
+    }
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mManager = new DownloadManager(mResolver, PACKAGE_NAME);
+    }
+
+    protected DownloadManager.Request getRequest() throws MalformedURLException {
+        return getRequest(getServerUri(REQUEST_PATH));
+    }
+
+    protected DownloadManager.Request getRequest(String path) {
+        return new DownloadManager.Request(Uri.parse(path));
+    }
+
+    protected Download enqueueRequest(DownloadManager.Request request) {
+        return new Download(mManager.enqueue(request));
+    }
+}
index 822ab54..350c63d 100644 (file)
@@ -28,7 +28,6 @@ import tests.http.RecordedRequest;
 
 import java.io.InputStream;
 import java.net.MalformedURLException;
-import java.util.List;
 
 /**
  * This test exercises the entire download manager working together -- it requests downloads through
@@ -38,6 +37,10 @@ import java.util.List;
  */
 @LargeTest
 public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFunctionalTest {
+    public DownloadManagerFunctionalTest() {
+        super(new FakeSystemFacade());
+    }
+
     public void testBasicRequest() throws Exception {
         enqueueResponse(HTTP_OK, FILE_CONTENT);
 
@@ -46,7 +49,8 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti
         assertEquals(Downloads.STATUS_PENDING, getDownloadStatus(downloadUri));
         assertTrue(mTestContext.mHasServiceBeenStarted);
 
-        RecordedRequest request = runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS);
+        runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS);
+        RecordedRequest request = takeRequest();
         assertEquals("GET", request.getMethod());
         assertEquals(path, request.getPath());
         assertEquals(FILE_CONTENT, getDownloadContents(downloadUri));
@@ -65,40 +69,6 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti
                          getDownloadFilename(downloadUri));
     }
 
-    public void testFileNotFound() throws Exception {
-        enqueueEmptyResponse(HTTP_NOT_FOUND);
-        Uri downloadUri = requestDownload("/nonexistent_path");
-        assertEquals(Downloads.STATUS_PENDING, getDownloadStatus(downloadUri));
-        runUntilStatus(downloadUri, HTTP_NOT_FOUND);
-    }
-
-    public void testRetryAfter() throws Exception {
-        final int delay = 120;
-        enqueueEmptyResponse(HTTP_SERVICE_UNAVAILABLE).addHeader("Retry-after", delay);
-        Uri downloadUri = requestDownload("/path");
-        runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED);
-
-        // download manager adds random 0-30s offset
-        mSystemFacade.incrementTimeMillis((delay + 31) * 1000);
-
-        enqueueResponse(HTTP_OK, FILE_CONTENT);
-        runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS);
-    }
-
-    public void testBasicConnectivityChanges() throws Exception {
-        enqueueResponse(HTTP_OK, FILE_CONTENT);
-        Uri downloadUri = requestDownload("/path");
-
-        // without connectivity, download immediately pauses
-        mSystemFacade.mActiveNetworkType = null;
-        startService(null);
-        waitForDownloadToStop(getStatusReader(downloadUri), Downloads.STATUS_RUNNING_PAUSED);
-
-        // connecting should start the download
-        mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_WIFI;
-        runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS);
-    }
-
     public void testRoaming() throws Exception {
         mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_MOBILE;
         mSystemFacade.mIsRoaming = true;
@@ -106,15 +76,13 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti
         // for a normal download, roaming is fine
         enqueueResponse(HTTP_OK, FILE_CONTENT);
         Uri downloadUri = requestDownload("/path");
-        startService(null);
         runUntilStatus(downloadUri, Downloads.STATUS_SUCCESS);
 
         // when roaming is disallowed, the download should pause...
         downloadUri = requestDownload("/path");
         updateDownload(downloadUri, Downloads.COLUMN_DESTINATION,
                        Integer.toString(Downloads.DESTINATION_CACHE_PARTITION_NOROAMING));
-        startService(null);
-        waitForDownloadToStop(getStatusReader(downloadUri), Downloads.STATUS_RUNNING_PAUSED);
+        runUntilStatus(downloadUri, Downloads.STATUS_RUNNING_PAUSED);
 
         // ...and pick up when we're off roaming
         enqueueResponse(HTTP_OK, FILE_CONTENT);
@@ -134,20 +102,9 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti
         }
     }
 
-    private RecordedRequest runUntilStatus(Uri downloadUri, int status) throws Exception {
-        return super.runUntilStatus(getStatusReader(downloadUri), status);
-    }
-
-    private StatusReader getStatusReader(final Uri downloadUri) {
-        return new StatusReader() {
-            public int getStatus() {
-                return getDownloadStatus(downloadUri);
-            }
-
-            public boolean isComplete(int status) {
-                return !Downloads.isStatusInformational(status);
-            }
-        };
+    private void runUntilStatus(Uri downloadUri, int status) throws Exception {
+        runService();
+        assertEquals(status, getDownloadStatus(downloadUri));
     }
 
     protected int getDownloadStatus(Uri downloadUri) {
index d35b558..297c1d3 100644 (file)
@@ -8,8 +8,10 @@ import android.test.AssertionFailedError;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Queue;
 
 public class FakeSystemFacade implements SystemFacade {
     long mTimeMillis = 0;
@@ -19,6 +21,7 @@ public class FakeSystemFacade implements SystemFacade {
     List<Intent> mBroadcastsSent = new ArrayList<Intent>();
     Map<Integer,Notification> mActiveNotifications = new HashMap<Integer,Notification>();
     List<Notification> mCanceledNotifications = new ArrayList<Notification>();
+    Queue<Thread> mStartedThreads = new LinkedList<Thread>();
 
     void incrementTimeMillis(long delta) {
         mTimeMillis += delta;
@@ -72,4 +75,15 @@ public class FakeSystemFacade implements SystemFacade {
             cancelNotification(id);
         }
     }
+
+    @Override
+    public void startThread(Thread thread) {
+        mStartedThreads.add(thread);
+    }
+
+    public void runAllThreads() {
+        while (!mStartedThreads.isEmpty()) {
+            mStartedThreads.poll().run();
+        }
+    }
 }
index 00419a3..ba3059b 100644 (file)
@@ -22,7 +22,6 @@ import android.net.ConnectivityManager;
 import android.net.DownloadManager;
 import android.net.Uri;
 import android.os.Environment;
-import android.os.ParcelFileDescriptor;
 import android.provider.Downloads;
 import android.test.suitebuilder.annotation.LargeTest;
 import tests.http.MockResponse;
@@ -35,77 +34,21 @@ import java.net.MalformedURLException;
 import java.util.List;
 
 @LargeTest
-public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTest {
-    private static final String PACKAGE_NAME = "my.package.name";
+public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     private static final int HTTP_NOT_ACCEPTABLE = 406;
     private static final int HTTP_LENGTH_REQUIRED = 411;
-    private static final String REQUEST_PATH = "/path";
     private static final String REDIRECTED_PATH = "/other_path";
     private static final String ETAG = "my_etag";
 
-    class Download implements StatusReader {
-        final long mId;
+    protected File mTestDirectory;
 
-        private Download(long downloadId) {
-            this.mId = downloadId;
-        }
-
-        public int getStatus() {
-            return (int) getLongField(DownloadManager.COLUMN_STATUS);
-        }
-
-        public boolean isComplete(int status) {
-            return status != DownloadManager.STATUS_PENDING
-                    && status != DownloadManager.STATUS_RUNNING
-                    && status != DownloadManager.STATUS_PAUSED;
-        }
-
-        String getStringField(String field) {
-            Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
-            try {
-                assertEquals(1, cursor.getCount());
-                cursor.moveToFirst();
-                return cursor.getString(cursor.getColumnIndexOrThrow(field));
-            } finally {
-                cursor.close();
-            }
-        }
-
-        long getLongField(String field) {
-            Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
-            try {
-                assertEquals(1, cursor.getCount());
-                cursor.moveToFirst();
-                return cursor.getLong(cursor.getColumnIndexOrThrow(field));
-            } finally {
-                cursor.close();
-            }
-        }
-
-        String getContents() throws Exception {
-            ParcelFileDescriptor downloadedFile = mManager.openDownloadedFile(mId);
-            assertTrue("Invalid file descriptor: " + downloadedFile,
-                       downloadedFile.getFileDescriptor().valid());
-            InputStream stream = new FileInputStream(downloadedFile.getFileDescriptor());
-            try {
-                return readStream(stream);
-            } finally {
-                stream.close();
-            }
-        }
-
-        RecordedRequest runUntilStatus(int status) throws Exception {
-            return PublicApiFunctionalTest.this.runUntilStatus(this, status);
-        }
+    public PublicApiFunctionalTest() {
+        super(new FakeSystemFacade());
     }
 
-    private DownloadManager mManager;
-    private File mTestDirectory;
-
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        mManager = new DownloadManager(mResolver, PACKAGE_NAME);
 
         mTestDirectory = new File(Environment.getExternalStorageDirectory() + File.separator
                                   + "download_manager_functional_test");
@@ -142,7 +85,8 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
                      download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP));
 
         mSystemFacade.incrementTimeMillis(10);
-        RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        RecordedRequest request = takeRequest();
         assertEquals("GET", request.getMethod());
         assertEquals(REQUEST_PATH, request.getPath());
 
@@ -190,14 +134,15 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
                      download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
         assertEquals(FILE_CONTENT.length(),
                      download.getLongField(DownloadManager.COLUMN_TOTAL_SIZE_BYTES));
+        takeRequest(); // get the first request out of the queue
 
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
-        RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
         assertEquals(FILE_CONTENT.length(),
                      download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
         assertEquals(FILE_CONTENT, download.getContents());
 
-        List<String> headers = request.getHeaders();
+        List<String> headers = takeRequest().getHeaders();
         assertTrue("No Range header: " + headers,
                    headers.contains("Range: bytes=" + initialLength + "-"));
         assertTrue("No ETag header: " + headers, headers.contains("If-Match: " + ETAG));
@@ -299,10 +244,11 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         enqueueEmptyResponse(HTTP_OK);
         Download download = enqueueRequest(getRequest().setRequestHeader("Header1", "value1")
                                            .setRequestHeader("Header2", "value2"));
-        RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
 
-        assertTrue(request.getHeaders().contains("Header1: value1"));
-        assertTrue(request.getHeaders().contains("Header2: value2"));
+        List<String> headers = takeRequest().getHeaders();
+        assertTrue(headers.contains("Header1: value1"));
+        assertTrue(headers.contains("Header2: value2"));
     }
 
     public void testDelete() throws Exception {
@@ -331,19 +277,6 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
     }
 
-    /**
-     * Test for race conditions when the service is flooded with startService() calls while running
-     * a download.
-     */
-    public void testFloodServiceWithStarts() throws Exception {
-        enqueueResponse(HTTP_OK, FILE_CONTENT);
-        Download download = enqueueRequest(getRequest());
-        while (download.getStatus() != DownloadManager.STATUS_SUCCESSFUL) {
-            startService(null);
-            Thread.sleep(10);
-        }
-    }
-
     public void testRedirect301() throws Exception {
         RecordedRequest lastRequest = runRedirectionTest(301);
         // for 301, upon retry, we reuse the redirected URI
@@ -375,7 +308,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
     }
 
     public void testNoContentType() throws Exception {
-        enqueueResponse(HTTP_OK, "", false);
+        enqueueResponse(HTTP_OK, "").removeHeader("Content-Type");
         runSimpleFailureTest(HTTP_NOT_ACCEPTABLE);
     }
 
@@ -394,8 +327,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
 
         mManager.remove(download.mId);
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
-        startService(null);
-        Thread.sleep(500); // TODO: eliminate this when we can run the service synchronously
+        runService();
         // if the cancel didn't work, we should get an unexpected request to the HTTP server
     }
 
@@ -404,13 +336,6 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         Download download = enqueueRequest(getRequest());
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
 
-        long startTimeMillis = System.currentTimeMillis();
-        while (mSystemFacade.mBroadcastsSent.isEmpty()) {
-            Thread.sleep(100);
-            if (System.currentTimeMillis() > startTimeMillis + 500) {
-                fail("Timed out waiting for broadcast intent");
-            }
-        }
         assertEquals(1, mSystemFacade.mBroadcastsSent.size());
         Intent broadcast = mSystemFacade.mBroadcastsSent.get(0);
         assertEquals(DownloadManager.ACTION_DOWNLOAD_COMPLETE, broadcast.getAction());
@@ -435,6 +360,19 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         assertEquals(PACKAGE_NAME, broadcast.getPackage());
     }
 
+    public void testBasicConnectivityChanges() throws Exception {
+        enqueueResponse(HTTP_OK, FILE_CONTENT);
+        Download download = enqueueRequest(getRequest());
+
+        // without connectivity, download immediately pauses
+        mSystemFacade.mActiveNetworkType = null;
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+
+        // connecting should start the download
+        mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_WIFI;
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+    }
+
     public void testAllowedNetworkTypes() throws Exception {
         mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_MOBILE;
 
@@ -446,8 +384,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         // restrict a download to wifi...
         download = enqueueRequest(getRequest()
                                   .setAllowedNetworkTypes(DownloadManager.Request.NETWORK_WIFI));
-        startService(null);
-        waitForDownloadToStop(download, DownloadManager.STATUS_PAUSED);
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
         // ...then enable wifi
         mSystemFacade.mActiveNetworkType = ConnectivityManager.TYPE_WIFI;
         enqueueEmptyResponse(HTTP_OK);
@@ -464,8 +401,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
 
         // disallow roaming for a download...
         download = enqueueRequest(getRequest().setAllowedOverRoaming(false));
-        startService(null);
-        waitForDownloadToStop(download, DownloadManager.STATUS_PAUSED);
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
         // ...then turn off roaming
         mSystemFacade.mIsRoaming = false;
         enqueueEmptyResponse(HTTP_OK);
@@ -476,12 +412,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         enqueueEmptyResponse(HTTP_OK);
         enqueueRequest(getRequest());
         mResolver.resetNotified();
-        startService(null);
-        synchronized(mResolver) {
-            if (!mResolver.mNotifyWasCalled) {
-                mResolver.wait(2000);
-            }
-        }
+        runService();
         assertTrue(mResolver.mNotifyWasCalled);
     }
 
@@ -491,16 +422,32 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
         assertEquals(0, mSystemFacade.mActiveNotifications.size());
         assertEquals(0, mSystemFacade.mCanceledNotifications.size());
-        
+
         enqueueEmptyResponse(HTTP_OK);
         download = enqueueRequest(
                 getRequest()
                 .setShowNotification(DownloadManager.Request.NOTIFICATION_WHEN_RUNNING));
         download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
         assertEquals(1, mSystemFacade.mActiveNotifications.size());
+
         // The notification doesn't actually get canceled until the UpdateThread runs again, which
-        // gets triggered by the DownloadThread updating the status in the provider.  This is
-        // tough to test right now, so I'll leave it until the overall structure is changed.
+        // gets triggered by the DownloadThread updating the status in the provider.
+        runService();
+        assertEquals(0, mSystemFacade.mActiveNotifications.size());
+        assertEquals(1, mSystemFacade.mCanceledNotifications.size());
+    }
+
+    public void testRetryAfter() throws Exception {
+        final int delay = 120;
+        enqueueEmptyResponse(HTTP_SERVICE_UNAVAILABLE).addHeader("Retry-after", delay);
+        Download download = enqueueRequest(getRequest());
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+
+        // download manager adds random 0-30s offset
+        mSystemFacade.incrementTimeMillis((delay + 31) * 1000);
+
+        enqueueEmptyResponse(HTTP_OK);
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
     }
 
     private void runSimpleFailureTest(int expectedErrorCode) throws Exception {
@@ -524,27 +471,15 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         enqueueInterruptedDownloadResponses(5);
 
         Download download = enqueueRequest(getRequest());
-        RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_PAUSED);
-        assertEquals(REQUEST_PATH, request.getPath());
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+        assertEquals(REQUEST_PATH, takeRequest().getPath());
 
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
-        request = download.runUntilStatus(DownloadManager.STATUS_PAUSED);
-        assertEquals(REDIRECTED_PATH, request.getPath());
+        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
+        assertEquals(REDIRECTED_PATH, takeRequest().getPath());
 
         mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
-        request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
-        return request;
-    }
-
-    private DownloadManager.Request getRequest() throws MalformedURLException {
-        return getRequest(getServerUri(REQUEST_PATH));
-    }
-
-    private DownloadManager.Request getRequest(String path) {
-        return new DownloadManager.Request(Uri.parse(path));
-    }
-
-    private Download enqueueRequest(DownloadManager.Request request) {
-        return new Download(mManager.enqueue(request));
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+        return takeRequest();
     }
 }
diff --git a/tests/src/com/android/providers/downloads/ThreadingTest.java b/tests/src/com/android/providers/downloads/ThreadingTest.java
new file mode 100644 (file)
index 0000000..e8d86d6
--- /dev/null
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.downloads;
+
+import android.net.DownloadManager;
+import android.test.suitebuilder.annotation.LargeTest;
+
+/**
+ * Download manager tests that require multithreading.
+ */
+@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());
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        Thread.sleep(50); // give threads a chance to finish
+        super.tearDown();
+    }
+
+    /**
+     * Test for race conditions when the service is flooded with startService() calls while running
+     * a download.
+     */
+    public void testFloodServiceWithStarts() throws Exception {
+        enqueueResponse(HTTP_OK, FILE_CONTENT);
+        Download download = enqueueRequest(getRequest());
+        while (download.getStatus() != DownloadManager.STATUS_SUCCESSFUL) {
+            startService(null);
+            Thread.sleep(10);
+        }
+    }
+}