[Android] Remove test file after download test

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
index 0c390756d5f7d27952302d2f4150e525e1c1976a..3fd68ee169927336020ffb6c38b666c8c308fb0c 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
@@ -9,6 +9,7 @@ import android.test.FlakyTest;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.view.View;
 
+import org.chromium.base.Log;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.R;
@@ -31,6 +32,7 @@ import java.io.File;
  * Tests Chrome download feature by attempting to download some files.
  */
 public class DownloadTest extends DownloadTestBase {
+    private static final String TAG = "cr_DownloadTest";
     private static final String SUPERBO_CONTENTS =
             "plain text response from a POST";
 
@@ -311,26 +313,32 @@ public class DownloadTest extends DownloadTestBase {
         File dir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS);
         assertTrue(dir.isDirectory());
         final File file = new File(dir, "test.gzip");
-        if (!file.exists()) {
-            assertTrue(file.createNewFile());
-        }
-
-        // Open in a new tab again.
-        loadUrl(url);
-        waitForFocus();
-
-        View currentView = getActivity().getActivityTab().getView();
-        TouchCommon.longPressView(currentView);
-        getInstrumentation().invokeContextMenuAction(
-                getActivity(), R.id.contextmenu_open_in_new_tab, 0);
-        waitForNewTabToStabilize(2);
-
-        goToLastTab();
-        assertPollForInfoBarSize(1);
+        try {
+            if (!file.exists()) {

[gone, 2016/01/13 22:27:12]

This file really shouldn't ever exist between test runs.  It should be deleted.

[boliu, 2016/01/13 22:34:01]

Well, I assume there are corner cases. Eg native crash in the middle of the test
or something. I can remove the check, but probably also the assertTrue for
createNewFile as well.

[gone, 2016/01/13 22:36:32]

Yeah, I meant that the file should always be explicitly deleted even if it
already exists.  There shouldn't be any state that crosses over between tests
like this, but I don't know what good that'll do in general.

+                assertTrue(file.createNewFile());
+            }
 
-        // Now create two new files by clicking on the infobars.
-        assertTrue("OVERWRITE button wasn't found",
-                InfoBarUtil.clickPrimaryButton(getInfoBars().get(0)));
+            // Open in a new tab again.
+            loadUrl(url);
+            waitForFocus();
+
+            View currentView = getActivity().getActivityTab().getView();
+            TouchCommon.longPressView(currentView);
+            getInstrumentation().invokeContextMenuAction(
+                    getActivity(), R.id.contextmenu_open_in_new_tab, 0);
+            waitForNewTabToStabilize(2);
+
+            goToLastTab();
+            assertPollForInfoBarSize(1);
+
+            // Now create two new files by clicking on the infobars.
+            assertTrue("OVERWRITE button wasn't found",
+                    InfoBarUtil.clickPrimaryButton(getInfoBars().get(0)));
+        } finally {
+            if (!file.delete()) {
+                Log.d(TAG, "Failed to delete test.gzip");

[gone, 2016/01/13 22:27:12]

Is this something that should be handled in the setUp() and tearDown() phases? 
Maybe the test should create a temporary test-specific directory so that other
tests can't accidentally make a test.gzip file on its own (e.g. what
TestTabModelDirectory does)?

[boliu, 2016/01/13 22:34:01]

Not every single test wants this. I can put the remove in tearDown, but then
it's awkward to not match up with a setUp.
It has to be the shared download directory, for testing downloading duplicated
files. :/

[gone, 2016/01/13 22:36:32]

Eh, not so awkward, but if you're fine with that that's alright.

+            }
+        }
     }
 
     @MediumTest