diff options
Diffstat (limited to 'src')
6 files changed, 223 insertions, 109 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java index c318413c05..1916b089c9 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -90,14 +91,23 @@ public abstract class FileWriteActionTestCase extends BuildViewTestCase { assertTrue(output.isExecutable()); } + private enum KeyAttributes { + DATA, + MAKE_EXECUTABLE + } + protected void checkComputesConsistentKeys() throws Exception { - ActionTester.runTest(4, new ActionTester.ActionCombinationFactory() { - @Override - public Action generate(int i) { - return createAction(NULL_ACTION_OWNER, outputArtifact, - (i & 1) == 0 ? "0" : "1", - (i & 2) == 0); - } - }); + ActionTester.runTest( + KeyAttributes.class, + new ActionTester.ActionCombinationFactory<KeyAttributes>() { + @Override + public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) { + return createAction( + NULL_ACTION_OWNER, + outputArtifact, + attributesToFlip.contains(KeyAttributes.DATA) ? "0" : "1", + attributesToFlip.contains(KeyAttributes.MAKE_EXECUTABLE)); + } + }); } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java index 97eba84d26..b21470127b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java @@ -19,6 +19,7 @@ import static org.mockito.Mockito.mock; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; @@ -250,6 +251,13 @@ public class PopulateTreeArtifactActionTest extends BuildViewTestCase { } } + private enum KeyAttributes { + ARCHIVE, + TREE_ARTIFACT, + ARCHIVE_MANIFEST, + ZIPPER + } + @Test public void testComputeKey() throws Exception { final Artifact archiveA = getSourceArtifact("myArchiveA.zip"); @@ -263,23 +271,32 @@ public class PopulateTreeArtifactActionTest extends BuildViewTestCase { final FilesToRunProvider zipperB = FilesToRunProvider.fromSingleExecutableArtifact( getSourceArtifact("unzipBinaryB")); - ActionTester.runTest(16, new ActionCombinationFactory() { - @Override - public Action generate(int i) { - Artifact archive = (i & 1) == 0 ? archiveA : archiveB; - Artifact treeArtifactToPopulate = (i & 2) == 0 - ? treeArtifactToPopulateA : treeArtifactToPopulateB; - Artifact archiveManifest = (i & 4) == 0 ? archiveManifestA : archiveManifestB; - FilesToRunProvider zipper = (i & 8) == 0 ? zipperA : zipperB; - - return new PopulateTreeArtifactAction( - ActionsTestUtil.NULL_ACTION_OWNER, - archive, - archiveManifest, - treeArtifactToPopulate, - zipper); - } - }); + ActionTester.runTest( + KeyAttributes.class, + new ActionCombinationFactory<KeyAttributes>() { + @Override + public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) { + Artifact archive = + attributesToFlip.contains(KeyAttributes.ARCHIVE) ? archiveA : archiveB; + Artifact treeArtifactToPopulate = + attributesToFlip.contains(KeyAttributes.TREE_ARTIFACT) + ? treeArtifactToPopulateA + : treeArtifactToPopulateB; + Artifact archiveManifest = + attributesToFlip.contains(KeyAttributes.ARCHIVE_MANIFEST) + ? archiveManifestA + : archiveManifestB; + FilesToRunProvider zipper = + attributesToFlip.contains(KeyAttributes.ZIPPER) ? zipperA : zipperB; + + return new PopulateTreeArtifactAction( + ActionsTestUtil.NULL_ACTION_OWNER, + archive, + archiveManifest, + treeArtifactToPopulate, + zipper); + } + }); } private PopulateTreeArtifactAction createPopulateTreeArtifactAction() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java index 8629684b75..5b8ba357a0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.AbstractAction; @@ -364,50 +365,66 @@ public class SpawnActionTest extends BuildViewTestCase { assertThat(inputFiles).isEmpty(); } + private enum KeyAttributes { + EXECUTABLE_PATH, + EXECUTABLE, + MNEMONIC, + RUNFILES_SUPPLIER, + RUNFILES_SUPPLIER_PATH, + ENVIRONMENT + } + @Test public void testComputeKey() throws Exception { final Artifact artifactA = getSourceArtifact("a"); final Artifact artifactB = getSourceArtifact("b"); - ActionTester.runTest(64, new ActionCombinationFactory() { - @Override - public Action generate(int i) { - SpawnAction.Builder builder = builder(); - builder.addOutput(destinationArtifact); - - PathFragment executable = (i & 1) == 0 ? artifactA.getExecPath() : artifactB.getExecPath(); - if ((i & 2) == 0) { - builder.setExecutable(executable); - } else { - builder.setJavaExecutable(executable, jarArtifact, "Main", ImmutableList.<String>of()); - } - - builder.setMnemonic((i & 4) == 0 ? "a" : "b"); - - if ((i & 8) == 0) { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("a"))); - } else { - builder.addRunfilesSupplier(runfilesSupplier(artifactB, PathFragment.create("a"))); - } - - if ((i & 16) == 0) { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("aa"))); - } else { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("ab"))); - } - - Map<String, String> env = new HashMap<>(); - if ((i & 32) == 0) { - env.put("foo", "bar"); - } - builder.setEnvironment(env); - - Action[] actions = builder.build(ActionsTestUtil.NULL_ACTION_OWNER, - collectingAnalysisEnvironment, targetConfig); - collectingAnalysisEnvironment.registerAction(actions); - return actions[0]; - } - }); + ActionTester.runTest( + KeyAttributes.class, + new ActionCombinationFactory<KeyAttributes>() { + @Override + public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) { + SpawnAction.Builder builder = builder(); + builder.addOutput(destinationArtifact); + + PathFragment executable = + attributesToFlip.contains(KeyAttributes.EXECUTABLE_PATH) + ? artifactA.getExecPath() + : artifactB.getExecPath(); + if (attributesToFlip.contains(KeyAttributes.EXECUTABLE)) { + builder.setExecutable(executable); + } else { + builder.setJavaExecutable( + executable, jarArtifact, "Main", ImmutableList.<String>of()); + } + + builder.setMnemonic(attributesToFlip.contains(KeyAttributes.MNEMONIC) ? "a" : "b"); + + if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER)) { + builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("a"))); + } else { + builder.addRunfilesSupplier(runfilesSupplier(artifactB, PathFragment.create("a"))); + } + + if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER_PATH)) { + builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("aa"))); + } else { + builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("ab"))); + } + + Map<String, String> env = new HashMap<>(); + if (attributesToFlip.contains(KeyAttributes.ENVIRONMENT)) { + env.put("foo", "bar"); + } + builder.setEnvironment(env); + + Action[] actions = + builder.build( + ActionsTestUtil.NULL_ACTION_OWNER, collectingAnalysisEnvironment, targetConfig); + collectingAnalysisEnvironment.registerAction(actions); + return actions[0]; + } + }); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java index 58eab302a4..482e933f09 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java @@ -16,62 +16,90 @@ package com.google.devtools.build.lib.analysis.util; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Actions; +import com.google.devtools.build.lib.util.Preconditions; +import java.util.BitSet; +import java.util.EnumSet; /** * Test helper for testing {@link Action} implementations. */ public class ActionTester { - /** - * A generator for action instances. - */ - public interface ActionCombinationFactory { + /** A generator for action instances. */ + public interface ActionCombinationFactory<E extends Enum<E>> { /** - * Returns a new action instance. The parameter {@code i} is used to vary the parameters used to - * create the action. Implementations should do something like this: <code> + * Returns a new action instance. The parameter {@code attributesToFlip} is used to vary the + * parameters used to create the action. Implementations should do something like this: <code> * <pre> + * private enum KeyAttributes { ATTR_1, ATTR_2, ATTR_3, ATTR_4 } * return new MyAction(owner, inputs, outputs, configuration, - * (i & 1) == 0 ? a1 : a2, - * (i & 2) == 0 ? b1 : b2, - * (i & 4) == 0 ? c1 : c2); - * (i & 16) == 0 ? d1 : d2); + * attributesToFlip.contains(ATTR_0) ? a1 : a2, + * attributesToFlip.contains(ATTR_1) ? b1 : b2, + * attributesToFlip.contains(ATTR_2) ? c1 : c2, + * attributesToFlip.contains(ATTR_3) ? d1 : d2); * </pre> * </code> * - * <p>The wrap-around (in this case at 32) is intentional and is checked for by the testing - * method. - * * <p>To reduce the combinatorial complexity of testing an action class, all elements that are * only used to change the executed command line should go into a single parameter, and the key * computation should take the generated command line into account. * * <p>Furthermore, when called with identical parameters, this method should return different * instances (i.e. according to {@code ==}), but they should have the same key. + * + * @param attributesToFlip */ - Action generate(int i) throws InterruptedException; + Action generate(ImmutableSet<E> attributesToFlip) throws InterruptedException; } /** - * Tests that different actions have different keys. The count should specify how many different - * permutations the {@link ActionCombinationFactory} can generate. + * Tests that different actions have different keys. The attributeCount should specify how many + * different permutations the {@link ActionCombinationFactory} should generate. */ - public static void runTest(int count, ActionCombinationFactory factory) throws Exception { + public static <E extends Enum<E>> void runTest( + Class<E> attributeClass, ActionCombinationFactory<E> factory) throws Exception { + int attributesCount = attributeClass.getEnumConstants().length; + Preconditions.checkArgument( + attributesCount <= 30, + "Maximum attribute count is 30, more will overflow the max array size."); + int count = (int) Math.pow(2, attributesCount); Action[] actions = new Action[count]; for (int i = 0; i < actions.length; i++) { - actions[i] = factory.generate(i); + actions[i] = factory.generate(makeEnumSetInitializedTo(attributeClass, i)); } // Sanity check that the count is correct. - assertThat(Actions.canBeShared(actions[0], factory.generate(count))).isTrue(); + assertThat( + Actions.canBeShared( + actions[0], factory.generate(makeEnumSetInitializedTo(attributeClass, count)))) + .isTrue(); for (int i = 0; i < actions.length; i++) { - assertThat(Actions.canBeShared(actions[i], factory.generate(i))).isTrue(); + assertThat( + Actions.canBeShared( + actions[i], factory.generate(makeEnumSetInitializedTo(attributeClass, i)))) + .isTrue(); for (int j = i + 1; j < actions.length; j++) { assertWithMessage(i + " and " + j).that(Actions.canBeShared(actions[i], actions[j])) .isFalse(); } } } + + private static <E extends Enum<E>> ImmutableSet<E> makeEnumSetInitializedTo( + Class<E> attributeClass, int seed) { + EnumSet<E> result = EnumSet.<E>noneOf(attributeClass); + BitSet b = BitSet.valueOf(new long[] {seed}); + E[] attributes = attributeClass.getEnumConstants(); + for (int i = 0; i < attributes.length; i++) { + if (b.get(i)) { + result.add(attributes[i]); + } + } + return Sets.immutableEnumSet(result); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index 546f78fa66..a5358344ce 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -246,6 +246,15 @@ public class CppLinkActionTest extends BuildViewTestCase { assertThat(linkAction.getEnvironment()).containsEntry("foo", "bar"); } + private enum NonStaticAttributes { + OUTPUT_FILE, + COMPILATION_INPUTS, + NATIVE_DEPS, + USE_TEST_ONLY_FLAGS, + FAKE, + RUNTIME_SOLIB_DIR + } + /** * This mainly checks that non-static links don't have identical keys. Many options are only * allowed on non-static links, and we test several of them here. @@ -262,30 +271,39 @@ public class CppLinkActionTest extends BuildViewTestCase { final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); ActionTester.runTest( - 64, - new ActionCombinationFactory() { + NonStaticAttributes.class, + new ActionCombinationFactory<NonStaticAttributes>() { @Override - public Action generate(int i) throws InterruptedException { + public Action generate(ImmutableSet<NonStaticAttributes> attributesToFlip) + throws InterruptedException { CppLinkActionBuilder builder = new CppLinkActionBuilder( ruleContext, - (i & 2) == 0 ? dynamicOutputFile : staticOutputFile, + attributesToFlip.contains(NonStaticAttributes.OUTPUT_FILE) + ? dynamicOutputFile + : staticOutputFile, CppHelper.getToolchain(ruleContext, ":cc_toolchain"), CppHelper.getFdoSupport(ruleContext, ":cc_toolchain")) {}; builder.addCompilationInputs( - (i & 1) == 0 ? ImmutableList.of(oFile) : ImmutableList.of(oFile2)); - if ((i & 2) == 0) { + attributesToFlip.contains(NonStaticAttributes.COMPILATION_INPUTS) + ? ImmutableList.of(oFile) + : ImmutableList.of(oFile2)); + if (attributesToFlip.contains(NonStaticAttributes.OUTPUT_FILE)) { builder.setLinkType(LinkTargetType.DYNAMIC_LIBRARY); builder.setLibraryIdentifier("foo"); } else { builder.setLinkType(LinkTargetType.EXECUTABLE); } builder.setLinkStaticness(LinkStaticness.DYNAMIC); - builder.setNativeDeps((i & 4) == 0); - builder.setUseTestOnlyFlags((i & 8) == 0); - builder.setFake((i & 16) == 0); - builder.setRuntimeSolibDir((i & 32) == 0 ? null : PathFragment.create("so1")); + builder.setNativeDeps(attributesToFlip.contains(NonStaticAttributes.NATIVE_DEPS)); + builder.setUseTestOnlyFlags( + attributesToFlip.contains(NonStaticAttributes.USE_TEST_ONLY_FLAGS)); + builder.setFake(attributesToFlip.contains(NonStaticAttributes.FAKE)); + builder.setRuntimeSolibDir( + attributesToFlip.contains(NonStaticAttributes.RUNTIME_SOLIB_DIR) + ? null + : PathFragment.create("so1")); builder.setFeatureConfiguration(featureConfiguration); return builder.build(); @@ -293,6 +311,11 @@ public class CppLinkActionTest extends BuildViewTestCase { }); } + private enum StaticKeyAttributes { + OUTPUT_FILE, + COMPILATION_INPUTS + } + /** * This mainly checks that static library links don't have identical keys, and it also compares * them with simple dynamic library links. @@ -309,21 +332,28 @@ public class CppLinkActionTest extends BuildViewTestCase { final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); ActionTester.runTest( - 4, - new ActionCombinationFactory() { + StaticKeyAttributes.class, + new ActionCombinationFactory<StaticKeyAttributes>() { @Override - public Action generate(int i) throws InterruptedException { + public Action generate(ImmutableSet<StaticKeyAttributes> attributes) + throws InterruptedException { CppLinkActionBuilder builder = new CppLinkActionBuilder( ruleContext, - (i & 2) == 0 ? staticOutputFile : dynamicOutputFile, + attributes.contains(StaticKeyAttributes.OUTPUT_FILE) + ? staticOutputFile + : dynamicOutputFile, CppHelper.getToolchain(ruleContext, ":cc_toolchain"), CppHelper.getFdoSupport(ruleContext, ":cc_toolchain")) {}; builder.addCompilationInputs( - (i & 1) == 0 ? ImmutableList.of(oFile) : ImmutableList.of(oFile2)); + attributes.contains(StaticKeyAttributes.COMPILATION_INPUTS) + ? ImmutableList.of(oFile) + : ImmutableList.of(oFile2)); builder.setLinkType( - (i & 2) == 0 ? LinkTargetType.STATIC_LIBRARY : LinkTargetType.DYNAMIC_LIBRARY); + attributes.contains(StaticKeyAttributes.OUTPUT_FILE) + ? LinkTargetType.STATIC_LIBRARY + : LinkTargetType.DYNAMIC_LIBRARY); builder.setLibraryIdentifier("foo"); builder.setFeatureConfiguration(featureConfiguration); return builder.build(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java index 70284d8003..d5f471e87b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -134,6 +135,15 @@ public class LTOBackendActionTest extends BuildViewTestCase { .containsExactly(bitcode1Artifact, bitcode2Artifact, index2Artifact); } + private enum KeyAttributes { + EXECUTABLE, + IMPORTS_INFO, + MNEMONIC, + RUNFILES_SUPPLIER, + INPUT, + ENVIRONMENT + } + @Test public void testComputeKey() throws Exception { final Artifact artifactA = getSourceArtifact("a"); @@ -142,26 +152,28 @@ public class LTOBackendActionTest extends BuildViewTestCase { final Artifact artifactBimports = getSourceArtifact("b.imports"); ActionTester.runTest( - 64, - new ActionCombinationFactory() { + KeyAttributes.class, + new ActionCombinationFactory<KeyAttributes>() { @Override - public Action generate(int i) { + public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) { LTOBackendAction.Builder builder = new LTOBackendAction.Builder(); builder.addOutput(destinationArtifact); PathFragment executable = - (i & 1) == 0 ? artifactA.getExecPath() : artifactB.getExecPath(); + attributesToFlip.contains(KeyAttributes.EXECUTABLE) + ? artifactA.getExecPath() + : artifactB.getExecPath(); builder.setExecutable(executable); - if ((i & 2) == 0) { + if (attributesToFlip.contains(KeyAttributes.IMPORTS_INFO)) { builder.addImportsInfo(new HashMap<PathFragment, Artifact>(), artifactAimports); } else { builder.addImportsInfo(new HashMap<PathFragment, Artifact>(), artifactBimports); } - builder.setMnemonic((i & 4) == 0 ? "a" : "b"); + builder.setMnemonic(attributesToFlip.contains(KeyAttributes.MNEMONIC) ? "a" : "b"); - if ((i & 8) == 0) { + if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER)) { builder.addRunfilesSupplier( new RunfilesSupplierImpl(PathFragment.create("a"), Runfiles.EMPTY, artifactA)); } else { @@ -169,14 +181,14 @@ public class LTOBackendActionTest extends BuildViewTestCase { new RunfilesSupplierImpl(PathFragment.create("a"), Runfiles.EMPTY, artifactB)); } - if ((i & 16) == 0) { + if (attributesToFlip.contains(KeyAttributes.INPUT)) { builder.addInput(artifactA); } else { builder.addInput(artifactB); } Map<String, String> env = new HashMap<>(); - if ((i & 32) == 0) { + if (attributesToFlip.contains(KeyAttributes.ENVIRONMENT)) { env.put("foo", "bar"); } builder.setEnvironment(env); |