Merge branch 'main' into testWithWhitespace

Author: brettchabot

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

@@ -33,7 +33,6 @@
 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. */
@@ -189,32 +188,19 @@ private Bundle getTargetInstrumentationArguments() {
   /**
    * Instrumentation params are delimited by comma, each param is stripped from leading and trailing
    * 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(",")) {
-        // ShellExecutor exhibits weird behavior when a param has a whitespace in it.
-        // so we need to split by white-space to remove the spaces.
-        Collections.addAll(cleanedParams, param.strip().split(" "));
+        cleanedParams.add(param.strip());
       }
       arguments.remove(ORCHESTRATOR_FORWARDED_INSTRUMENTATION_ARGS);
     }
     return cleanedParams;
   }
 
-  /**
-   * This method must maintain the order of the params
-   *
-   * <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> buildShellParams(Bundle arguments) throws IOException, ClientNotConnected {
     List<String> params = new ArrayList<>();
     params.add("instrument");
@@ -229,12 +215,6 @@ private List<String> buildShellParams(Bundle arguments) throws IOException, Clie
       params.add(arguments.getString(key));
     }
     params.add(getTargetInstrumentation());
-    for (String param : params) {
-      if (param.isEmpty() || param.contains(" ")) {
-        throw new IllegalStateException(
-            "Params must not contain any white-space to avoid encoding issues.");
-      }
-    }
     return params;
   }
 

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

@@ -52,7 +52,6 @@ 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

@@ -21,13 +21,11 @@
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
-import static org.junit.Assert.assertThrows;
 
 import android.content.Context;
 import android.os.Bundle;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.common.base.Joiner;
-import com.google.common.truth.Truth;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -214,24 +212,6 @@ public void testRun_buildsParams_givenInstrumentationParamsAreHandledCorrectlyMu
     assertContainsRunnerArgs(runnable.params, "--A --B --C --key value");
   }
 
-  @Test
-  public void testRun_buildsParams_givenValueWithSpaceParamThrows() {
-    arguments.putString("someArgument", "A B");
-    FakeListener listener = new FakeListener();
-    FakeTestRunnable runnable =
-        new FakeTestRunnable(context, "secret", arguments, outputStream, listener, null, true);
-    assertThrows(IllegalStateException.class, runnable::run);
-  }
-
-  @Test
-  public void testRun_buildsParams_givenEmptyStringParamThrows() {
-    arguments.putString("someArgument", "");
-    FakeListener listener = new FakeListener();
-    FakeTestRunnable runnable =
-        new FakeTestRunnable(context, "secret", arguments, outputStream, listener, null, true);
-    assertThrows(IllegalStateException.class, runnable::run);
-  }
-
   private static void assertContainsRunnerArgs(List<String> params, String... containsArgs) {
     String cmdArgs = Joiner.on(" ").join(params);
     assertThat(cmdArgs, startsWith("instrument -w -r"));
@@ -240,14 +220,4 @@ private static void assertContainsRunnerArgs(List<String> params, String... cont
     }
     assertThat(cmdArgs, endsWith("targetInstrumentation/targetRunner"));
   }
-
-  @Test
-  public void testRun_buildsParams_givenInstrumentationParamsWithSpaceMaintainsOrder() {
-    arguments.putString("orchestratorInstrumentationArgs", "--A,--B,--key value");
-    FakeListener listener = new FakeListener();
-    FakeTestRunnable runnable =
-        new FakeTestRunnable(context, "secret", arguments, outputStream, listener, null, true);
-    runnable.run();
-    Truth.assertThat(runnable.params).containsAtLeast("--A", "--B", "--key", "value").inOrder();
-  }
 }