From 12fc87cb670f93b09f50e3f4637e1a4fa740b111 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 7 Jun 2019 11:33:41 -0700 Subject: [PATCH] Remove obsolete fields from AndroidInstrumentationInfo. AndroidInstrumentationInfo is used to access the target/instrumentation app while ApkInfo is used to access the test app. RELNOTES: None PiperOrigin-RevId: 252087467 --- .../lib/rules/android/AndroidBinary.java | 5 +-- .../android/AndroidInstrumentationInfo.java | 35 +++--------------- .../AndroidInstrumentationTestBase.java | 9 +++-- .../android/AndroidBootstrap.java | 4 +-- .../AndroidInstrumentationInfoApi.java | 36 ++----------------- .../FakeAndroidInstrumentationInfo.java | 19 +++------- .../lib/rules/android/AndroidBinaryTest.java | 24 +++++-------- .../AndroidInstrumentationTestTest.java | 4 +-- 8 files changed, 32 insertions(+), 104 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 0bb4eeafb847c2..5e10d0ec12cd25 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -574,11 +574,8 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( ApkInfo targetApkProvider = ruleContext.getPrerequisite("instruments", Mode.TARGET, ApkInfo.PROVIDER); - Artifact targetApk = targetApkProvider.getApk(); - Artifact instrumentationApk = zipAlignedApk; - AndroidInstrumentationInfo instrumentationProvider = - new AndroidInstrumentationInfo(targetApk, instrumentationApk, targetApkProvider); + new AndroidInstrumentationInfo(targetApkProvider); builder.addNativeDeclaredProvider(instrumentationProvider); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationInfo.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationInfo.java index 7b73000f18e493..5444dfe04bfba3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationInfo.java @@ -13,54 +13,31 @@ // limitations under the License. package com.google.devtools.build.lib.rules.android; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuiltinProvider; import com.google.devtools.build.lib.packages.NativeInfo; import com.google.devtools.build.lib.skylarkbuildapi.android.AndroidInstrumentationInfoApi; import com.google.devtools.build.lib.syntax.EvalException; -import com.google.devtools.build.lib.syntax.Runtime; /** * A provider for targets that create Android instrumentations. Consumed by Android testing rules. */ @Immutable public class AndroidInstrumentationInfo extends NativeInfo - implements AndroidInstrumentationInfoApi { + implements AndroidInstrumentationInfoApi { private static final String SKYLARK_NAME = "AndroidInstrumentationInfo"; public static final AndroidInstrumentationInfoProvider PROVIDER = new AndroidInstrumentationInfoProvider(); - private final Artifact targetApk; - private final Artifact instrumentationApk; private final ApkInfo target; - AndroidInstrumentationInfo(Artifact targetApk, Artifact instrumentationApk) { + AndroidInstrumentationInfo(ApkInfo target) { super(PROVIDER); - this.targetApk = targetApk; - this.instrumentationApk = instrumentationApk; - this.target = null; - } - - AndroidInstrumentationInfo(Artifact targetApk, Artifact instrumentationApk, ApkInfo target) { - super(PROVIDER); - this.targetApk = targetApk; - this.instrumentationApk = instrumentationApk; this.target = target; } - @Override - public Artifact getTargetApk() { - return targetApk; - } - - @Override - public Artifact getInstrumentationApk() { - return instrumentationApk; - } - @Override public ApkInfo getTarget() { return target; @@ -69,17 +46,15 @@ public ApkInfo getTarget() { /** Provider for {@link AndroidInstrumentationInfo}. */ public static class AndroidInstrumentationInfoProvider extends BuiltinProvider - implements AndroidInstrumentationInfoApiProvider { + implements AndroidInstrumentationInfoApiProvider { private AndroidInstrumentationInfoProvider() { super(SKYLARK_NAME, AndroidInstrumentationInfo.class); } @Override - public AndroidInstrumentationInfoApi createInfo( - Artifact targetApk, Artifact instrumentationApk, Object target) throws EvalException { - return new AndroidInstrumentationInfo( - targetApk, instrumentationApk, target == Runtime.NONE ? null : (ApkInfo) target); + public AndroidInstrumentationInfoApi createInfo(ApkInfo target) throws EvalException { + return new AndroidInstrumentationInfo(target); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java index 991da36be91648..059d271b21fae9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java @@ -207,10 +207,15 @@ private static AndroidInstrumentationInfo getInstrumentationProvider(RuleContext "test_app", Mode.TARGET, AndroidInstrumentationInfo.PROVIDER); } + @Nullable + private static ApkInfo getApkProvider(RuleContext ruleContext) { + return ruleContext.getPrerequisite("test_app", Mode.TARGET, ApkInfo.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(); + return getInstrumentationProvider(ruleContext).getTarget().getApk(); } /** @@ -219,7 +224,7 @@ private static Artifact getTargetApk(RuleContext ruleContext) { */ @Nullable private static Artifact getInstrumentationApk(RuleContext ruleContext) { - return getInstrumentationProvider(ruleContext).getInstrumentationApk(); + return getApkProvider(ruleContext).getApk(); } /** The support APKs from the {@code support_apks} and {@code fixtures} attributes. */ diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidBootstrap.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidBootstrap.java index 472293bcba76c0..b41e4c1bd592de 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidBootstrap.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidBootstrap.java @@ -31,7 +31,7 @@ public class AndroidBootstrap implements Bootstrap { private final AndroidSkylarkCommonApi androidCommon; private final ApkInfoApiProvider apkInfoProvider; - private final AndroidInstrumentationInfoApiProvider androidInstrumentationInfoProvider; + private final AndroidInstrumentationInfoApiProvider androidInstrumentationInfoProvider; private final AndroidDeviceBrokerInfoApiProvider androidDeviceBrokerInfoProvider; private final AndroidResourcesInfoApi.AndroidResourcesInfoApiProvider androidResourcesInfoProvider; @@ -40,7 +40,7 @@ public class AndroidBootstrap implements Bootstrap { public AndroidBootstrap( AndroidSkylarkCommonApi androidCommon, ApkInfoApiProvider apkInfoProvider, - AndroidInstrumentationInfoApiProvider androidInstrumentationInfoProvider, + AndroidInstrumentationInfoApiProvider androidInstrumentationInfoProvider, AndroidDeviceBrokerInfoApiProvider androidDeviceBrokerInfoProvider, AndroidResourcesInfoApiProvider androidResourcesInfoProvider, AndroidNativeLibsInfoApiProvider androidNativeLibsInfoProvider) { diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidInstrumentationInfoApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidInstrumentationInfoApi.java index 402ddb1f224b5b..6646df38f175b3 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidInstrumentationInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/AndroidInstrumentationInfoApi.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.skylarkbuildapi.android; -import com.google.devtools.build.lib.skylarkbuildapi.FileApi; import com.google.devtools.build.lib.skylarkbuildapi.ProviderApi; import com.google.devtools.build.lib.skylarkbuildapi.StructApi; import com.google.devtools.build.lib.skylarkinterface.Param; @@ -34,28 +33,13 @@ + "Android instrumentation and target APKs to run in a test", documented = false, category = SkylarkModuleCategory.PROVIDER) -public interface AndroidInstrumentationInfoApi> - extends StructApi { +public interface AndroidInstrumentationInfoApi> extends StructApi { /** * Name of this info object. */ public static String NAME = "AndroidInstrumentationInfo"; - @SkylarkCallable( - name = "target_apk", - doc = "Returns the target APK of the instrumentation test.", - documented = false, - structField = true) - FileT getTargetApk(); - - @SkylarkCallable( - name = "instrumentation_apk", - doc = "Returns the instrumentation APK that should be executed.", - documented = false, - structField = true) - FileT getInstrumentationApk(); - @SkylarkCallable( name = "target", doc = "Returns the target ApkInfo of the instrumentation test.", @@ -71,8 +55,7 @@ public interface AndroidInstrumentationInfoApi> + public interface AndroidInstrumentationInfoApiProvider> extends ProviderApi { @SkylarkCallable( @@ -80,27 +63,14 @@ public interface AndroidInstrumentationInfoApiProvider< doc = "The AndroidInstrumentationInfo constructor.", documented = false, parameters = { - @Param( - name = "target_apk", - type = FileApi.class, - named = true, - doc = "The target APK of the instrumentation test."), - @Param( - name = "instrumentation_apk", - type = FileApi.class, - named = true, - doc = "The instrumentation APK that should be executed."), @Param( name = "target", type = ApkInfoApi.class, named = true, - defaultValue = "None", - noneable = true, doc = "The target ApkInfo of the instrumentation test.") }, selfCall = true) @SkylarkConstructor(objectType = AndroidInstrumentationInfoApi.class, receiverNameForDoc = NAME) - public AndroidInstrumentationInfoApi createInfo( - FileT targetApk, FileT instrumentationApk, Object target) throws EvalException; + public AndroidInstrumentationInfoApi createInfo(ApkT target) throws EvalException; } } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidInstrumentationInfo.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidInstrumentationInfo.java index aa7dc164eaebc4..8f59c3e84e6934 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidInstrumentationInfo.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidInstrumentationInfo.java @@ -15,7 +15,6 @@ package com.google.devtools.build.skydoc.fakebuildapi.android; import com.google.devtools.build.lib.events.Location; -import com.google.devtools.build.lib.skylarkbuildapi.FileApi; import com.google.devtools.build.lib.skylarkbuildapi.android.AndroidInstrumentationInfoApi; import com.google.devtools.build.lib.skylarkbuildapi.android.ApkInfoApi; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; @@ -23,17 +22,7 @@ /** Fake implementation of {@link AndroidInstrumentationInfoApi}. */ public class FakeAndroidInstrumentationInfo - implements AndroidInstrumentationInfoApi> { - - @Override - public FileApi getTargetApk() { - return null; - } - - @Override - public FileApi getInstrumentationApk() { - return null; - } + implements AndroidInstrumentationInfoApi> { @Override public ApkInfoApi getTarget() { @@ -55,11 +44,11 @@ public void repr(SkylarkPrinter printer) {} /** Fake implementation of {@link AndroidInstrumentationInfoApiProvider}. */ public static class FakeAndroidInstrumentationInfoProvider - implements AndroidInstrumentationInfoApiProvider> { + implements AndroidInstrumentationInfoApiProvider> { @Override - public AndroidInstrumentationInfoApi> createInfo( - FileApi targetApk, FileApi instrumentationApk, Object target) throws EvalException { + public AndroidInstrumentationInfoApi> createInfo(ApkInfoApi target) + throws EvalException { return new FakeAndroidInstrumentationInfo(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 837bad93c4d014..252bf6d1450966 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -4267,16 +4267,14 @@ public void testInstrumentationInfoAccessibleFromSkylark() throws Exception { scratch.file( "java/com/google/android/instr/instr.bzl", "def _impl(ctx):", - " target = ctx.attr.dep[AndroidInstrumentationInfo].target_apk", - " instr = ctx.attr.dep[AndroidInstrumentationInfo].instrumentation_apk", - " return [DefaultInfo(files=depset([target,instr]))]", + " target = ctx.attr.dep[AndroidInstrumentationInfo].target.signed_apk", + " return [DefaultInfo(files=depset([target]))]", "instr = rule(implementation=_impl,", " attrs={'dep': attr.label(providers=[AndroidInstrumentationInfo])})"); ConfiguredTarget instr = getConfiguredTarget("//java/com/google/android/instr"); assertThat(instr).isNotNull(); assertThat(prettyArtifactNames(instr.getProvider(FilesToRunProvider.class).getFilesToRun())) - .containsExactly( - "java/com/google/android/instr/b1.apk", "java/com/google/android/instr/b2.apk"); + .containsExactly("java/com/google/android/instr/b2.apk"); } @Test @@ -4295,17 +4293,14 @@ public void testInstrumentationInfoCreatableFromSkylark() throws Exception { scratch.file( "java/com/google/android/instr/instr.bzl", "def _impl(ctx):", - " target = ctx.attr.dep[AndroidInstrumentationInfo].target_apk", - " instr = ctx.attr.dep[AndroidInstrumentationInfo].instrumentation_apk", - " return [AndroidInstrumentationInfo(target_apk=target,instrumentation_apk=instr)]", + " target = ctx.attr.dep[AndroidInstrumentationInfo].target", + " return [AndroidInstrumentationInfo(target=target)]", "instr = rule(implementation=_impl,", " attrs={'dep': attr.label(providers=[AndroidInstrumentationInfo])})"); ConfiguredTarget instr = getConfiguredTarget("//java/com/google/android/instr"); assertThat(instr).isNotNull(); - assertThat(instr.get(AndroidInstrumentationInfo.PROVIDER).getTargetApk().prettyPrint()) + assertThat(instr.get(AndroidInstrumentationInfo.PROVIDER).getTarget().getApk().prettyPrint()) .isEqualTo("java/com/google/android/instr/b2.apk"); - assertThat(instr.get(AndroidInstrumentationInfo.PROVIDER).getInstrumentationApk().prettyPrint()) - .isEqualTo("java/com/google/android/instr/b1.apk"); } @Test @@ -4321,12 +4316,9 @@ public void testInstrumentationInfoProviderHasApks() throws Exception { " manifest = 'AndroidManifest.xml')"); ConfiguredTarget b1 = getConfiguredTarget("//java/com/google/android/instr:b1"); AndroidInstrumentationInfo provider = b1.get(AndroidInstrumentationInfo.PROVIDER); - assertThat(provider.getTargetApk()).isNotNull(); - assertThat(provider.getTargetApk().prettyPrint()) + assertThat(provider.getTarget()).isNotNull(); + assertThat(provider.getTarget().getApk().prettyPrint()) .isEqualTo("java/com/google/android/instr/b2.apk"); - assertThat(provider.getInstrumentationApk()).isNotNull(); - assertThat(provider.getInstrumentationApk().prettyPrint()) - .isEqualTo("java/com/google/android/instr/b1.apk"); } @Test 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 3a55fd3b739a4a..b56acedfca16ea 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 @@ -258,11 +258,11 @@ private static Artifact getDeviceFixtureScript(ConfiguredTarget deviceScriptFixt } private static Artifact getInstrumentationApk(ConfiguredTarget instrumentation) { - return instrumentation.get(AndroidInstrumentationInfo.PROVIDER).getInstrumentationApk(); + return instrumentation.get(AndroidInstrumentationInfo.PROVIDER).getTarget().getApk(); } private static Artifact getTargetApk(ConfiguredTarget instrumentation) { - return instrumentation.get(AndroidInstrumentationInfo.PROVIDER).getTargetApk(); + return instrumentation.get(ApkInfo.PROVIDER).getApk(); } private String getTestStubContents(ConfiguredTarget androidInstrumentationTest) throws Exception {