From 33cd68e18f554b98194b4ce924580d3333ab9217 Mon Sep 17 00:00:00 2001 From: laszlocsomor Date: Tue, 18 Jul 2017 09:21:33 +0200 Subject: Automated rollback of commit a76c94be7c56b93fc5a2f9ececfba7ac1f61f69c. *** Reason for rollback *** Caused memory regression. *** Original change description *** CustomCommandLine.Builder: clean up its interface In this commit: - remove unused methods and classes - turn CustomCommandLine.ArgvFragment into an interface - remove the CustomCommandLine.TreeFileArtifactArgvFragment abstract class; it only had one remaining subclass - add @Nullable annotations where nulls are fine - add Precondition checks for non-nullable args - simplify the interface by removing add* methods that can be composed of other add* methods; this makes it easier to see... *** RELNOTES: none PiperOrigin-RevId: 162320031 --- .../build/lib/actions/CustomCommandLineTest.java | 174 ++++++++------------- 1 file changed, 65 insertions(+), 109 deletions(-) (limited to 'src/test/java/com/google/devtools/build/lib/actions') diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java index f7995b42d3..ea061beb2d 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java @@ -18,7 +18,6 @@ import static org.junit.Assert.fail; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; -import com.google.common.testing.NullPointerTester; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; @@ -33,6 +32,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -71,18 +71,15 @@ public class CustomCommandLineTest { @Test public void testStringsArgs() { - CustomCommandLine cl = - CustomCommandLine.builder().add("--arg").add(ImmutableList.of("a", "b")).build(); + CustomCommandLine cl = CustomCommandLine.builder().add("--arg", + ImmutableList.of("a", "b")).build(); assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--arg", "a", "b")); } @Test public void testArtifactJoinStringArgs() { - CustomCommandLine cl = - CustomCommandLine.builder() - .add("--path") - .addJoinStrings(":", ImmutableList.of("foo", "bar")) - .build(); + CustomCommandLine cl = CustomCommandLine.builder().addJoinStrings("--path", ":", + ImmutableList.of("foo", "bar")).build(); assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "foo:bar")); } @@ -90,8 +87,17 @@ public class CustomCommandLineTest { public void testJoinValues() { CustomCommandLine cl = CustomCommandLine.builder() - .add("--path") - .addJoinValues(":", ImmutableList.of("foo", "bar", "baz"), String::toUpperCase) + .addJoinValues( + "--path", + ":", + ImmutableList.of("foo", "bar", "baz"), + new Function() { + @Nullable + @Override + public String apply(@Nullable String s) { + return s.toUpperCase(); + } + }) .build(); assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "FOO:BAR:BAZ")); } @@ -104,11 +110,8 @@ public class CustomCommandLineTest { @Test public void testArtifactExecPathsArgs() { - CustomCommandLine cl = - CustomCommandLine.builder() - .add("--path") - .addExecPaths(ImmutableList.of(artifact1, artifact2)) - .build(); + CustomCommandLine cl = CustomCommandLine.builder().addExecPaths("--path", + ImmutableList.of(artifact1, artifact2)).build(); assertThat(cl.arguments()) .isEqualTo(ImmutableList.of("--path", "dir/file1.txt", "dir/file2.txt")); } @@ -122,11 +125,8 @@ public class CustomCommandLineTest { @Test public void testArtifactJoinExecPathArgs() { - CustomCommandLine cl = - CustomCommandLine.builder() - .add("--path") - .addJoinExecPaths(":", ImmutableList.of(artifact1, artifact2)) - .build(); + CustomCommandLine cl = CustomCommandLine.builder().addJoinExecPaths("--path", ":", + ImmutableList.of(artifact1, artifact2)).build(); assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "dir/file1.txt:dir/file2.txt")); } @@ -136,6 +136,13 @@ public class CustomCommandLineTest { assertThat(cl.arguments()).isEqualTo(ImmutableList.of("dir/file1.txt")); } + @Test + public void testJoinPathArgs() { + CustomCommandLine cl = CustomCommandLine.builder().addJoinPaths(":", + ImmutableList.of(artifact1.getExecPath(), artifact2.getExecPath())).build(); + assertThat(cl.arguments()).isEqualTo(ImmutableList.of("dir/file1.txt:dir/file2.txt")); + } + @Test public void testPathsArgs() { CustomCommandLine cl = CustomCommandLine.builder().addPaths("%s:%s", @@ -167,15 +174,12 @@ public class CustomCommandLineTest { @Test public void testCombinedArgs() { - CustomCommandLine cl = - CustomCommandLine.builder() - .add("--arg") - .add("--args") - .add(ImmutableList.of("abc")) - .add("--path1") - .addExecPaths(ImmutableList.of(artifact1)) - .addExecPath("--path2", artifact2) - .build(); + CustomCommandLine cl = CustomCommandLine.builder() + .add("--arg") + .add("--args", ImmutableList.of("abc")) + .addExecPaths("--path1", ImmutableList.of(artifact1)) + .addExecPath("--path2", artifact2) + .build(); assertThat(cl.arguments()) .isEqualTo( ImmutableList.of( @@ -183,73 +187,13 @@ public class CustomCommandLineTest { } @Test - public void testAddNulls() throws Exception { - Artifact treeArtifact = createTreeArtifact("myTreeArtifact"); - assertThat(treeArtifact).isNotNull(); - - CustomCommandLine cl = - CustomCommandLine.builder() - .add((CharSequence) null) - .add((Label) null) - .add((Iterable) null) - .add(ImmutableList.of()) - .addExecPaths(null) - .addExecPaths(ImmutableList.of()) - .addExecPath("foo", null) - .addPlaceholderTreeArtifactExecPath(null) - .addJoinStrings("foo", null) - .addJoinStrings("foo", ImmutableList.of()) - .addJoinValues("foo", null, String::toString) - .addJoinValues("foo", ImmutableList.of(), String::toString) - .addJoinExecPaths("foo", null) - .addJoinExecPaths("foo", ImmutableList.of()) - .addPath(null) - .addPaths("foo") - .addPaths("foo", (PathFragment[]) null) - .addPaths("foo", new PathFragment[0]) - .addBeforeEachPath("foo", null) - .addBeforeEachPath("foo", ImmutableList.of()) - .addBeforeEach("foo", null) - .addBeforeEach("foo", ImmutableList.of()) - .addBeforeEachExecPath("foo", null) - .addBeforeEachExecPath("foo", ImmutableList.of()) - .addFormatEach("%s", null) - .addFormatEach("%s", ImmutableList.of()) - .add((CustomArgv) null) - .add((CustomMultiArgv) null) - .build(); + public void testAddNulls() { + CustomCommandLine cl = CustomCommandLine.builder() + .add("--args", null) + .addExecPaths(null, ImmutableList.of(artifact1)) + .addExecPath(null, null) + .build(); assertThat(cl.arguments()).isEqualTo(ImmutableList.of()); - - CustomCommandLine.Builder obj = CustomCommandLine.builder(); - Class clazz = CustomCommandLine.Builder.class; - NullPointerTester npt = - new NullPointerTester() - .setDefault(Artifact.class, artifact1) - .setDefault(String.class, "foo") - .setDefault(PathFragment[].class, new PathFragment[] {PathFragment.create("foo")}); - - npt.testMethod(obj, clazz.getMethod("addExecPath", String.class, Artifact.class)); - npt.testMethod(obj, clazz.getMethod("addParamFile", String.class, Artifact.class)); - npt.testMethod(obj, clazz.getMethod("addPaths", String.class, PathFragment[].class)); - - npt.setDefault(Iterable.class, ImmutableList.of("foo")); - npt.testMethod(obj, clazz.getMethod("addJoinStrings", String.class, Iterable.class)); - npt.testMethod( - obj, clazz.getMethod("addJoinValues", String.class, Iterable.class, Function.class)); - npt.testMethod(obj, clazz.getMethod("addBeforeEach", String.class, Iterable.class)); - npt.testMethod(obj, clazz.getMethod("addFormatEach", String.class, Iterable.class)); - - npt.setDefault(Iterable.class, ImmutableList.of(artifact1)); - npt.testMethod(obj, clazz.getMethod("addJoinExecPaths", String.class, Iterable.class)); - npt.testMethod(obj, clazz.getMethod("addBeforeEachExecPath", String.class, Iterable.class)); - - npt.setDefault(Iterable.class, ImmutableList.of(PathFragment.create("foo"))); - npt.testMethod(obj, clazz.getMethod("addBeforeEachPath", String.class, Iterable.class)); - - npt.setDefault(Artifact.class, treeArtifact); - npt.testMethod( - obj, clazz.getMethod("addJoinExpandedTreeArtifactExecPath", String.class, Artifact.class)); - npt.testMethod(obj, clazz.getMethod("addExpandedTreeArtifactExecPaths", Artifact.class)); } @Test @@ -257,13 +201,10 @@ public class CustomCommandLineTest { Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1"); Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2"); - CustomCommandLine commandLineTemplate = - CustomCommandLine.builder() - .add("--argOne") - .addPlaceholderTreeArtifactExecPath(treeArtifactOne) - .add("--argTwo") - .addPlaceholderTreeArtifactExecPath(treeArtifactTwo) - .build(); + CustomCommandLine commandLineTemplate = CustomCommandLine.builder() + .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne) + .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo) + .build(); TreeFileArtifact treeFileArtifactOne = createTreeFileArtifact( treeArtifactOne, "children/child1"); @@ -282,18 +223,33 @@ public class CustomCommandLineTest { .inOrder(); } + @Test + public void testTreeFileArtifactExecPathWithTemplateArgs() { + Artifact treeArtifact = createTreeArtifact("myArtifact/treeArtifact1"); + + CustomCommandLine commandLineTemplate = CustomCommandLine.builder() + .addPlaceholderTreeArtifactFormattedExecPath("path:%s", treeArtifact) + .build(); + + TreeFileArtifact treeFileArtifact = createTreeFileArtifact( + treeArtifact, "children/child1"); + + CustomCommandLine commandLine = commandLineTemplate.evaluateTreeFileArtifacts( + ImmutableList.of(treeFileArtifact)); + + assertThat(commandLine.arguments()).containsExactly( + "path:myArtifact/treeArtifact1/children/child1"); + } + @Test public void testTreeFileArtifactArgThrowWithoutSubstitution() { Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1"); Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2"); - CustomCommandLine commandLineTemplate = - CustomCommandLine.builder() - .add("--argOne") - .addPlaceholderTreeArtifactExecPath(treeArtifactOne) - .add("--argTwo") - .addPlaceholderTreeArtifactExecPath(treeArtifactTwo) - .build(); + CustomCommandLine commandLineTemplate = CustomCommandLine.builder() + .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne) + .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo) + .build(); try { commandLineTemplate.arguments(); -- cgit v1.2.3