fix broken DownloadManager tests
Vasu Nori [Mon, 6 Dec 2010 23:16:23 +0000 (15:16 -0800)]
one big change in this CL is addition of a new feature to MockWebServer.
It can now play a long response to the Downloading thread to keep it busy
while something - such as cancel/remove - can be done to that Download Request.

Also, added changes to FakeSystemFacade to start threads in normal fashion
instead of queuing them up and later running just their run() methods.

the following tests should work now
packages/providers/DownloadProvider/tests/src/com/android/providers/downloads/
  DownloadManagerFunctionalTest.java
  PublicApiFunctionalTest.java
  ThreadingTest.java
  PublicApiAccessTest.java
  DownloadProviderPermissionsTest.java

the following are flaky. they need to be split up into smaller tests.
frameworks/base/core/tests/coretests/src/android/app/
  DownloadManagerIntegrationTest.java
  DownloadManagerStressTest.java

Change-Id: Ia0b11963f92e8f8365f701761dcbce467be3ee9b

tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/AbstractPublicApiTest.java
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/tests/http/MockResponse.java
tests/src/tests/http/MockWebServer.java

index a06597f..5283d42 100644 (file)
@@ -59,6 +59,14 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     protected MockContentResolverWithNotify mResolver;
     protected TestContext mTestContext;
     protected FakeSystemFacade mSystemFacade;
+    protected static String STRING_1K;
+    static {
+        StringBuilder buff = new StringBuilder();
+        for (int i = 0; i < 1024; i++) {
+            buff.append("a" + i % 26);
+        }
+        STRING_1K = buff.toString();
+    }
 
     static class MockContentResolverWithNotify extends MockContentResolver {
         public boolean mNotifyWasCalled = false;
@@ -161,6 +169,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     @Override
     protected void tearDown() throws Exception {
         cleanUpDownloads();
+        mServer.shutdown();
         super.tearDown();
     }
 
@@ -205,7 +214,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     }
 
     /**
-     * Enqueue a response from the MockWebServer.
+     * Enqueue a String response from the MockWebServer.
      */
     MockResponse enqueueResponse(int status, String body) {
         MockResponse response = new MockResponse()
@@ -216,6 +225,18 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
         mServer.enqueue(response);
         return response;
     }
+    /**
+     * Enqueue a byte[] response from the MockWebServer.
+     */
+    MockResponse enqueueResponse(int status, byte[] body) {
+        MockResponse response = new MockResponse()
+                                .setResponseCode(status)
+                                .setBody(body)
+                                .addHeader("Content-type", "text/plain")
+                                .setCloseConnectionAfter(true);
+        mServer.enqueue(response);
+        return response;
+    }
 
     MockResponse enqueueEmptyResponse(int status) {
         return enqueueResponse(status, "");
index ed443b0..c38c2f1 100644 (file)
@@ -20,6 +20,8 @@ import android.app.DownloadManager;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.ParcelFileDescriptor;
+import android.provider.Downloads;
+import android.util.Log;
 
 import java.io.FileInputStream;
 import java.io.InputStream;
@@ -41,6 +43,22 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadManagerFunct
             return (int) getLongField(DownloadManager.COLUMN_STATUS);
         }
 
+        public int getStatusIfExists() {
+            Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
+            try {
+                if (cursor.getCount() > 0) {
+                    cursor.moveToFirst();
+                    return (int) cursor.getLong(cursor.getColumnIndexOrThrow(
+                            DownloadManager.COLUMN_STATUS));
+                } else {
+                    // the row doesn't exist
+                    return -1;
+                }
+            } finally {
+                cursor.close();
+            }
+        }
+
         String getStringField(String field) {
             Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
             try {
@@ -79,6 +97,63 @@ public abstract class AbstractPublicApiTest extends AbstractDownloadManagerFunct
             runService();
             assertEquals(status, getStatus());
         }
+
+        // max time to wait before giving up on the current download operation.
+        private static final int MAX_TIME_TO_WAIT_FOR_OPERATION = 5;
+        // while waiting for the above time period, sleep this long to yield to the
+        // 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 {
+            int sleepCounter = MAX_TIME_TO_WAIT_FOR_OPERATION * 1000 / TIME_TO_SLEEP;
+            int numBytesReceivedSoFar = 0;
+            int totalBytes = 0;
+            for (int i = 0; i < sleepCounter; i++) {
+                Cursor cursor = mManager.query(new DownloadManager.Query().setFilterById(mId));
+                try {
+                    assertEquals(1, cursor.getCount());
+                    cursor.moveToFirst();
+                    numBytesReceivedSoFar = cursor.getInt(
+                            cursor.getColumnIndexOrThrow(
+                                    DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
+                    totalBytes = cursor.getInt(
+                            cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TOTAL_SIZE_BYTES));
+                } finally {
+                    cursor.close();
+                }
+                Log.i(LOG_TAG, "in runUntilProgress, numBytesReceivedSoFar: " +
+                        numBytesReceivedSoFar + ", totalBytes: " + totalBytes);
+                if (totalBytes == 0) {
+                    fail("total_bytes should not be zero");
+                    return false;
+                } else {
+                    if (numBytesReceivedSoFar * 100 / totalBytes >= progress) {
+                        // progress_so_far is >= progress%. we are done
+                        return true;
+                    }
+                }
+                // download not done yet. sleep a while and try again
+                Thread.sleep(TIME_TO_SLEEP);
+            }
+            Log.i(LOG_TAG, "FAILED in runUntilProgress, numBytesReceivedSoFar: " +
+                    numBytesReceivedSoFar + ", totalBytes: " + totalBytes);
+            return false; // failed
+        }
     }
 
     protected static final String PACKAGE_NAME = "my.package.name";
index bc3de1d..7a2bfdf 100644 (file)
@@ -41,7 +41,7 @@ public class DownloadManagerFunctionalTest extends AbstractDownloadManagerFuncti
         super(new FakeSystemFacade());
     }
 
-    public void testBasicRequest() throws Exception {
+    public void testDownloadTextFile() throws Exception {
         enqueueResponse(HTTP_OK, FILE_CONTENT);
 
         String path = "/download_manager_test_path";
index 5263015..9620ffc 100644 (file)
@@ -23,12 +23,16 @@ public class FakeSystemFacade implements SystemFacade {
     Map<Long,Notification> mActiveNotifications = new HashMap<Long,Notification>();
     List<Notification> mCanceledNotifications = new ArrayList<Notification>();
     Queue<Thread> mStartedThreads = new LinkedList<Thread>();
+    private boolean returnActualTime = false;
 
     void incrementTimeMillis(long delta) {
         mTimeMillis += delta;
     }
 
     public long currentTimeMillis() {
+        if (returnActualTime) {
+            return System.currentTimeMillis();
+        }
         return mTimeMillis;
     }
 
@@ -81,9 +85,18 @@ public class FakeSystemFacade implements SystemFacade {
         }
     }
 
+    public boolean startThreadsWithoutWaiting = false;
+    public void setStartThreadsWithoutWaiting(boolean flag) {
+        this.startThreadsWithoutWaiting = flag;
+    }
+
     @Override
     public void startThread(Thread thread) {
-        mStartedThreads.add(thread);
+        if (startThreadsWithoutWaiting) {
+            thread.start();
+        } else {
+            mStartedThreads.add(thread);
+        }
     }
 
     public void runAllThreads() {
@@ -91,4 +104,8 @@ public class FakeSystemFacade implements SystemFacade {
             mStartedThreads.poll().run();
         }
     }
+
+    public void setReturnActualTime(boolean flag) {
+        returnActualTime = flag;
+    }
 }
index cad01df..5d14930 100644 (file)
@@ -16,6 +16,7 @@
 
 package com.android.providers.downloads;
 
+
 import android.app.DownloadManager;
 import android.content.Intent;
 import android.database.Cursor;
@@ -53,17 +54,18 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         mTestDirectory = new File(Environment.getExternalStorageDirectory() + File.separator
                                   + "download_manager_functional_test");
         if (mTestDirectory.exists()) {
-            mTestDirectory.delete();
-        }
-        if (!mTestDirectory.mkdir()) {
-            throw new RuntimeException("Couldn't create test directory: "
-                                       + mTestDirectory.getPath());
+            for (File file : mTestDirectory.listFiles()) {
+                file.delete();
+            }
+        } else {
+            mTestDirectory.mkdir();
         }
+        mSystemFacade.setStartThreadsWithoutWaiting(false);
     }
 
     @Override
     protected void tearDown() throws Exception {
-        if (mTestDirectory != null) {
+        if (mTestDirectory != null && mTestDirectory.exists()) {
             for (File file : mTestDirectory.listFiles()) {
                 file.delete();
             }
@@ -184,6 +186,18 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
         return response;
     }
 
+    // enqueue a huge response to keep the receiveing thread in DownloadThread.java busy for a while
+    // give enough time to do something (cancel/remove etc) on that downloadrequest
+    // while it is in progress
+    private void enqueueContinuingResponse() {
+        int numPackets = 100;
+        int contentLength =  STRING_1K.length() * numPackets;
+        enqueueResponse(HTTP_OK, STRING_1K)
+               .addHeader("Content-length", contentLength)
+               .addHeader("Etag", ETAG)
+               .setNumPackets(numPackets);
+    }
+
     public void testFiltering() throws Exception {
         enqueueEmptyResponse(HTTP_OK);
         Download download1 = enqueueRequest(getRequest());
@@ -301,7 +315,7 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     }
 
     private Uri getExternalUri() {
-        return Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile").build();
+        return Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile.txt").build();
     }
 
     public void testRequestHeaders() throws Exception {
@@ -379,14 +393,22 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     }
 
     public void testCancel() throws Exception {
-        enqueuePartialResponse(0, 5);
+        mSystemFacade.setStartThreadsWithoutWaiting(true);
+        // return 'real time' from FakeSystemFacade so that DownloadThread will report progress
+        mSystemFacade.setReturnActualTime(true);
+        enqueueContinuingResponse();
         Download download = enqueueRequest(getRequest());
-        download.runUntilStatus(DownloadManager.STATUS_PAUSED);
-
+        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);
-        mSystemFacade.incrementTimeMillis(RETRY_DELAY_MILLIS);
-        runService();
-        // if the cancel didn't work, we should get an unexpected request to the HTTP server
+        startService(null);
+        int status = download.runUntilDone();
+        // make sure the row is gone from the database
+        assertEquals(-1, status);
+        mSystemFacade.setReturnActualTime(false);
     }
 
     public void testDownloadCompleteBroadcast() throws Exception {
@@ -524,14 +546,15 @@ public class PublicApiFunctionalTest extends AbstractPublicApiTest {
     }
 
     public void testExistingFile() throws Exception {
+        // download a file which already exists.
+        // downloadservice should simply create filename with "-" and a number attached
+        // at the end; i.e., download shouldnot fail.
         Uri destination = getExternalUri();
         new File(destination.getPath()).createNewFile();
 
         enqueueEmptyResponse(HTTP_OK);
         Download download = enqueueRequest(getRequest().setDestinationUri(destination));
-        download.runUntilStatus(DownloadManager.STATUS_FAILED);
-        assertEquals(DownloadManager.ERROR_FILE_ALREADY_EXISTS,
-                     download.getLongField(DownloadManager.COLUMN_REASON));
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
     }
 
     public void testEmptyFields() throws Exception {
index 4cda92d..aec5490 100644 (file)
@@ -36,6 +36,7 @@ public class MockResponse {
     private Map<String, String> headers = new HashMap<String, String>();
     private byte[] body = EMPTY_BODY;
     private boolean closeConnectionAfter = false;
+    private int numPackets = 0;
 
     public MockResponse() {
         addHeader("Content-Length", 0);
@@ -133,4 +134,14 @@ public class MockResponse {
         this.closeConnectionAfter = closeConnectionAfter;
         return this;
     }
+
+    public int getNumPackets() {
+        return numPackets;
+    }
+
+    public MockResponse setNumPackets(int numPackets) {
+        this.numPackets = numPackets;
+        return this;
+    }
+
 }
index 11c8063..6096783 100644 (file)
@@ -16,6 +16,9 @@
 
 package tests.http;
 
+import android.text.TextUtils;
+import android.util.Log;
+
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
@@ -59,6 +62,7 @@ public final class MockWebServer {
     private final Queue<Future<?>> futures = new LinkedList<Future<?>>();
 
     private int port = -1;
+    private ServerSocket serverSocket;
 
     public int getPort() {
         if (port == -1) {
@@ -111,26 +115,35 @@ public final class MockWebServer {
      * down.
      */
     public void play() throws IOException {
-        final ServerSocket ss = new ServerSocket(0);
-        ss.setReuseAddress(true);
-        port = ss.getLocalPort();
+        serverSocket = new ServerSocket(0);
+        serverSocket.setReuseAddress(true);
+        port = serverSocket.getLocalPort();
         submitCallable(new Callable<Void>() {
             public Void call() throws Exception {
                 int count = 0;
                 while (true) {
                     if (count > 0 && responseQueue.isEmpty()) {
-                        ss.close();
+                        serverSocket.close();
                         executor.shutdown();
                         return null;
                     }
 
-                    serveConnection(ss.accept());
+                    serveConnection(serverSocket.accept());
                     count++;
                 }
             }
         });
     }
 
+    /**
+     * shutdown the webserver
+     */
+    public void shutdown() throws IOException {
+        responseQueue.clear();
+        serverSocket.close();
+        executor.shutdown();
+    }
+
     private void serveConnection(final Socket s) {
         submitCallable(new Callable<Void>() {
             public Void call() throws Exception {
@@ -148,8 +161,7 @@ public final class MockWebServer {
                         }
                     }
                     requestQueue.add(request);
-                    MockResponse response = computeResponse(request);
-                    writeResponse(out, response);
+                    MockResponse response = sendResponse(out, request);
                     if (response.shouldCloseConnectionAfter()) {
                         break;
                     }
@@ -241,7 +253,6 @@ public final class MockWebServer {
         } else {
             throw new UnsupportedOperationException("Unexpected method: " + request);
         }
-
         return new RecordedRequest(request, headers, chunkSizes,
                 requestBody.numBytesReceived, requestBody.toByteArray(), sequenceNumber);
     }
@@ -249,14 +260,32 @@ public final class MockWebServer {
     /**
      * Returns a response to satisfy {@code request}.
      */
-    private MockResponse computeResponse(RecordedRequest request) throws InterruptedException {
+    private MockResponse sendResponse(OutputStream out, RecordedRequest request)
+            throws InterruptedException, IOException {
         if (responseQueue.isEmpty()) {
             throw new IllegalStateException("Unexpected request: " + request);
         }
-        return responseQueue.take();
-    }
+        MockResponse response = responseQueue.take();
+        writeResponse(out, response, false);
+        if (response.getNumPackets() > 0) {
+            // there are continuing packets to send as part of this response.
+            for (int i = 0; i < response.getNumPackets(); i++) {
+                writeResponse(out, response, true);
+                // delay sending next continuing response just a little bit
+                Thread.sleep(100);
+            }
+        }
+        return response;
+     }
 
-    private void writeResponse(OutputStream out, MockResponse response) throws IOException {
+    private void writeResponse(OutputStream out, MockResponse response,
+            boolean continuingPacket) throws IOException {
+        if (continuingPacket) {
+            // this is a continuing response - just send the body - no headers, status
+            out.write(response.getBody());
+            out.flush();
+            return;
+        }
         out.write((response.getStatus() + "\r\n").getBytes(ASCII));
         for (String header : response.getHeaders()) {
             out.write((header + "\r\n").getBytes(ASCII));