From 81dbe79d09b4fa98d7803018b5d6d026e2e52a2e Mon Sep 17 00:00:00 2001 From: asteinb Date: Tue, 3 Apr 2018 07:08:09 -0700 Subject: Add methods to parse resources without assets - Add ParsedAndroidResources to wrap AndroidResources and resource parsing output. - Implement parse() method in AndroidResources, and support for it elsewhere - Move some supporting methods to the right place (setting up an aapt2 sdk for tests goes to the base test rule, and creating a dummy DataBinding zip goes to the DataBinding class). - Tests for new parse() method, including support for getting a test RuleContext instance RELNOTES: none PiperOrigin-RevId: 191436027 --- .../build/lib/rules/android/AndroidBinaryTest.java | 21 --- .../rules/android/AndroidBuildViewTestCase.java | 26 +++ .../lib/rules/android/AndroidResourcesTest.java | 174 ++++++++++++++++++++- .../google/devtools/build/lib/rules/android/BUILD | 3 + .../build/lib/rules/android/ResourceTestBase.java | 2 +- 5 files changed, 201 insertions(+), 25 deletions(-) (limited to 'src/test/java/com/google/devtools/build') 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 499da948b7..61d238f8d2 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 @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.testutil.MoreAsserts; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -3400,26 +3399,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ")"); } - private void mockAndroidSdkWithAapt2() throws IOException { - scratch.file( - "sdk/BUILD", - "android_sdk(", - " name = 'sdk',", - " aapt = 'aapt',", - " aapt2 = 'aapt2',", - " adb = 'adb',", - " aidl = 'aidl',", - " android_jar = 'android.jar',", - " apksigner = 'apksigner',", - " dx = 'dx',", - " framework_aidl = 'framework_aidl',", - " main_dex_classes = 'main_dex_classes',", - " main_dex_list_creator = 'main_dex_list_creator',", - " proguard = 'proguard',", - " shrinked_android_jar = 'shrinked_android_jar',", - " zipalign = 'zipalign')"); - } - @Test public void testAapt2WithAndroidSdk() throws Exception { mockAndroidSdkWithAapt2(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java index 6deb85f35c..48accc2d5c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java @@ -383,4 +383,30 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { .getActionForArtifactEndingWith(getFilesToBuild(binary), "_proguard.jar")) .isNull(); } + + /** + * Creates a mock SDK with aapt2. + * + *

You'll need to use a configuration pointing to it, such as "--android_sdk=//sdk:sdk", to use + * it. + */ + void mockAndroidSdkWithAapt2() throws Exception { + scratch.file( + "sdk/BUILD", + "android_sdk(", + " name = 'sdk',", + " aapt = 'aapt',", + " aapt2 = 'aapt2',", + " adb = 'adb',", + " aidl = 'aidl',", + " android_jar = 'android.jar',", + " apksigner = 'apksigner',", + " dx = 'dx',", + " framework_aidl = 'framework_aidl',", + " main_dex_classes = 'main_dex_classes',", + " main_dex_list_creator = 'main_dex_list_creator',", + " proguard = 'proguard',", + " shrinked_android_jar = 'shrinked_android_jar',", + " zipalign = 'zipalign')"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java index 056970f322..1f81b1300c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java @@ -19,10 +19,20 @@ import static com.google.common.truth.Truth.assertWithMessage; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Streams; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Optional; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -35,6 +45,8 @@ public class AndroidResourcesTest extends ResourceTestBase { private static final ImmutableList RESOURCES_ROOTS = ImmutableList.of(DEFAULT_RESOURCE_ROOT); + private static final ImmutableSet TOOL_FILENAMES = ImmutableSet.of("aapt2", "empty.sh"); + @Before @Test public void testGetResourceRootsNoResources() throws Exception { @@ -91,7 +103,7 @@ public class AndroidResourcesTest extends ResourceTestBase { @Test public void testFilterEmpty() throws Exception { - assertFilter(ImmutableList.of(), ImmutableList.of()); + assertFilter(ImmutableList.of(), ImmutableList.of()); } @Test @@ -102,8 +114,7 @@ public class AndroidResourcesTest extends ResourceTestBase { @Test public void testFilterToEmpty() throws Exception { - assertFilter( - getResources("values-en/foo.xml", "values-es/bar.xml"), ImmutableList.of()); + assertFilter(getResources("values-en/foo.xml", "values-es/bar.xml"), ImmutableList.of()); } @Test @@ -168,4 +179,161 @@ public class AndroidResourcesTest extends ResourceTestBase { Iterables.concat(filteredDepsBuilder.build(), filteredResources)); } } + + @Test + public void testParseNoCompile() throws Exception { + useConfiguration("--android_aapt=aapt"); + + RuleContext ruleContext = getRuleContext(/* useDataBinding = */ true); + ParsedAndroidResources parsed = assertParse(ruleContext); + + // Since we are not using aapt2, there should be no compiled symbols + assertThat(parsed.getCompiledSymbols()).isNull(); + + // The parse action should take resources in and output symbols + assertActionArtifacts( + ruleContext, + /* inputs = */ parsed.getResources(), + /* outputs = */ ImmutableList.of(parsed.getSymbols())); + } + + @Test + public void testParseAndCompile() throws Exception { + mockAndroidSdkWithAapt2(); + useConfiguration("--android_sdk=//sdk:sdk", "--android_aapt=aapt2"); + + RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false); + ParsedAndroidResources parsed = assertParse(ruleContext); + + assertThat(parsed.getCompiledSymbols()).isNotNull(); + + // The parse action should take resources in and output symbols + assertActionArtifacts( + ruleContext, + /* inputs = */ parsed.getResources(), + /* outputs = */ ImmutableList.of(parsed.getSymbols())); + + // Since there was no data binding, the compile action should just take in resources and output + // compiled symbols. + assertActionArtifacts( + ruleContext, + /* inputs = */ parsed.getResources(), + /* outputs = */ ImmutableList.of(parsed.getCompiledSymbols())); + } + + @Test + public void testParseWithDataBinding() throws Exception { + mockAndroidSdkWithAapt2(); + useConfiguration("--android_sdk=//sdk:sdk", "--android_aapt=aapt2"); + + RuleContext ruleContext = getRuleContext(/* useDataBinding = */ true); + + ParsedAndroidResources parsed = assertParse(ruleContext); + + // The parse action should take resources and busybox artifacts in and output symbols + assertActionArtifacts( + ruleContext, + /* inputs = */ parsed.getResources(), + /* outputs = */ ImmutableList.of(parsed.getSymbols())); + + // The compile action should take in resources and manifest in and output compiled symbols and + // an unused data binding zip. + assertActionArtifacts( + ruleContext, + /* inputs = */ ImmutableList.builder() + .addAll(parsed.getResources()) + .add(getManifest()) + .build(), + /* outputs = */ ImmutableList.of( + parsed.getCompiledSymbols(), + DataBinding.getSuffixedInfoFile(ruleContext, "_unused"))); + } + + /** + * Assets that the action used to generate the given outputs has the expected inputs and outputs. + */ + private void assertActionArtifacts( + RuleContext ruleContext, ImmutableList inputs, ImmutableList outputs) { + // Actions must have at least one output + assertThat(outputs).isNotEmpty(); + + // Get the action from one of the outputs + ActionAnalysisMetadata action = + ruleContext.getAnalysisEnvironment().getLocalGeneratingAction(outputs.get(0)); + assertThat(action).isNotNull(); + + assertThat(removeToolingArtifacts(action.getInputs())).containsExactlyElementsIn(inputs); + + assertThat(action.getOutputs()).containsExactlyElementsIn(outputs); + } + + /** Remove busybox and aapt2 tooling artifacts from a list of action inputs */ + private Iterable removeToolingArtifacts(Iterable inputArtifacts) { + return Streams.stream(inputArtifacts) + .filter( + artifact -> + // Not a known tool + !TOOL_FILENAMES.contains(artifact.getFilename()) + // Not one of the various busybox tools (we get different ones on different OSs) + && !artifact.getFilename().contains("busybox") + // Not a params file + && !artifact.getFilename().endsWith(".params")) + .collect(Collectors.toList()); + } + + /** + * Validates that a parse action was invoked correctly. Returns the {@link ParsedAndroidResources} + * for further validation. + */ + private ParsedAndroidResources assertParse(RuleContext ruleContext) throws Exception { + ImmutableList resources = getResources("values-en/foo.xml", "drawable-hdpi/bar.png"); + AndroidResources raw = + new AndroidResources( + resources, AndroidResources.getResourceRoots(ruleContext, resources, "resource_files")); + StampedAndroidManifest manifest = + new StampedAndroidManifest(ruleContext, getManifest(), "some.java.pkg", false); + + ParsedAndroidResources parsed = raw.parse(ruleContext, manifest); + + // Inherited values should be equal + assertThat(raw).isEqualTo(parsed); + + // Label should be set from RuleContext + assertThat(parsed.getLabel()).isEqualTo(ruleContext.getLabel()); + + return parsed; + } + + private Artifact getManifest() { + return getResource("some/path/AndroidManifest.xml"); + } + + /** Gets a dummy rule context object by creating a dummy target. */ + private RuleContext getRuleContext(boolean useDataBinding) throws Exception { + ConfiguredTarget target = + scratchConfiguredTarget( + "java/foo", + "target", + "android_library(name = 'target',", + useDataBinding ? " enable_data_binding = True" : "", + ")"); + RuleContext dummy = getRuleContext(target); + + ExtendedEventHandler eventHandler = new StoredEventHandler(); + assertThat(targetConfig.isActionsEnabled()).isTrue(); + return view.getRuleContextForTesting( + eventHandler, + target, + new CachingAnalysisEnvironment( + view.getArtifactFactory(), + skyframeExecutor.getActionKeyContext(), + ConfiguredTargetKey.of(target.getLabel(), targetConfig), + /*isSystemEnv=*/ false, + targetConfig.extendedSanityChecks(), + eventHandler, + /*env=*/ null, + targetConfig.isActionsEnabled()), + new BuildConfigurationCollection( + ImmutableList.of(dummy.getConfiguration()), dummy.getHostConfiguration())); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD index f95862e4ba..6eab50b80f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD @@ -109,6 +109,8 @@ java_test( deps = [ ":ResourceTestBase", "//src/main/java/com/google/devtools/build/lib:android-rules", + "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:packages-internal", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -122,6 +124,7 @@ java_library( name = "ResourceTestBase", srcs = ["ResourceTestBase.java"], deps = [ + ":AndroidBuildViewTestCase", "//src/main/java/com/google/devtools/build/lib:packages-internal", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java index 9ea174a8ad..e7fd7d4ad3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java @@ -37,7 +37,7 @@ import org.junit.After; import org.junit.Before; /** Base class for tests that work with resource artifacts. */ -public abstract class ResourceTestBase { +public abstract class ResourceTestBase extends AndroidBuildViewTestCase { public static final String RESOURCE_ROOT = "java/android/res"; private static final ArtifactOwner OWNER = () -> { -- cgit v1.2.3