Restore Previous Test Options Behavior

subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java

@@ -81,6 +81,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
@@ -1359,7 +1360,7 @@ private void validate(String output, String displayName) {
                         i++;
                     } else if (isDeprecationMessageInHelpDescription(line)) {
                         i++;
-                    } else if (expectedDeprecationWarnings.removeIf(warning -> line.contains(warning))) {
+                    } else if (removeFirstExpectedDeprecationWarning(warning -> line.contains(warning))) {
                         // Deprecation warning is expected
                         i++;
                         i = skipStackTrace(lines, i);
@@ -1380,6 +1381,15 @@ private void validate(String output, String displayName) {
                 }
             }
 
+            private boolean removeFirstExpectedDeprecationWarning(final Predicate<String> condition) {
+                final Optional<String> firstMatch = expectedDeprecationWarnings.stream().filter(condition).findFirst();
+                if (firstMatch.isPresent()) {
+                    return expectedDeprecationWarnings.remove(firstMatch.get());
+                } else {
+                    return false;
+                }
+            }
+
             private int skipStackTrace(List<String> lines, int i) {
                 while (i < lines.size() && STACK_TRACE_ELEMENT.matcher(lines.get(i)).matches()) {
                     i++;

subprojects/plugins/src/main/java/org/gradle/api/plugins/jvm/internal/DefaultJvmTestSuite.java

@@ -24,6 +24,7 @@
 import org.gradle.api.artifacts.dsl.DependencyHandler;
 import org.gradle.api.internal.tasks.AbstractTaskDependency;
 import org.gradle.api.internal.tasks.TaskDependencyResolveContext;
+import org.gradle.api.internal.tasks.testing.TestFramework;
 import org.gradle.api.internal.tasks.testing.filter.DefaultTestFilter;
 import org.gradle.api.internal.tasks.testing.junit.JUnitTestFramework;
 import org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestFramework;
@@ -41,6 +42,8 @@
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
+import java.util.HashMap;
+import java.util.Map;
 
 public abstract class DefaultJvmTestSuite implements JvmTestSuite {
     public enum Frameworks {
@@ -135,19 +138,24 @@ public DefaultJvmTestSuite(String name, ConfigurationContainer configurations, D
 
         addDefaultTestTarget();
 
+        // Until the values here can be finalized upon the user setting them (see the org.gradle.api.tasks.testing.Test#testFramework(Closure) method),
+        // in Gradle 8, we will be executing the provider lambda used as the convention multiple times.  So make sure, within a Test Suite, that we
+        // always return the same one via computeIfAbsent() against this map.
+        final Map<Frameworks, TestFramework> testFrameworkMap = new HashMap<>(3);
+
         this.targets.withType(JvmTestSuiteTarget.class).configureEach(target -> {
             target.getTestTask().configure(task -> {
                 task.getTestFrameworkProperty().convention(getTestingFramework().map(framework -> {
                     switch(framework.framework) {
                         case NONE: // fall-through
                         case JUNIT4: // fall-through
                         case KOTLIN_TEST:
-                            return new JUnitTestFramework(task, (DefaultTestFilter) task.getFilter());
+                            return testFrameworkMap.computeIfAbsent(framework.framework, f -> new JUnitTestFramework(task, (DefaultTestFilter) task.getFilter()));
                         case JUNIT_JUPITER: // fall-through
                         case SPOCK:
-                            return new JUnitPlatformTestFramework((DefaultTestFilter) task.getFilter());
+                            return testFrameworkMap.computeIfAbsent(framework.framework, f -> new JUnitPlatformTestFramework((DefaultTestFilter) task.getFilter()));
                         case TESTNG:
-                            return new TestNGTestFramework(task, task.getClasspath(), (DefaultTestFilter) task.getFilter(), getObjectFactory());
+                            return testFrameworkMap.computeIfAbsent(framework.framework, f -> new TestNGTestFramework(task, task.getClasspath(), (DefaultTestFilter) task.getFilter(), getObjectFactory()));
                         default:
                             throw new IllegalStateException("do not know how to handle " + framework);
                     }

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/TestTaskIntegrationTest.groovy

@@ -282,6 +282,152 @@ class TestTaskIntegrationTest extends JUnitMultiVersionIntegrationSpec {
         extraArgs << [[], ["--tests", "MyTest"]]
     }
 
+    def "options set prior to setting same test framework will warn and have no effect"() {
+        ignoreWhenJupiter()
+
+        given:
+        file('src/test/java/MyTest.java') << standaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """
+            import org.gradle.api.internal.tasks.testing.*
+
+            apply plugin: 'java'
+
+            ${mavenCentralRepository()}
+            dependencies {
+                testImplementation 'junit:junit:${JUnitCoverage.NEWEST}'
+            }
+
+            test {
+                options {
+                    excludeCategories = ["Slow"]
+                }
+                useJUnit()
+            }
+
+            tasks.register('verifyTestOptions') {
+                doLast {
+                    assert tasks.getByName("test").getOptions().getClass() == JUnitOptions.class
+                    assert !tasks.getByName("test").getOptions().getExcludeCategories().contains("Slow")
+                }
+            }
+        """.stripIndent()
+
+        executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated.")
+
+        expect:
+        succeeds("test", "verifyTestOptions", "--warn")
+    }
+
+    def "options set prior to changing test framework will produce additional warning and have no effect"() {
+        ignoreWhenJUnitPlatform()
+
+        given:
+        file('src/test/java/MyTest.java') << junitPlatformStandaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """
+            import org.gradle.api.internal.tasks.testing.*
+
+            apply plugin: 'java'
+
+            ${mavenCentralRepository()}
+            dependencies {
+                testImplementation 'org.junit.jupiter:junit-jupiter:${JUnitCoverage.LATEST_JUPITER_VERSION}'
+            }
+
+            test {
+                options {
+                    excludeCategories = ["Slow"]
+                }
+                useJUnitPlatform()
+            }
+
+            tasks.register('verifyTestOptions') {
+                doLast {
+                    assert tasks.getByName("test").getOptions().getClass() == JUnitPlatformOptions.class
+                }
+            }
+        """.stripIndent()
+
+        executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated.")
+
+        when:
+        succeeds("test", "verifyTestOptions", "--warn")
+
+        then:
+        outputContains("Test framework is changing from 'JUnitTestFramework', previous option configuration would not be applicable.")
+    }
+
+    def "options accessed and not explicitly configured prior to setting test framework will also warn"() {
+        given:
+        file('src/test/java/MyTest.java') << junitPlatformStandaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """
+            import org.gradle.api.internal.tasks.testing.*
+
+            apply plugin: 'java'
+
+            ${mavenCentralRepository()}
+            dependencies {
+                testImplementation 'org.junit.jupiter:junit-jupiter:${JUnitCoverage.LATEST_JUPITER_VERSION}'
+            }
+
+            def options = test.getOptions()
+
+            test {
+                useJUnitPlatform()
+            }
+
+            tasks.register('verifyTestOptions') {
+                doLast {
+                    assert options.getClass() == JUnitOptions.class
+                    assert tasks.getByName("test").getOptions().getClass() == JUnitPlatformOptions.class
+                }
+            }
+        """.stripIndent()
+
+        executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated.")
+
+        expect:
+        succeeds("test", "verifyTestOptions", "--warn")
+    }
+
+    def "options configured after setting test framework works"() {
+        given:
+        file('src/test/java/MyTest.java') << junitPlatformStandaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """
+            import org.gradle.api.internal.tasks.testing.*
+
+            apply plugin: 'java'
+
+            ${mavenCentralRepository()}
+            dependencies {
+                testImplementation 'org.junit.jupiter:junit-jupiter:${JUnitCoverage.LATEST_JUPITER_VERSION}'
+            }
+
+            test {
+                useJUnitPlatform()
+                options {
+                    excludeTags = ["Slow"]
+                }
+            }
+
+            tasks.register('verifyTestOptions') {
+                doLast {
+                    assert tasks.getByName("test").getOptions().getExcludeTags().contains("Slow")
+                }
+            }
+        """.stripIndent()
+
+        expect:
+        succeeds("test", "verifyTestOptions", "--warn")
+    }
+
     private static String standaloneTestClass() {
         return testClass('MyTest')
     }
@@ -300,6 +446,24 @@ class TestTaskIntegrationTest extends JUnitMultiVersionIntegrationSpec {
         """.stripIndent()
     }
 
+    private static String junitPlatformStandaloneTestClass() {
+        return junitPlatformTestClass('MyTest')
+    }
+
+    private static String junitPlatformTestClass(String className) {
+        return """
+            import org.junit.jupiter.api.*;
+
+            public class $className {
+               @Test
+               public void test() {
+                  System.out.println(System.getProperty("java.version"));
+                  Assertions.assertEquals(1,1);
+               }
+            }
+        """.stripIndent()
+    }
+
     private static String java9Build() {
         """
             apply plugin: 'java'

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec.groovy

@@ -122,6 +122,7 @@ public class MyTest {
         |testing {
         |   suites {
         |       test {
+        |           useJUnit()
         |           targets {
         |               all {
         |                   testTask.configure {
@@ -143,7 +144,23 @@ public class MyTest {
 
         when:
         resources.maybeCopy("JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged")
-        buildFile << "test.useJUnit()"
+        buildFile << """
+        |testing {
+        |   suites {
+        |       test {
+        |           useJUnit()
+        |           targets {
+        |               all {
+        |                   testTask.configure {
+        |                       options {
+        |                           ${type} 'org.gradle.CategoryB'
+        |                       }
+        |                   }
+        |               }
+        |           }
+        |       }
+        |   }
+        |}""".stripMargin()
 
         and:
         succeeds ':test'

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGUpToDateCheckIntegrationTest.groovy

@@ -45,7 +45,26 @@ class TestNGUpToDateCheckIntegrationTest extends AbstractIntegrationSpec {
     @Issue('https://github.com/gradle/gradle/issues/4924')
     def 'test task is up-to-date when #property is changed because it should not impact output'() {
         given:
-        TestNGCoverage.enableTestNG(buildFile)
+        buildScript """
+            apply plugin: "java"
+            ${mavenCentralRepository()}
+            testing {
+                suites {
+                    test {
+                        useTestNG('${TestNGCoverage.NEWEST}')
+                        targets {
+                            all {
+                                testTask.configure {
+                                    options {
+                                        /* left empty */
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        """
 
         when:
         succeeds ':test'
@@ -97,7 +116,26 @@ class TestNGUpToDateCheckIntegrationTest extends AbstractIntegrationSpec {
     @Issue('https://github.com/gradle/gradle/issues/4924')
     def "re-executes test when #property is changed"() {
         given:
-        TestNGCoverage.enableTestNG(buildFile)
+        buildScript """
+            apply plugin: "java"
+            ${mavenCentralRepository()}
+            testing {
+                suites {
+                    test {
+                        useTestNG('${TestNGCoverage.NEWEST}')
+                        targets {
+                            all {
+                                testTask.configure {
+                                    options {
+                                        /* left empty */
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        """
 
         when:
         succeeds ':test'