Add more assertions about test suite expansion behavior

Characterizing this correctly this time: Target patterns are evaluated with two different sets of semantics. For determining which targets to build, only test suites in exclusion target patterns are expanded. For determining which tests to run (or determining which targets to build if --build_tests_only is set), test suites in all target patterns are expanded. In each case, test suites are expanded for each target pattern individually, not for the whole set of targets after the list of target patterns is processed.

Also update a related code comment.

The newer implementation for this logic already has equivalent tests for this behavior, in particular testFilterNegativeTestFromTestSuite and testNegativeTestSuiteExpanded in LoadingPhaseRunnerTest.

RELNOTES: None
PiperOrigin-RevId: 174756583

1 files changed, 17 insertions, 8 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 bf8a2cb23f..b62e5a89de 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
@@ -807,15 +807,24 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe
 
   @Test
   public void testTestSuiteExclusion() throws Exception {
-    // This step doesn't do test_suite expansion of the individual target patterns. Doing so
-    // would cause an exclusion of the targets in a package to exclude all tests referred to in test
-    // suites in that package.
+    // Test suites are expanded differently depending on if FILTER_TESTS is used. Those semantics
+    // are used for determining what tests are run (and for determining what targets are built
+    // only if --build_only_tests is set). Test suites are expanded for each target pattern
+    // in sequence, not the whole set of target patterns after all the inclusions and exclusions
+    // are processed. Test suites are expanded for inclusion test targets in all cases, but for
+    // exclusion test targets only for determining what tests should be run.
     scratch.file(
-        "parent/excluded/BUILD",
-        "test_suite(name = 'test_suite', tests = ['//parent/other:specific_test'])");
-    scratch.file("parent/other/BUILD", "cc_test(name = 'specific_test')");
-    assertThat(parseList("parent/...", "-parent/excluded/...")).containsExactlyElementsIn(
-        labels("//parent/other:specific_test"));
+        "parent/test_suite/BUILD",
+        "test_suite(name = 'test_suite', tests = ['//parent/test:specific_test'])");
+    scratch.file("parent/test/BUILD", "cc_test(name = 'specific_test')");
+    assertThat(parseList("parent/...", "-parent/test_suite/..."))
+        .containsExactlyElementsIn(labels("//parent/test:specific_test"));
+    assertThat(parseList(FILTER_TESTS, "parent/...", "-parent/test_suite/...")).isEmpty();
+    assertThat(parseList("parent/test_suite:test_suite", "-parent/test:specific_test"))
+        .containsExactlyElementsIn(labels("//parent/test_suite:test_suite"));
+    assertThat(
+            parseList(FILTER_TESTS, "parent/test_suite:test_suite", "-parent/test:specific_test"))
+        .isEmpty();
   }
 
   /** Regression test for bug: "blaze test "no targets found" warning now fatal" */