diff options
author | Nathan Harmata <nharmata@google.com> | 2016-04-22 18:32:11 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-04-22 21:19:36 +0000 |
commit | 1c15569abb3fda303a6016f31ad0480c79ba85c1 (patch) | |
tree | 971b3ab4267830f11a23f73b78453c53783d7f79 /src/test | |
parent | ac619562934b41e201ca5fb917310cb145bf5dd8 (diff) |
Fix target parsing bug with targets in the empty package (e.g. "blah" was incorrectly not being parsed as "//:blah").
Also add tests for parsing absolute labels in the empty package.
The empty package has been a thing in Bazel for a while now.
Note that the old error message in this case "couldn't determine target for filename 'blah'" was almost always misleading and unhelpful since we were (almost certainly incorrectly) assuming the user intended for 'blah' to be an input file in the empty package. Now the error message would be "no such target '//:blah'") which is similarly misleading and unhelpful but probably marginally less so. If we desire to improve this, a future cleanup can introduce smarter error messages.
--
MOS_MIGRATED_REVID=120566819
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java | 114 |
1 files changed, 88 insertions, 26 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java index 68b9f64a35..c99a5822b5 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java @@ -53,6 +53,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe private Set<Label> rulesBeneathFoo; private Set<Label> rulesBeneathFooBar; private Set<Label> rulesBeneathOtherrules; + private Set<Label> rulesInTopLevelPackage; private Set<Label> rulesInFoo; private Set<Label> rulesInFooBar; private Set<Label> rulesInOtherrules; @@ -60,6 +61,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe private Set<Label> targetsInFooBar; private Set<Label> targetsBeneathFoo; private Set<Label> targetsInOtherrules; + private Set<Label> targetsInTopLevelPackage; @Before public final void createFiles() throws Exception { @@ -67,6 +69,10 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe boolean hasImplicitCcOutputs = ruleClassProvider.getRuleClassMap().get("cc_library") .getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE; + scratch.file("BUILD", + "filegroup(name = 'fg', srcs = glob(['*.cc']))"); + scratch.file("foo.cc"); + scratch.file("foo/BUILD", "cc_library(name = 'foo1', srcs = [ 'foo1.cc' ], hdrs = [ 'foo1.h' ])", "exports_files(['baz/bang'])"); @@ -90,9 +96,11 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe rulesBeneathFooBar = labels("//foo/bar:bar1", "//foo/bar:bar2"); rulesBeneathOtherrules = labels( "//otherrules:suite1", "//otherrules:wiz", "//otherrules:group"); + rulesInTopLevelPackage = labels("//:fg"); rulesInFoo = labels("//foo:foo1"); rulesInFooBar = labels("//foo/bar:bar1", "//foo/bar:bar2"); rulesInOtherrules = rulesBeneathOtherrules; + targetsInTopLevelPackage = labels("//:BUILD", "//:foo.cc", "//:fg"); targetsInFoo = labels( "//foo:foo1", @@ -409,7 +417,11 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe private void runFindAllRules(String pattern) throws Exception { assertThat(parseList(pattern)) - .containsExactlyElementsIn(Sets.union(rulesBeneathFoo, rulesBeneathOtherrules)); + .containsExactlyElementsIn(ImmutableSet.builder() + .addAll(rulesBeneathFoo) + .addAll(rulesBeneathOtherrules) + .addAll(rulesInTopLevelPackage) + .build()); assertNoEvents(); eventCollector.clear(); } @@ -423,7 +435,11 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe private void runFindAllTargets(String pattern) throws Exception { assertThat(parseList(pattern)) - .containsExactlyElementsIn(Sets.union(targetsBeneathFoo, targetsInOtherrules)); + .containsExactlyElementsIn(ImmutableSet.builder() + .addAll(targetsBeneathFoo) + .addAll(targetsInOtherrules) + .addAll(targetsInTopLevelPackage) + .build()); assertNoEvents(); eventCollector.clear(); } @@ -440,7 +456,11 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe scratch.file("experimental/BUILD", "cc_library(name = 'experimental', srcs = [ 'experimental.cc' ])"); assertThat(parseList("//...")) - .containsExactlyElementsIn(Sets.union(rulesBeneathFoo, rulesBeneathOtherrules)); + .containsExactlyElementsIn(ImmutableSet.builder() + .addAll(rulesBeneathFoo) + .addAll(rulesBeneathOtherrules) + .addAll(rulesInTopLevelPackage) + .build()); assertNoEvents(); } @@ -489,12 +509,6 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe } @Test - public void testGivesUpIfPackageDoesNotExist() throws Exception { - expectError("couldn't determine target from filename 'does/not/exist'", - "does/not/exist"); - } - - @Test public void testParsesIterableOfLabels() throws Exception { Set<Label> labels = Sets.newHashSet(Label.parseAbsolute("//foo/bar:bar1"), Label.parseAbsolute("//foo:foo1")); @@ -526,11 +540,6 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe parseIndividualTargetRelative("bar:wiz/all").toString()); } - @Test - public void testAll() throws Exception { - expectError("couldn't determine target from filename 'all'", "all"); - } - /** Regression test for a bug. */ @Test public void testDotDotDotDoesntMatchDeletedPackages() throws Exception { @@ -693,16 +702,6 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe /** Regression test for bug: "Bogus 'helpful' error message" */ @Test - public void testHelpfulMessageForFileOutsideOfAnyPackage() throws Exception { - scratch.file("goo/wiz/file"); - expectError("couldn't determine target from filename 'goo/wiz/file'", - "goo/wiz/file"); - expectError("couldn't determine target from filename 'goo/wiz'", - "goo/wiz"); - } - - /** Regression test for bug: "Bogus 'helpful' error message" */ - @Test public void testHelpfulMessageForDirectoryWhichIsASubdirectoryOfAPackage() throws Exception { scratch.file("bar/BUILD"); scratch.file("bar/quux/somefile"); @@ -890,7 +889,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe @Test public void testKeepGoingBadFilenameTarget() throws Exception { assertKeepGoing(rulesBeneathFoo, - "couldn't determine target from filename 'bad/filename/target'", + "no such target '//:bad/filename/target'", "bad/filename/target", "foo/..."); } @@ -902,7 +901,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe /*keepGoing=*/false); fail(); } catch (TargetParsingException e) { - assertThat(e.getMessage()).contains("couldn't determine target from filename"); + assertThat(e.getMessage()).contains("no such target"); } } @@ -1092,5 +1091,68 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe public void testExternalPackage() throws Exception { parseList("external:all"); } + + @Test + public void testTopLevelPackage_Relative_BuildFile() throws Exception { + Set<Label> result = parseList("BUILD"); + assertThat(result).containsExactly(Label.parseAbsolute("//:BUILD")); + } + + @Test + public void testTopLevelPackage_Relative_DeclaredTarget() throws Exception { + Set<Label> result = parseList("fg"); + assertThat(result).containsExactly(Label.parseAbsolute("//:fg")); + } + + @Test + public void testTopLevelPackage_Relative_All() throws Exception { + expectError("no such target '//:all'", "all"); + } + + @Test + public void testTopLevelPackage_Relative_ColonAll() throws Exception { + Set<Label> result = parseList(":all"); + assertThat(result).containsExactly(Label.parseAbsolute("//:fg")); + } + + @Test + public void testTopLevelPackage_Relative_InputFile() throws Exception { + Set<Label> result = parseList("foo.cc"); + assertThat(result).containsExactly(Label.parseAbsolute("//:foo.cc")); + } + + @Test + public void testTopLevelPackage_Relative_InputFile_NoSuchInputFile() throws Exception { + expectError("no such target '//:nope.cc'", "nope.cc"); + } + + @Test + public void testTopLevelPackage_Absolute_BuildFile() throws Exception { + Set<Label> result = parseList("//:BUILD"); + assertThat(result).containsExactly(Label.parseAbsolute("//:BUILD")); + } + + @Test + public void testTopLevelPackage_Absolute_DeclaredTarget() throws Exception { + Set<Label> result = parseList("//:fg"); + assertThat(result).containsExactly(Label.parseAbsolute("//:fg")); + } + + @Test + public void testTopLevelPackage_Absolute_All() throws Exception { + Set<Label> result = parseList("//:all"); + assertThat(result).containsExactly(Label.parseAbsolute("//:fg")); + } + + @Test + public void testTopLevelPackage_Absolute_InputFile() throws Exception { + Set<Label> result = parseList("//:foo.cc"); + assertThat(result).containsExactly(Label.parseAbsolute("//:foo.cc")); + } + + @Test + public void testTopLevelPackage_Absolute_InputFile_NoSuchInputFile() throws Exception { + expectError("no such target '//:nope.cc'", "//:nope.cc"); + } } |