Fix bug in skyframe-based test-suite expansion

It was tracking filtered tests and then applying the filter at the next higher
level.

I also added a bunch of comments - we actually have four implementations of
test suite expansion, and they are not consistent. Sorry about that.

PiperOrigin-RevId: 199629485

1 files changed, 3 insertions, 1 deletions

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 5767d83f7c..26a8a9f71c 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
@@ -52,6 +52,7 @@ public class TestSuite implements RuleConfiguredTargetFactory {
 
     List<String> tagsAttribute = new ArrayList<>(
         ruleContext.attributes().get("tags", Type.STRING_LIST));
+    // TODO(ulfjack): This is inconsistent with the other places that do test_suite expansion.
     tagsAttribute.remove("manual");
     Pair<Collection<String>, Collection<String>> requiredExcluded =
         TestTargetUtils.sortTagsBySense(tagsAttribute);
@@ -66,9 +67,10 @@ public class TestSuite implements RuleConfiguredTargetFactory {
               getPrerequisites(ruleContext, "tests"),
               getPrerequisites(ruleContext, "$implicit_tests"))) {
       if (dep.getProvider(TestProvider.class) != null) {
+        // getTestTags maps to Rule.getRuleTags.
         List<String> tags = dep.getProvider(TestProvider.class).getTestTags();
         if (!TestTargetUtils.testMatchesFilters(
-            tags, requiredExcluded.first, requiredExcluded.second, true)) {
+            tags, requiredExcluded.first, requiredExcluded.second)) {
           // This test does not match our filter. Ignore it.
           continue;
         }