diff options
author | laszlocsomor <laszlocsomor@google.com> | 2017-07-18 09:21:33 +0200 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2017-07-18 09:49:23 +0200 |
commit | 33cd68e18f554b98194b4ce924580d3333ab9217 (patch) | |
tree | 2b7ed87b0543fc5591bc9e74ed7e46d72e74007f /src/test/java/com/google/devtools/build/lib | |
parent | c238b573aef850ecc4c2a7bdb49233de225d6b12 (diff) |
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
Diffstat (limited to 'src/test/java/com/google/devtools/build/lib')
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java | 174 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java | 4 |
2 files changed, 66 insertions, 112 deletions
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<String, String>() { + @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")); } @@ -137,6 +137,13 @@ public class CustomCommandLineTest { } @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", artifact1.getExecPath(), artifact1.getRootRelativePath()).build(); @@ -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<String>) null) - .add(ImmutableList.<String>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<CustomCommandLine.Builder> 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"); @@ -283,17 +224,32 @@ public class CustomCommandLineTest { } @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(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java index 6e5de2fce5..8711b47a4b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java @@ -132,9 +132,7 @@ public class ParamFileWriteActionTest extends BuildViewTestCase { return CustomCommandLine.builder() .add("--flag1") .add("--flag2") - .add("--flag3") - .add("value1") - .add("value2") + .add("--flag3", ImmutableList.of("value1", "value2")) .build(); } |