From 3948159e71ac65a0b1195c33274eee2bedf89284 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 22 Nov 2017 08:19:26 -0800 Subject: Add proper test coverage for filtering resource deps RELNOTES: none PiperOrigin-RevId: 176659117 --- .../rules/android/ResourceFilterFactoryTest.java | 101 +++++++++++++-------- 1 file changed, 62 insertions(+), 39 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java index 42a8c213ac..6a5c5d5ccb 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -32,7 +31,6 @@ import com.google.devtools.build.lib.rules.android.ResourceFilterFactory.AddDyna import com.google.devtools.build.lib.rules.android.ResourceFilterFactory.FilterBehavior; import com.google.devtools.build.lib.testutil.FakeAttributeMapper; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,7 +43,7 @@ import org.junit.runners.JUnit4; public class ResourceFilterFactoryTest extends ResourceTestBase { private NestedSet getResourceContainers(ImmutableList... resources) - throws LabelSyntaxException { + throws Exception { NestedSetBuilder builder = NestedSetBuilder.naiveLinkOrder(); for (ImmutableList resourceList : resources) { builder.add(getResourceContainer(resourceList)); @@ -54,7 +52,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { } private ResourceContainer getResourceContainer(ImmutableList resources) - throws LabelSyntaxException { + throws Exception { // Get dummy objects for irrelevant values required by the builder. Artifact manifest = getResource("manifest"); @@ -67,6 +65,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { return ResourceContainer.builder() .setResources(resources) + .setResourcesRoots( + LocalResourceContainer.getResourceRoots(errorConsumer, resources, "resource_files")) .setLabel(label) .setManifestExported(false) .setManifest(manifest) @@ -417,31 +417,38 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { @Test public void testFilterLocalAndTransitive() throws Exception { - Artifact localResourceToKeep = getResource("drawable-en-hdpi/foo.png"); - Artifact localResourceToDiscard = getResource("drawable-en-ldpi/foo.png"); + Artifact localResourceToKeep = getResource("drawable-en-hdpi/local.png"); + Artifact localResourceToDiscard = getResource("drawable-en-ldpi/local.png"); // These resources go in different ResourceContainers to ensure we are filter across all // resources. - Artifact transitiveResourceToKeep = getResource("transitive/drawable-en-hdpi/foo.png"); - Artifact transitiveResourceToDiscard = getResource("transitive/drawable-en-ldpi/foo.png"); + Artifact directResourceToKeep = getResource("direct/drawable-en-hdpi/direct.png"); + Artifact directResourceToDiscard = getResource("direct/drawable-en-ldpi/direct.png"); + Artifact transitiveResourceToKeep = getResource("transitive/drawable-en-hdpi/transitive.png"); + Artifact transitiveResourceToDiscard = + getResource("transitive/drawable-en-ldpi/transitive.png"); LocalResourceContainer localResources = LocalResourceContainer.forResources( errorConsumer, ImmutableList.of(localResourceToKeep, localResourceToDiscard)); - NestedSet containers = - getResourceContainers( - ImmutableList.of(transitiveResourceToDiscard), - ImmutableList.of(transitiveResourceToKeep)); - ResourceDependencies resourceDependencies = ResourceDependencies.empty() .withResources( - containers, - containers, + getResourceContainers( + ImmutableList.of(transitiveResourceToDiscard), + ImmutableList.of(transitiveResourceToKeep)), + getResourceContainers( + ImmutableList.of(directResourceToDiscard), + ImmutableList.of(directResourceToKeep)), new NestedSetBuilder(Order.NAIVE_LINK_ORDER) - .add(transitiveResourceToDiscard) - .add(transitiveResourceToKeep) + .add(directResourceToDiscard) + .add(directResourceToKeep) + .addTransitive( + NestedSetBuilder.create( + Order.NAIVE_LINK_ORDER, + transitiveResourceToDiscard, + transitiveResourceToKeep)) .build()); ResourceFilterFactory resourceFilterFactory = @@ -455,35 +462,51 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { ResourceDependencies filteredResourceDeps = resourceDependencies.filter(filter); - assertThat(filteredResourceDeps.getTransitiveResources()) - .containsExactly(transitiveResourceToKeep); + // TODO: Remove - assert was same order before + assertThat(resourceDependencies.getTransitiveResources()) + .containsAllOf(directResourceToKeep, transitiveResourceToKeep) + .inOrder(); - for (NestedSet returnedContainers : - ImmutableList.of( - filteredResourceDeps.getTransitiveResourceContainers(), - filteredResourceDeps.getDirectResourceContainers())) { + assertThat(filteredResourceDeps.getTransitiveResources()) + .containsExactly(directResourceToKeep, transitiveResourceToKeep) + .inOrder(); - assertThat(returnedContainers).hasSize(2); + List directContainers = + filteredResourceDeps.getDirectResourceContainers().toList(); + assertThat(directContainers).hasSize(2); - Iterator transitiveContainers = returnedContainers.iterator(); + ResourceContainer directToDiscard = directContainers.get(0); + assertThat(directToDiscard.getResources()).isEmpty(); + assertThat(directToDiscard.getResourcesRoots()).isEmpty(); - // The first transitive resource container was the one with the resource to discard - assertThat(transitiveContainers.hasNext()).isTrue(); - assertThat(transitiveContainers.next().getResources()).isEmpty(); + ResourceContainer directToKeep = directContainers.get(1); + assertThat(directToKeep.getResources()).containsExactly(directResourceToKeep); + assertThat(directToKeep.getResourcesRoots()) + .containsExactly( + directResourceToKeep.getExecPath().getParentDirectory().getParentDirectory()); - // The second transitive resource container was the one with the resource to keep - assertThat(transitiveContainers.hasNext()).isTrue(); - assertThat(transitiveContainers.next().getResources()) - .containsExactly(transitiveResourceToKeep); + List transitiveContainers = + filteredResourceDeps.getTransitiveResourceContainers().toList(); + assertThat(transitiveContainers).hasSize(2); - // No resource containers were added. - assertThat(transitiveContainers.hasNext()).isFalse(); + ResourceContainer transitiveToDiscard = transitiveContainers.get(0); + assertThat(transitiveToDiscard.getResources()).isEmpty(); + assertThat(transitiveToDiscard.getResourcesRoots()).isEmpty(); - // We tell the resource processing actions to ignore references to filtered resources from - // dependencies. - assertThat(resourceFilterFactory.getResourcesToIgnoreInExecution()) - .containsExactly("drawable-en-ldpi/foo.png"); - } + ResourceContainer transitiveToKeep = transitiveContainers.get(1); + assertThat(transitiveToKeep.getResources()) + .containsExactly(transitiveResourceToKeep); + assertThat(transitiveToKeep.getResourcesRoots()) + .containsExactly( + transitiveResourceToKeep + .getExecPath() + .getParentDirectory() + .getParentDirectory()); + + // We tell the resource processing actions to ignore references to filtered resources from + // dependencies. + assertThat(resourceFilterFactory.getResourcesToIgnoreInExecution()) + .containsExactly("drawable-en-ldpi/direct.png", "drawable-en-ldpi/transitive.png"); } @Test -- cgit v1.2.3