Fix issue with forwarding arguments with space

The Android Platform has a [very naive options parsing implemented within the instrumentation code](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/cmds/am/src/com/android/commands/am/Am.java;l=160-204;drc=61197364367c9e404c7da6900658f1b16c42d0da;bpv=0;bpt=0?q=am.java&ss=android%2Fplatform%2Fsuperproject%2Fmain) where it assumes that arguments delimited by strings live in their own index (e.g. "am A B" is translated to 3 different strings of "am", "A", "B"). This is clearly illustrated by the usage of `opt.equals("..")` where spaces break that assumption. However, the bigger issue here is the error-reporting for this is horrible because it's being reported via `System.err` which the shell executor will give back to the Orchestrator, which puts it in a file that is not used at all! This behaviour should be changed and `stderr` must be printed out to the logcat. However, that's a separate issue

The previous trial to fix this have tried to throw an exception whenever white space was found. However, it seems that Kotlin allows test methods with space in them. In order to avoid falling into the same mistake, I did the following:

1. I avoided throwing an exception. This will allow unexpected corner-cases to keep working. However, if they are destined to fail then debugging will be a headache.
2. Only data that is ingested through `ORCHESTRATOR_FORWARDED_INSTRUMENTATION_ARGS` is processed. This is because keys can only exist in the value of that key, which is what can cause problems. Other sources of data don't need processing as they're not keys but values which the option parser can deal with.

PiperOrigin-RevId: 759194585

Author: copybara-androidxtest

runner/android_test_orchestrator/CHANGELOG.md

@@ -6,6 +6,10 @@
 
 **Bug Fixes**
 
+* Fix a bug where the instrumentation test application would not startup if the
+arguments passed to `ORCHESTRATOR_FORWARDED_INSTRUMENTATION_ARGS` contains
+spaces.
+
 **New Features**
 
 **Breaking Changes**

runner/android_test_orchestrator/java/androidx/test/orchestrator/TestRunnable.java

@@ -33,6 +33,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /** Runnable to run a single am instrument command to execute a single test. */
@@ -187,14 +188,26 @@ private Bundle getTargetInstrumentationArguments() {
 
   /**
    * Instrumentation params are delimited by comma, each param is stripped from leading and trailing
-   * whitespace.
+   * whitespace. *
+   *
+   * <p>The order of the params are critical to the correctness here as we split up params that have
+   * whitespace (eg: key value) into two different params `key` and `value` which means that those
+   * two different params must be next to each other the entire time.
    */
   private List<String> getInstrumentationParamsAndRemoveBundleArgs(Bundle arguments) {
     List<String> cleanedParams = new ArrayList<>();
     String forwardedArgs = arguments.getString(ORCHESTRATOR_FORWARDED_INSTRUMENTATION_ARGS);
     if (forwardedArgs != null) {
       for (String param : forwardedArgs.split(",")) {
-        cleanedParams.add(param.strip());
+        // The instrumentation code within the Android Platform was not designed to deal
+        // with whitespace in the arguments. The options parsing logic:
+        // https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/cmds/am/src/com/android/commands/am/Am.java;l=160-204;drc=61197364367c9e404c7da6900658f1b16c42d0da;bpv=0;bpt=0?q=am.java&ss=android%2Fplatform%2Fsuperproject%2Fmain
+        // uses `opt.equal("--key")` which assumes that each individual key will live
+        // in its separate string. However, if we start sending strings like "--key value" then
+        // options parsing will fail. The problem here is that this termination is very subtle.
+        // as the instrumentation does not report to logcat, but to System.err which can sometimes
+        // buffer the error and silently drop it on process exit.
+        Collections.addAll(cleanedParams, param.strip().split(" "));
       }
       arguments.remove(ORCHESTRATOR_FORWARDED_INSTRUMENTATION_ARGS);
     }

runner/android_test_orchestrator/javatests/androidx/test/orchestrator/BUILD

@@ -55,6 +55,7 @@ axt_android_local_test(
         "//ext/junit",
         "//runner/android_test_orchestrator",
         "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
         "@maven//:org_hamcrest_hamcrest_core",
         "@maven//:org_hamcrest_hamcrest_library",
     ],

runner/android_test_orchestrator/javatests/androidx/test/orchestrator/TestRunnableTest.java

@@ -16,6 +16,7 @@
 package androidx.test.orchestrator;
 
 import static androidx.test.core.app.ApplicationProvider.getApplicationContext;
+import static com.google.common.truth.Truth.assertThat;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.endsWith;
@@ -202,6 +203,33 @@ public void testRun_buildsParams_givenInstrumentationParamsAreHandledCorrectlySi
     assertContainsRunnerArgs(runnable.params, "--no-hidden-api-checks");
   }
 
+  @Test
+  public void testRun_buildsParams_givenInstrumentationParamsAreSpaceDelimited() {
+    arguments.putString("orchestratorInstrumentationArgs", "--abi x86_64");
+    FakeListener listener = new FakeListener();
+    FakeTestRunnable runnable =
+        new FakeTestRunnable(context, "secret", arguments, outputStream, listener, null, true);
+    runnable.run();
+    assertThat(runnable.params).containsAtLeast("--abi", "x86_64").inOrder();
+  }
+
+  @Test
+  public void testRun_buildsParams_allowTestNamesWithSpace() {
+    FakeListener listener = new FakeListener();
+    FakeTestRunnable runnable =
+        new FakeTestRunnable(
+            context,
+            "secret",
+            arguments,
+            outputStream,
+            listener,
+            "com.google.android.example.MyClass#methodNameWith Space",
+            true);
+    runnable.run();
+    assertContainsRunnerArgs(
+        runnable.params, "-e class com.google.android.example.MyClass#methodNameWith Space");
+  }
+
   @Test
   public void testRun_buildsParams_givenInstrumentationParamsAreHandledCorrectlyMultipleParam() {
     arguments.putString("orchestratorInstrumentationArgs", "--A,--B,--C,--key value");