Modify android_instrumentation_test.instrumentation to accept android_binary instead of android_instrumentation.

GITHUB: #903

RELNOTES: None.
PiperOrigin-RevId: 173572553

4 files changed, 73 insertions, 64 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTest.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTest.java
index 78a21cc5a0..20e9f4bbb8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTest.java
@@ -35,12 +35,13 @@ import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTa
 import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
 import java.io.IOException;
 import javax.annotation.Nullable;
 
-/** An implementation of the {@code android_instrumentation} rule. */
+/** An implementation of the {@code android_instrumentation_test} rule. */
 public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
 
   private static final Template ANDROID_INSTRUMENTATION_TEST_STUB_SCRIPT =
@@ -48,15 +49,29 @@ public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
           AndroidInstrumentationTest.class, "android_instrumentation_test_template.txt");
   private static final String TEST_SUITE_PROPERTY_NAME_FILE = "test_suite_property_name.txt";
 
+  /** Checks expected rule invariants, throws rule errors if anything is set wrong. */
+  private static void validateRuleContext(RuleContext ruleContext)
+      throws InterruptedException, RuleErrorException {
+    if (getInstrumentationProvider(ruleContext) == null) {
+      ruleContext.throwWithAttributeError(
+          "instrumentation",
+          String.format(
+              "The android_binary target at %s is missing an 'instruments' attribute. Please set "
+                  + "it as the label of the android_binary under test.",
+              ruleContext.attributes().get("instrumentation", BuildType.LABEL)));
+    }
+  }
+
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
+    validateRuleContext(ruleContext);
+
     // The wrapper script that invokes the test entry point.
     Artifact testExecutable = createTestExecutable(ruleContext);
 
     ImmutableList<TransitiveInfoCollection> runfilesDeps =
         ImmutableList.<TransitiveInfoCollection>builder()
-            .addAll(ruleContext.getPrerequisites("instrumentations", Mode.TARGET))
             .addAll(ruleContext.getPrerequisites("fixtures", Mode.TARGET))
             .add(ruleContext.getPrerequisite("target_device", Mode.HOST))
             .add(ruleContext.getPrerequisite("$test_entry_point", Mode.HOST))
@@ -65,6 +80,8 @@ public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
             .addArtifact(testExecutable)
+            .addArtifact(getInstrumentationApk(ruleContext))
+            .addArtifact(getTargetApk(ruleContext))
             .addTargets(runfilesDeps, RunfilesProvider.DEFAULT_RUNFILES)
             .addTransitiveArtifacts(AndroidCommon.getSupportApks(ruleContext))
             .addTransitiveArtifacts(getAdb(ruleContext).getFilesToRun())
@@ -105,9 +122,8 @@ public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
         .add(executableSubstitution("%adb%", getAdb(ruleContext)))
         .add(executableSubstitution("%device_script%", getTargetDevice(ruleContext)))
         .add(executableSubstitution("%test_entry_point%", getTestEntryPoint(ruleContext)))
-        .add(artifactListSubstitution("%target_apks%", getTargetApks(ruleContext)))
-        .add(
-            artifactListSubstitution("%instrumentation_apks%", getInstrumentationApks(ruleContext)))
+        .add(artifactSubstitution("%target_apk%", getTargetApk(ruleContext)))
+        .add(artifactSubstitution("%instrumentation_apk%", getInstrumentationApk(ruleContext)))
         .add(artifactListSubstitution("%support_apks%", getAllSupportApks(ruleContext)))
         .add(Substitution.ofSpaceSeparatedMap("%test_args%", getTestArgs(ruleContext)))
         .add(Substitution.ofSpaceSeparatedMap("%fixture_args%", getFixtureArgs(ruleContext)))
@@ -167,6 +183,10 @@ public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
     return Substitution.of(key, filesToRunProvider.getExecutable().getRunfilesPathString());
   }
 
+  private static Substitution artifactSubstitution(String key, Artifact artifact) {
+    return Substitution.of(key, artifact.getRunfilesPathString());
+  }
+
   private static Substitution artifactListSubstitution(String key, Iterable<Artifact> artifacts) {
     return Substitution.ofSpaceSeparatedList(
         key,
@@ -175,26 +195,25 @@ public class AndroidInstrumentationTest implements RuleConfiguredTargetFactory {
             .collect(ImmutableList.toImmutableList()));
   }
 
-  /**
-   * The target APKs from each {@code android_instrumentation} in the {@code instrumentations}
-   * attribute.
-   */
-  private static Iterable<Artifact> getTargetApks(RuleContext ruleContext) {
-    return Iterables.transform(
-        ruleContext.getPrerequisites(
-            "instrumentations", Mode.TARGET, AndroidInstrumentationInfo.PROVIDER),
-        AndroidInstrumentationInfo::getTargetApk);
+  @Nullable
+  private static AndroidInstrumentationInfo getInstrumentationProvider(RuleContext ruleContext) {
+    return ruleContext.getPrerequisite(
+        "instrumentation", Mode.TARGET, AndroidInstrumentationInfo.PROVIDER);
+  }
+
+  /** The target APK from the {@code android_binary} in the {@code instrumentation} attribute. */
+  @Nullable
+  private static Artifact getTargetApk(RuleContext ruleContext) {
+    return getInstrumentationProvider(ruleContext).getTargetApk();
   }
 
   /**
-   * The instrumentation APKs from each {@code android_instrumentation} in the {@code
-   * instrumentations} attribute.
+   * The instrumentation APK from the {@code android_binary} in the {@code instrumentation}
+   * attribute.
    */
-  private static Iterable<Artifact> getInstrumentationApks(RuleContext ruleContext) {
-    return Iterables.transform(
-        ruleContext.getPrerequisites(
-            "instrumentations", Mode.TARGET, AndroidInstrumentationInfo.PROVIDER),
-        AndroidInstrumentationInfo::getInstrumentationApk);
+  @Nullable
+  private static Artifact getInstrumentationApk(RuleContext ruleContext) {
+    return getInstrumentationProvider(ruleContext).getInstrumentationApk();
   }
 
   /** The support APKs from the {@code support_apks} and {@code fixtures} attributes. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestRule.java
index bb65e2e797..23f8dbe632 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestRule.java
@@ -35,10 +35,10 @@ public class AndroidInstrumentationTestRule implements RuleDefinition {
     return builder
         .setUndocumented()
         .add(
-            attr("instrumentations", LABEL_LIST)
+            attr("instrumentation", LABEL)
                 .mandatory()
                 .allowedFileTypes(FileTypeSet.NO_FILE)
-                .allowedRuleClasses("android_instrumentation"))
+                .allowedRuleClasses("android_binary"))
         .add(
             attr("target_device", LABEL)
                 .mandatory()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/android_instrumentation_test_template.txt b/src/main/java/com/google/devtools/build/lib/rules/android/android_instrumentation_test_template.txt
index c09d1cef10..fb92b97e2c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/android_instrumentation_test_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/android_instrumentation_test_template.txt
@@ -52,16 +52,14 @@ data_deps=$(join_paths ${WORKSPACE_DIR} "," ${data_deps})
 device_broker_type="%device_broker_type%"
 test_label="%test_label%"
 
-target_apks="%target_apks%"
-target_apks=$(join_paths ${WORKSPACE_DIR} "," ${target_apks})
+target_apk="%target_apk%"
 
-instrumentation_apks="%instrumentation_apks%"
-instrumentation_apks=$(join_paths ${WORKSPACE_DIR} "," ${instrumentation_apks})
+instrumentation_apk="%instrumentation_apk%"
 
 support_apks="%support_apks%"
 support_apks=$(join_paths ${WORKSPACE_DIR} "," ${support_apks})
 
-apks_to_install="${support_apks}${target_apks}${instrumentation_apks}"
+apks_to_install="${support_apks}${target_apk},${instrumentation_apk}"
 
 declare -A device_script_fixtures=( %device_script_fixtures% )
 
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestTest.java
index 647e583bcc..6084ee30b5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestTest.java
@@ -38,11 +38,7 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
     scratch.file(
         "java/com/app/BUILD",
         "android_binary(",
-        "  name = 'app1',",
-        "  manifest = 'AndroidManifest.xml',",
-        ")",
-        "android_binary(",
-        "  name = 'app2',",
+        "  name = 'app',",
         "  manifest = 'AndroidManifest.xml',",
         ")",
         "android_binary(",
@@ -52,23 +48,10 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
     scratch.file(
         "javatests/com/app/BUILD",
         "android_binary(",
-        "  name = 'instrumentation_app1',",
-        "  manifest = 'AndroidManifest.xml',",
-        ")",
-        "android_instrumentation(",
-        "  name = 'instrumentation1',",
-        "  target = '//java/com/app:app1',",
-        "  instrumentation = ':instrumentation_app1',",
-        ")",
-        "android_binary(",
-        "  name = 'instrumentation_app2',",
+        "  name = 'instrumentation_app',",
+        "  instruments = '//java/com/app',",
         "  manifest = 'AndroidManifest.xml',",
         ")",
-        "android_instrumentation(",
-        "  name = 'instrumentation2',",
-        "  target = '//java/com/app:app2',",
-        "  instrumentation = ':instrumentation_app2',",
-        ")",
         "android_device_script_fixture(",
         "  name = 'device_fixture',",
         "  cmd = 'foo bar',",
@@ -89,10 +72,7 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
         "javatests/com/app/ait/BUILD",
         "android_instrumentation_test(",
         "  name = 'ait',",
-        "  instrumentations = [",
-        "    '//javatests/com/app:instrumentation1',",
-        "    '//javatests/com/app:instrumentation2',",
-        "  ],",
+        "  instrumentation = '//javatests/com/app:instrumentation_app',",
         "  target_device = '//tools/android/emulated_device:nexus_6',",
         "  fixtures = [",
         "    '//javatests/com/app:device_fixture',",
@@ -168,10 +148,8 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
     assertThat(runfiles)
         .containsAllOf(
             getDeviceFixtureScript(getConfiguredTarget("//javatests/com/app:device_fixture")),
-            getInstrumentationApk(getConfiguredTarget("//javatests/com/app:instrumentation1")),
-            getTargetApk(getConfiguredTarget("//javatests/com/app:instrumentation1")),
-            getInstrumentationApk(getConfiguredTarget("//javatests/com/app:instrumentation2")),
-            getTargetApk(getConfiguredTarget("//javatests/com/app:instrumentation2")),
+            getInstrumentationApk(getConfiguredTarget("//javatests/com/app:instrumentation_app")),
+            getTargetApk(getConfiguredTarget("//javatests/com/app:instrumentation_app")),
             Iterables.getOnlyElement(
                 getConfiguredTarget("//javatests/com/app/ait:foo.txt")
                     .getProvider(FileProvider.class)
@@ -192,13 +170,8 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
             .getFileContents();
 
     assertThat(testExecutableScript)
-        .contains(
-            "instrumentation_apks=\"javatests/com/app/instrumentation1-instrumentation.apk "
-                + "javatests/com/app/instrumentation2-instrumentation.apk\"");
-    assertThat(testExecutableScript)
-        .contains(
-            "target_apks=\"javatests/com/app/instrumentation1-target.apk "
-                + "javatests/com/app/instrumentation2-target.apk\"");
+        .contains("instrumentation_apk=\"javatests/com/app/instrumentation_app.apk\"");
+    assertThat(testExecutableScript).contains("target_apk=\"java/com/app/app.apk\"");
     assertThat(testExecutableScript).contains("support_apks=\"java/com/app/support.apk\"");
     assertThat(testExecutableScript)
         .contains(
@@ -224,7 +197,7 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
         ")",
         "android_instrumentation_test(",
         "  name = 'ait',",
-        "  instrumentations = ['//javatests/com/app:instrumentation1'],",
+        "  instrumentation = '//javatests/com/app:instrumentation_app',",
         "  target_device = '//tools/android/emulated_device:nexus_6',",
         "  fixtures = [",
         "    ':host_fixture',",
@@ -233,6 +206,25 @@ public class AndroidInstrumentationTestTest extends AndroidBuildViewTestCase {
         ")");
   }
 
+  @Test
+  public void testInstrumentationBinaryIsInstrumenting() throws Exception {
+    checkError(
+        "javatests/com/app/instr",
+        "ait",
+        "The android_binary target at //javatests/com/app/instr:app "
+            + "is missing an 'instruments' attribute",
+        "android_binary(",
+        "  name = 'app',",
+        "  srcs = ['a.java'],",
+        "  manifest = 'AndroidManifest.xml',",
+        ")",
+        "android_instrumentation_test(",
+        "  name = 'ait',",
+        "  instrumentation = ':app',",
+        "  target_device = '//tools/android/emulated_device:nexus_6',",
+        ")");
+  }
+
   private static Artifact getDeviceFixtureScript(ConfiguredTarget deviceScriptFixture) {
     return getFirstArtifactEndingWith(
         deviceScriptFixture.getProvider(FileProvider.class).getFilesToBuild(), ".sh");