Fix incremental install tests not being able to list tests

Bug: 857001
Change-Id: Id4eebe86e34e30edfda8fa5825ceb1e3e303dd95
Reviewed-on: https://chromium-review.googlesource.com/1148929Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580071}

base/test/android/javatests/src/org/chromium/base/test/BaseChromiumAndroidJUnitRunner.java

@@ -181,11 +181,28 @@ public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
     }
 
     private TestRequest createListTestRequest(Bundle arguments) {
+        DexFile[] incrementalJars = null;
+        try {
+            Class<?> bootstrapClass =
+                    Class.forName("org.chromium.incrementalinstall.BootstrapApplication");
+            incrementalJars =
+                    (DexFile[]) bootstrapClass.getDeclaredField("sIncrementalDexFiles").get(null);
+        } catch (Exception e) {
+            // Not an incremental apk.
+        }
         RunnerArgs runnerArgs =
                 new RunnerArgs.Builder().fromManifest(this).fromBundle(arguments).build();
-        TestRequestBuilder builder = new IncrementalInstallTestRequestBuilder(this, arguments);
+        TestRequestBuilder builder;
+        if (incrementalJars != null) {
+            builder = new IncrementalInstallTestRequestBuilder(this, arguments, incrementalJars);
+        } else {
+            builder = new TestRequestBuilder(this, arguments);
+        }
         builder.addFromRunnerArgs(runnerArgs);
         builder.addApkToScan(getContext().getPackageCodePath());
+        // See crbug://841695. TestLoader.isTestClass is incorrectly deciding that
+        // InstrumentationTestSuite is a test class.
+        builder.removeTestClass("android.test.InstrumentationTestSuite");
         return builder.build();
     }
 
@@ -195,18 +212,36 @@ public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
 
     /**
      * Wraps TestRequestBuilder to make it work with incremental install.
+     * TestRequestBuilder does not know to look through the incremental install dex files, and has
+     * no api for telling it to do so. This class checks to see if the list of tests was given
+     * by the runner (mHasClassList), and if not overrides the auto-detection logic in build()
+     * to manually scan all .dex files.
      */
     private static class IncrementalInstallTestRequestBuilder extends TestRequestBuilder {
-        List<String> mExcludedPrefixes = new ArrayList<String>();
+        final List<String> mExcludedPrefixes = new ArrayList<String>();
+        final DexFile[] mIncrementalJars;
         boolean mHasClassList;
 
-        public IncrementalInstallTestRequestBuilder(Instrumentation instr, Bundle bundle) {
+        IncrementalInstallTestRequestBuilder(
+                Instrumentation instr, Bundle bundle, DexFile[] incrementalJars) {
             super(instr, bundle);
+            mIncrementalJars = incrementalJars;
+            try {
+                Field excludedPackagesField =
+                        TestRequestBuilder.class.getDeclaredField("DEFAULT_EXCLUDED_PACKAGES");
+                excludedPackagesField.setAccessible(true);
+                mExcludedPrefixes.addAll(Arrays.asList((String[]) excludedPackagesField.get(null)));
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
         }
 
         @Override
         public TestRequestBuilder addFromRunnerArgs(RunnerArgs runnerArgs) {
             mExcludedPrefixes.addAll(runnerArgs.notTestPackages);
+            // Without clearing, You get IllegalArgumentException:
+            // Ambiguous arguments: cannot provide both test package and test class(es) to run
+            runnerArgs.notTestPackages.clear();
             return super.addFromRunnerArgs(runnerArgs);
         }
 
@@ -224,37 +259,17 @@ public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
 
         @Override
         public TestRequest build() {
-            // See crbug://841695. TestLoader.isTestClass is incorrectly deciding that
-            // InstrumentationTestSuite is a test class.
-            removeTestClass("android.test.InstrumentationTestSuite");
             // If a test class was requested, then no need to iterate class loader.
-            if (mHasClassList) {
-                return super.build();
-            }
-            maybeScanIncrementalClasspath();
-            return super.build();
-        }
-
-        private void maybeScanIncrementalClasspath() {
-            DexFile[] incrementalJars = null;
-            try {
-                Class<?> bootstrapClass =
-                        Class.forName("org.chromium.incrementalinstall.BootstrapApplication");
-                incrementalJars =
-                        (DexFile[]) bootstrapClass.getDeclaredField("sIncrementalDexFiles")
-                                .get(null);
-            } catch (Exception e) {
-                // Not an incremental apk.
-            }
-            if (incrementalJars != null) {
+            if (!mHasClassList) {
                 // builder.addApkToScan uses new DexFile(path) under the hood, which on Dalvik OS's
                 // assumes that the optimized dex is in the default location (crashes).
                 // Perform our own dex file scanning instead as a workaround.
-                addTestClasses(incrementalJars, this);
+                scanIncrementalJarsForTestClasses();
             }
+            return super.build();
         }
 
-        private boolean startsWithAny(String str, List<String> prefixes) {
+        private static boolean startsWithAny(String str, List<String> prefixes) {
             for (String prefix : prefixes) {
                 if (str.startsWith(prefix)) {
                     return true;
@@ -263,20 +278,11 @@ public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
             return false;
         }
 
-        private void addTestClasses(DexFile[] dexFiles, TestRequestBuilder builder) {
+        private void scanIncrementalJarsForTestClasses() {
             Log.i(TAG, "Scanning incremental classpath.");
-            try {
-                Field excludedPackagesField =
-                        TestRequestBuilder.class.getDeclaredField("DEFAULT_EXCLUDED_PACKAGES");
-                excludedPackagesField.setAccessible(true);
-                mExcludedPrefixes.addAll(Arrays.asList((String[]) excludedPackagesField.get(null)));
-            } catch (Exception e) {
-                throw new RuntimeException(e);
-            }
-
             // Mirror TestRequestBuilder.getClassNamesFromClassPath().
             TestLoader loader = new TestLoader();
-            for (DexFile dexFile : dexFiles) {
+            for (DexFile dexFile : mIncrementalJars) {
                 Enumeration<String> classNames = dexFile.entries();
                 while (classNames.hasMoreElements()) {
                     String className = classNames.nextElement();