diff options
author | 2017-10-26 10:27:00 +0200 | |
---|---|---|
committer | 2017-10-26 10:59:41 +0200 | |
commit | 8cb1d2fb460a9caf47df58d7ff051d31080a77cb (patch) | |
tree | dcca2e1b72d55de0f990c289176765af8b88192a /src/test/java/com/google/devtools | |
parent | e012dcff33c26f5054066d5b8d9c26d356d5a30b (diff) |
Automated rollback of commit ca77f608e486bf7aa762565d25bf7b9e30f2268c.
This also rolls back unknown commit.
*** Reason for rollback ***
Affected expand_location Skylark API semantics - it no longer accepts ${abc} or plain dollar signs, but complains.
*** Original change description ***
Extend TemplateExpander to handle $(func param) expansion
Rewrite the Expander to use the new functionality; also rewrite the Skylark
expand_location function to use it.
PiperOrigin-RevId: 173508888
Diffstat (limited to 'src/test/java/com/google/devtools')
3 files changed, 86 insertions, 147 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/stringtemplate/TemplateExpanderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/stringtemplate/TemplateExpanderTest.java index d2740b6465..fa61f091b3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/stringtemplate/TemplateExpanderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/stringtemplate/TemplateExpanderTest.java @@ -18,193 +18,132 @@ import static org.junit.Assert.fail; import java.util.HashMap; import java.util.Map; -import java.util.function.Function; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; /** - * Unit tests for the {@link TemplateExpander}. + * Unit tests for the {@link + * com.google.devtools.build.lib.analysis.stringtemplate.TemplateExpander}, which expands variable + * references of the form <code>"$x"</code> and <code>"$(foo)"</code> into their corresponding + * values. */ @RunWith(JUnit4.class) public class TemplateExpanderTest { - private static final class TemplateContextImpl implements TemplateContext { - private final Map<String, String> vars = new HashMap<>(); - private final Map<String, Function<String, String>> functions = new HashMap<>(); - - @Override - public String lookupVariable(String name) - throws ExpansionException { - // Not a Make variable. Let the shell handle the expansion. - if (name.startsWith("$")) { - return name; - } - if (!vars.containsKey(name)) { - throw new ExpansionException(String.format("$(%s) not defined", name)); - } - return vars.get(name); - } - @Override - public String lookupFunction(String name, String param) throws ExpansionException { - if (!functions.containsKey(name)) { - throw new ExpansionException(String.format("$(%s) not defined", name)); - } - return functions.get(name).apply(param); - } - } + private TemplateContext context; - private TemplateContextImpl context; + private Map<String, String> vars = new HashMap<>(); @Before public final void createContext() throws Exception { - context = new TemplateContextImpl(); + context = new TemplateContext() { + @Override + public String lookupVariable(String name) + throws ExpansionException { + // Not a Make variable. Let the shell handle the expansion. + if (name.startsWith("$")) { + return name; + } + if (!vars.containsKey(name)) { + throw new ExpansionException(String.format("$(%s) not defined", name)); + } + return vars.get(name); + } + }; + + vars.put("SRCS", "src1 src2"); } - private String expand(String value) throws ExpansionException { - return TemplateExpander.expand(value, context); + private void assertExpansionEquals(String expected, String cmd) + throws ExpansionException { + assertThat(TemplateExpander.expand(cmd, context)).isEqualTo(expected); } - private ExpansionException expansionFailure(String cmd) { + private void assertExpansionFails(String expectedErrorSuffix, String cmd) { try { - expand(cmd); + TemplateExpander.expand(cmd, context); fail("Expansion of " + cmd + " didn't fail as expected"); - throw new AssertionError(); - } catch (ExpansionException e) { - return e; + } catch (Exception e) { + assertThat(e).hasMessageThat().isEqualTo(expectedErrorSuffix); } } @Test - public void testVariableExpansion() throws Exception { - context.vars.put("SRCS", "src1 src2"); - context.vars.put("<", "src1"); - context.vars.put("OUTS", "out1 out2"); - context.vars.put("@", "out1"); - context.vars.put("^", "src1 src2 dep1 dep2"); - context.vars.put("@D", "outdir"); - context.vars.put("BINDIR", "bindir"); - - assertThat(expand("$(SRCS)")).isEqualTo("src1 src2"); - assertThat(expand("$<")).isEqualTo("src1"); - assertThat(expand("$(OUTS)")).isEqualTo("out1 out2"); - assertThat(expand("$(@)")).isEqualTo("out1"); - assertThat(expand("$@")).isEqualTo("out1"); - assertThat(expand("$@,")).isEqualTo("out1,"); - - assertThat(expand("$(SRCS) $(OUTS)")).isEqualTo("src1 src2 out1 out2"); - - assertThat(expand("cmd")).isEqualTo("cmd"); - assertThat(expand("cmd $(SRCS),")).isEqualTo("cmd src1 src2,"); - assertThat(expand("label1 $(SRCS),")).isEqualTo("label1 src1 src2,"); - assertThat(expand(":label1 $(SRCS),")).isEqualTo(":label1 src1 src2,"); - } - - @Test - public void testUndefinedVariableExpansion() throws Exception { - assertThat(expansionFailure("$(foo)")) - .hasMessageThat().isEqualTo("$(foo) not defined"); - } - - @Test - public void testFunctionExpansion() throws Exception { - context.functions.put("foo", (String p) -> "FOO(" + p + ")"); - context.vars.put("bar", "bar"); - - assertThat(expand("$(foo baz)")).isEqualTo("FOO(baz)"); - assertThat(expand("$(bar) $(foo baz)")).isEqualTo("bar FOO(baz)"); - assertThat(expand("xyz$(foo baz)zyx")).isEqualTo("xyzFOO(baz)zyx"); - } - - @Test - public void testFunctionExpansionThrows() throws Exception { - try { - TemplateExpander.expand("$(foo baz)", new TemplateContext() { - @Override - public String lookupVariable(String name) throws ExpansionException { - throw new ExpansionException(name); - } - - @Override - public String lookupFunction(String name, String param) throws ExpansionException { - throw new ExpansionException(name + "(" + param + ")"); - } - }); - fail(); - } catch (ExpansionException e) { - assertThat(e).hasMessageThat().isEqualTo("foo(baz)"); - } - } + public void testExpansion() throws Exception { + vars.put("<", "src1"); + vars.put("OUTS", "out1 out2"); + vars.put("@", "out1"); + vars.put("^", "src1 src2 dep1 dep2"); + vars.put("@D", "outdir"); + vars.put("BINDIR", "bindir"); + + assertExpansionEquals("src1 src2", "$(SRCS)"); + assertExpansionEquals("src1", "$<"); + assertExpansionEquals("out1 out2", "$(OUTS)"); + assertExpansionEquals("out1", "$(@)"); + assertExpansionEquals("out1", "$@"); + assertExpansionEquals("out1,", "$@,"); + + assertExpansionEquals("src1 src2 out1 out2", "$(SRCS) $(OUTS)"); + + assertExpansionEquals("cmd", "cmd"); + assertExpansionEquals("cmd src1 src2,", "cmd $(SRCS),"); + assertExpansionEquals("label1 src1 src2,", "label1 $(SRCS),"); + assertExpansionEquals(":label1 src1 src2,", ":label1 $(SRCS),"); - @Test - public void testUndefinedFunctionExpansion() throws Exception { // Note: $(location x) is considered an undefined variable; - assertThat(expansionFailure("$(location label1), $(SRCS),")) - .hasMessageThat().isEqualTo("$(location) not defined"); - assertThat(expansionFailure("$(basename file)")) - .hasMessageThat().isEqualTo("$(basename) not defined"); + assertExpansionFails("$(location label1) not defined", + "$(location label1), $(SRCS),"); } @Test public void testRecursiveExpansion() throws Exception { // Expansion is recursive: $(recursive) -> $(SRCS) -> "src1 src2" - context.vars.put("SRCS", "src1 src2"); - context.vars.put("recursive", "$(SRCS)"); - assertThat(expand("$(recursive)")).isEqualTo("src1 src2"); - } + vars.put("recursive", "$(SRCS)"); + assertExpansionEquals("src1 src2", "$(recursive)"); - @Test - public void testRecursiveExpansionDoesNotSpanExpansionBoundaries() throws Exception { // Recursion does not span expansion boundaries: // $(recur2a)$(recur2b) --> "$" + "(SRCS)" --/--> "src1 src2" - context.vars.put("SRCS", "src1 src2"); - context.vars.put("recur2a", "$$"); - context.vars.put("recur2b", "(SRCS)"); - assertThat(expand("$(recur2a)$(recur2b)")).isEqualTo("$(SRCS)"); + vars.put("recur2a", "$$"); + vars.put("recur2b", "(SRCS)"); + assertExpansionEquals("$(SRCS)", "$(recur2a)$(recur2b)"); } @Test - public void testSelfInfiniteExpansionFailsGracefully() throws Exception { - context.vars.put("infinite", "$(infinite)"); - assertThat(expansionFailure("$(infinite)")).hasMessageThat() - .isEqualTo("potentially unbounded recursion during expansion of '$(infinite)'"); - } + public void testInfiniteRecursionFailsGracefully() throws Exception { + vars.put("infinite", "$(infinite)"); + assertExpansionFails("potentially unbounded recursion during expansion " + + "of '$(infinite)'", + "$(infinite)"); - @Test - public void testMutuallyInfiniteExpansionFailsGracefully() throws Exception { - context.vars.put("black", "$(white)"); - context.vars.put("white", "$(black)"); - assertThat(expansionFailure("$(white) is the new $(black)")).hasMessageThat() - .isEqualTo("potentially unbounded recursion during expansion of '$(black)'"); + vars.put("black", "$(white)"); + vars.put("white", "$(black)"); + assertExpansionFails("potentially unbounded recursion during expansion " + + "of '$(black)'", + "$(white) is the new $(black)"); } @Test public void testErrors() throws Exception { - assertThat(expansionFailure("$(SRCS")).hasMessageThat() - .isEqualTo("unterminated variable reference"); - assertThat(expansionFailure("$")).hasMessageThat().isEqualTo("unterminated $"); + assertExpansionFails("unterminated variable reference", "$(SRCS"); + assertExpansionFails("unterminated $", "$"); String suffix = "instead for \"Make\" variables, or escape the '$' as '$$' if you intended " + "this for the shell"; - assertThat(expansionFailure("for file in a b c;do echo $file;done")).hasMessageThat() - .isEqualTo("'$file' syntax is not supported; use '$(file)' " + suffix); - assertThat(expansionFailure("${file%:.*8}")).hasMessageThat() - .isEqualTo("'${file%:.*8}' syntax is not supported; use '$(file%:.*8)' " + suffix); - } - - @Test - public void testDollarDollar() throws Exception { - assertThat(expand("for file in a b c;do echo $$file;done")) - .isEqualTo("for file in a b c;do echo $file;done"); - assertThat(expand("$${file%:.*8}")).isEqualTo("${file%:.*8}"); - assertThat(expand("$$(basename file)")).isEqualTo("$(basename file)"); + assertExpansionFails("'$file' syntax is not supported; use '$(file)' " + suffix, + "for file in a b c;do echo $file;done"); + assertExpansionFails("'${file%:.*8}' syntax is not supported; use '$(file%:.*8)' " + suffix, + "${file%:.*8}"); } - // Regression test: check that the parameter is trimmed before expanding. @Test - public void testFunctionExpansionIsTrimmed() throws Exception { - context.functions.put("foo", (String p) -> "FOO(" + p + ")"); - assertThat(expand("$(foo baz )")).isEqualTo("FOO(baz)"); + public void testShellVariables() throws Exception { + assertExpansionEquals("for file in a b c;do echo $file;done", + "for file in a b c;do echo $$file;done"); + assertExpansionEquals("${file%:.*8}", "$${file%:.*8}"); + assertExpansionFails("$(basename file) not defined", "$(basename file)"); + assertExpansionEquals("$(basename file)", "$$(basename file)"); } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java index 8b2b1d4eaa..c4c9a66978 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java @@ -105,10 +105,10 @@ public class GenRuleCommandSubstitutionTest extends BuildViewTestCase { assertExpansionFails("$(locationz) not defined", "//test"); genrule("$(locationz )"); - assertExpansionFails("$(locationz) not defined", "//test"); + assertExpansionFails("$(locationz ) not defined", "//test"); genrule("$(locationz foo )"); - assertExpansionFails("$(locationz) not defined", "//test"); + assertExpansionFails("$(locationz foo ) not defined", "//test"); } @Test @@ -233,10 +233,10 @@ public class GenRuleCommandSubstitutionTest extends BuildViewTestCase { assertExpansionFails("$(locationsz) not defined", "//test"); genrule("$(locationsz )"); - assertExpansionFails("$(locationsz) not defined", "//test"); + assertExpansionFails("$(locationsz ) not defined", "//test"); genrule("$(locationsz foo )"); - assertExpansionFails("$(locationsz) not defined", "//test"); + assertExpansionFails("$(locationsz foo ) not defined", "//test"); } @Test @@ -447,8 +447,8 @@ public class GenRuleCommandSubstitutionTest extends BuildViewTestCase { assertNoEvents(); genrule("$(basename file)"); - assertExpansionFails("$(basename) not defined", "//test"); - assertContainsEvent("$(basename) not defined"); + assertExpansionFails("$(basename file) not defined", "//test"); + assertContainsEvent("$(basename file) not defined"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java index bc4b40e197..c4f5f5af30 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java @@ -172,7 +172,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase { setupSkylarkFunction(line); fail(); } catch (EvalException e) { - assertThat(e).hasMessageThat().isEqualTo(errorMsg); + assertThat(e).hasMessage(errorMsg); } } |