BazelTestSuiteRule is documented. The "suites" attribute is removed from BazelTestSuiteRule.

 - "suites" is deprecated
 - it has the exact same functionality as "tests"

--
MOS_MIGRATED_REVID=88426066

4 files changed, 79 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelTestSuiteRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelTestSuiteRule.java
index 54db469910..475c7d575b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelTestSuiteRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelTestSuiteRule.java
@@ -37,10 +37,66 @@ public final class BazelTestSuiteRule implements RuleDefinition {
     return builder
         .override(attr("testonly", BOOLEAN).value(true)
             .nonconfigurable("policy decision: should be consistent across configurations"))
+        /* <!-- #BLAZE_RULE(test_suite).ATTRIBUTE(tags) -->
+        List of text tags such as "small" or "database" or "-flaky". Tags may be any valid string.
+        ${SYNOPSIS}
+        <p>
+          Tags which begin with a "-" character are considered negative tags. The
+          preceding "-" character is not considered part of the tag, so a suite tag
+          of "-small" matches a test's "small" size. All other tags are considered
+          positive tags.
+        </p>
+        <p>
+          Optionally, to make positive tags more explicit, tags may also begin with the
+          "+" character, which will not be evaluated as part of the text of the tag. It
+          merely makes the positive and negative distinction easier to read.
+        </p>
+        <p>
+          Only test rules that match <b>all</b> of the positive tags and <b>none</b> of the negative
+          tags will be included in the test suite. Note that this does not mean that error checking
+          for dependencies on tests that are filtered out is skipped; the dependencies on skipped
+          tests still need to be legal (e.g. not blocked by visibility or obsoleteness constraints).
+        </p>
+        <p>
+          The <code>manual</code> tag keyword is treated specially. It marks the
+          <code>test_suite</code> target as "manual" so that it will be ignored by the wildcard
+          expansion and automated testing facilities. It does not work as a filter on the set
+          of tests in the suite. So when the <code>manual</code> tag is used on a test_suite, test
+          rules do not have to be tagged as <code>manual</code> to be included in the test suite.
+        </p>
+        <p>
+          Note that a test's <code>size</code> is treated as a tag for the purpose of filtering.
+        </p>
+        <p>
+          If you need a <code>test_suite</code> that contains tests with mutually exclusive tags
+          (e.g. all small and medium tests), you'll have to create three <code>test_suite</code>
+          rules: one for all small tests, one for all medium tests, and one that includes the
+          previous two.
+        </p>
+        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+
+        // TODO(bazel-team): we should have a boolean attribute instead of the "manual" hack
+
+        /* <!-- #BLAZE_RULE(test_suite).ATTRIBUTE(tests) -->
+        A list of test suites and test targets of any language.
+        ${SYNOPSIS}
+        <p>
+          Any <code>*_test</code> is accepted here, independent of the language. No
+          <code>*_binary</code> targets are accepted however, even if they happen to run a test.
+          Filtering by the specified <code>tags</code> is only done for tests listed directly in
+          this attribute. If this attribute contains <code>test_suite</code>s, the tests inside
+          those will not be filtered by this <code>test_suite</code> (they are considered to be
+          filtered already).
+        </p>
+        <p>
+          If the <code>tests</code> attribute is unspecified or empty, the rule will default to
+          including all test rules in the current BUILD file that are not tagged as
+          <code>manual</code> or marked as <code>obsolete</code>. These rules are still subject
+          to <code>tag</code> filtering.
+        </p>
+        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(attr("tests", LABEL_LIST).orderIndependent().allowedFileTypes()
             .nonconfigurable("policy decision: should be consistent across configurations"))
-        .add(attr("suites", LABEL_LIST).orderIndependent().allowedFileTypes()
-            .nonconfigurable("policy decision: should be consistent across configurations"))
         // This magic attribute contains all *test rules in the package, iff
         // tests=[] and suites=[]:
         .add(attr("$implicit_tests", LABEL_LIST)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 186b9d949f..b65e35858f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1348,7 +1348,8 @@ public class Package implements Serializable {
         AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
         if (rule.getRuleClass().equals("test_suite")
             && attributes.get("tests", Type.LABEL_LIST).isEmpty()
-            && attributes.get("suites", Type.LABEL_LIST).isEmpty()) {
+            && (!rule.getRuleClassObject().hasAttr("suites", Type.LABEL_LIST)
+                || attributes.get("suites", Type.LABEL_LIST).isEmpty())) {
           rule.setAttributeValueByName("$implicit_tests", allTests);
         }
       }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java
index dbd4dae03d..fb5cf5d4af 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java
@@ -281,7 +281,9 @@ public final class TestTargetUtils {
       // Note that testsAndSuites can contain input file targets; the test_suite rule does not
       // restrict the set of targets that can appear in tests or suites.
       testsAndSuites.addAll(getPrerequisites(testSuite, "tests"));
-      testsAndSuites.addAll(getPrerequisites(testSuite, "suites"));
+      if (testSuite.getRuleClassObject().hasAttr("suites", Type.LABEL_LIST)) {
+        testsAndSuites.addAll(getPrerequisites(testSuite, "suites"));
+      }
 
       // 1. Add all tests
       for (Target test : testsAndSuites) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
index ef795aa617..b0cb6e9986 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.test;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -60,9 +61,9 @@ public class TestSuite implements RuleConfiguredTargetFactory {
     // Manual tests are already filtered out there. That is what $implicit_tests is about.
     for (TransitiveInfoCollection dep :
           Iterables.concat(
-              ruleContext.getPrerequisites("tests", Mode.TARGET),
-              ruleContext.getPrerequisites("suites", Mode.TARGET),
-              ruleContext.getPrerequisites("$implicit_tests", Mode.TARGET))) {
+              getPrerequisites(ruleContext, "tests"),
+              getPrerequisites(ruleContext, "suites"),
+              getPrerequisites(ruleContext, "$implicit_tests"))) {
       if (dep.getProvider(TestProvider.class) != null) {
         List<String> tags = dep.getProvider(TestProvider.class).getTestTags();
         if (!TestTargetUtils.testMatchesFilters(
@@ -85,7 +86,19 @@ public class TestSuite implements RuleConfiguredTargetFactory {
         .build();
   }
 
+  private Iterable<? extends TransitiveInfoCollection> getPrerequisites(
+      RuleContext ruleContext, String attributeName) {
+    if (ruleContext.getRule().getRuleClassObject().hasAttr(attributeName, Type.LABEL_LIST)) {
+      return ruleContext.getPrerequisites(attributeName, Mode.TARGET);
+    } else {
+      return ImmutableList.<TransitiveInfoCollection>of();
+    }
+  }
+
   private void checkTestsAndSuites(RuleContext ruleContext, String attributeName) {
+    if (!ruleContext.getRule().getRuleClassObject().hasAttr(attributeName, Type.LABEL_LIST)) {
+      return;
+    }
     for (TransitiveInfoCollection dep : ruleContext.getPrerequisites(attributeName, Mode.TARGET)) {
       // TODO(bazel-team): Maybe convert the TransitiveTestsProvider into an inner interface.
       TransitiveTestsProvider provider = dep.getProvider(TransitiveTestsProvider.class);