Fix newly introduced bug where an empty test_suite.tests attribute was
incorrectly not causing the test_suite.$implicit_tests attribute to get set.
The test_suite.$implicit_tests attribute is the implementation of the latter paragraph in
the documentation of the test_suite.tests attribute
(https://docs.bazel.build/versions/3.3.0/be/general.html#test_suite); this bug
here was causing that feature to be broken in the "or empty" case.
This bug was introduced in commit a46ad6.
RELNOTES: None
PiperOrigin-RevId: 320109724
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index be7a417..40e9738 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -2235,12 +2235,13 @@
}
}
- // An instance of the built-in 'test_suite' rule with no explicit 'tests'
- // attribute gets an '$implicit_tests' attribute, whose value is a
- // shared per-package list of all test labels, populated later.
+ // An instance of the built-in 'test_suite' rule with an undefined or empty 'tests' attribute
+ // attribute gets an '$implicit_tests' attribute, whose value is a shared per-package list of
+ // all test labels, populated later.
if (this.name.equals("test_suite")) {
Attribute implicitTests = this.getAttributeByName("$implicit_tests");
- if (implicitTests != null && !definedAttrIndices.get(this.getAttributeIndex("tests"))) {
+ if (implicitTests != null
+ && NonconfigurableAttributeMapper.of(rule).get("tests", BuildType.LABEL_LIST).isEmpty()) {
boolean explicit = true; // so that it appears in query output
rule.setAttributeValue(implicitTests, pkgBuilder.testSuiteImplicitTests, explicit);
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 48247c5..24dc17e 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -453,14 +453,16 @@
"/x/BUILD",
"java_test(name='j')",
"test_suite(name='t1')",
- "test_suite(name='t2', tests=['//foo'])",
+ "test_suite(name='t2', tests=[])",
"test_suite(name='t3', tests=['//foo'])",
+ "test_suite(name='t4', tests=['//foo'])",
"cc_test(name='c')");
Package pkg = packages.createPackage("x", RootedPath.toRootedPath(root, path));
// Things to note:
- // - the t1 refers to both :j and :c, even though :c is a forward reference.
- // - $implicit_tests is empty unless tests=[]
+ // - The '$implicit_tests' attribute is unset unless the 'tests' attribute is unset or empty.
+ // - The '$implicit_tests' attribute's value for t1 and t2 is magically able to contain both j
+ // and c, even though c is instantiated after t1 and t2 are.
assertThat(attributes(pkg.getRule("t1")).get("$implicit_tests", BuildType.LABEL_LIST))
.containsExactlyElementsIn(
@@ -468,9 +470,14 @@
Label.parseAbsolute("//x:c", ImmutableMap.of()),
Label.parseAbsolute("//x:j", ImmutableMap.of())));
assertThat(attributes(pkg.getRule("t2")).get("$implicit_tests", BuildType.LABEL_LIST))
- .isEmpty();
+ .containsExactlyElementsIn(
+ Sets.newHashSet(
+ Label.parseAbsolute("//x:c", ImmutableMap.of()),
+ Label.parseAbsolute("//x:j", ImmutableMap.of())));
assertThat(attributes(pkg.getRule("t3")).get("$implicit_tests", BuildType.LABEL_LIST))
.isEmpty();
+ assertThat(attributes(pkg.getRule("t4")).get("$implicit_tests", BuildType.LABEL_LIST))
+ .isEmpty();
}
@Test
