From 301ab54fb4e1249c8bc593e16cb0efe18392e504 Mon Sep 17 00:00:00 2001
From: Michael Muller <mmuller@enduden.com>
Date: Tue, 5 Nov 2019 12:04:56 -0500
Subject: [PATCH] Improve FilteringTest so it works in all cases (#339)

* Improve FilteringTest so it works in all cases

- Disable failOnMatchingTests so we don't fail if we filter out all of the
  tests for any given task.
- Add excludeTestCases flag so that we can turn this behavior off for test
  tasks that need to run TestCas/TestSuite classes.
- Add includeAllTests() function for test tasks where we don't explicitly
  include our required tests.

This makes all test tasks in core FilteringTest, with the exception of the
default test task which now does nothing (it just depends on the other test
tasks).

* Improve wording of excludeTestCases comment
---
 core/build.gradle | 59 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/core/build.gradle b/core/build.gradle
index f710bfe74..aa498629c 100644
--- a/core/build.gradle
+++ b/core/build.gradle
@@ -590,14 +590,43 @@ artifacts {
  */
 class FilteringTest extends Test {
 
-  void setTests(List<String> tests) {
-    // Common exclude pattern. See README in parent directory for explanation.
-    exclude "**/*TestCase.*", "**/*TestSuite.*"
-    include tests
+  private void applyTestFilter() {
     if (project.testFilter) {
       testNameIncludePatterns = project.testFilter.split(',')
+
+      // By default, gradle test tasks will produce a failure if no tests
+      // match the include/exclude/filter rules.  Since test filtering allows us
+      // to select a set of tests from a particular task, we don't want this
+      // behavior.
+      filter.failOnNoMatchingTests = false
     }
   }
+
+  /**
+   * Set to false if you also want to include TestCase and TestSuite classes.
+   *
+   * <p>Must be defined before "test", if at all.
+   */
+  boolean excludeTestCases = true
+
+  void setTests(List<String> tests) {
+    // Common exclude pattern. See README in parent directory for explanation.
+    if (excludeTestCases) {
+      exclude "**/*TestCase.*", "**/*TestSuite.*"
+    }
+    include tests
+    applyTestFilter()
+  }
+
+  /**
+   * Include all of the tests (except Test{Case,TestSuite}).  This actually
+   * doesn't explicitly "include" anything, in which cast the Test class tries
+   * to include everything that is not explicitly excluded.
+   */
+  void includeAllTests() {
+    exclude "**/*TestCase.*", "**/*TestSuite.*"
+    applyTestFilter()
+  }
 }
 
 task fragileTest(type: FilteringTest) {
@@ -620,12 +649,9 @@ task outcastTest(type: FilteringTest) {
 }
 
 // Dedicated test suite for schema-dependent tests.
-task sqlIntegrationTest(type: Test) {
-  include 'google/registry/schema/integration/SqlIntegrationTestSuite.*'
-  // Copied from FilteringTest. Not inheriting b/c it excludes TestSuites.
-  if (project.testFilter) {
-    testNameIncludePatterns = project.testFilter.split(',')
-  }
+task sqlIntegrationTest(type: FilteringTest) {
+  excludeTestCases = false
+  tests = ['google/registry/schema/integration/SqlIntegrationTestSuite.*']
 }
 
 task findGoldenImages(type: JavaExec) {
@@ -710,9 +736,8 @@ task flowDocsTool(type: JavaExec) {
   args arguments
 }
 
-test {
-  // Common exclude pattern. See README in parent directory for explanation.
-  exclude "**/*TestCase.*", "**/*TestSuite.*"
+task standardTest(type: FilteringTest) {
+  includeAllTests()
   exclude fragileTestPatterns
   exclude outcastTestPatterns
 
@@ -734,7 +759,13 @@ test {
   doFirst {
     new File(screenshotsDir).deleteDir()
   }
-}.dependsOn(fragileTest, outcastTest)
+}
+
+test {
+  // Don't run any tests from this task, all testing gets done in the
+  // FilteringTest tasks.
+  exclude "**"
+}.dependsOn(fragileTest, outcastTest, standardTest)
 
 createUberJar('nomulus', 'nomulus', 'google.registry.tools.RegistryTool')
 project.nomulus.dependsOn project(':third_party').jar