diff options
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"); |