Restore Previous Test Options Behavior (for Release Branch)

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.stream.Collectors;
@@ -1336,7 +1337,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);
@@ -1357,6 +1358,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 {
@@ -82,17 +85,17 @@ public String getDependency(String version) {
         }
     }
 
-    private static class TestingFramework {
-        private final Frameworks framework;
+    private static class VersionedTestingFramework {
+        private final Frameworks type;
         private final String version;
 
-        private TestingFramework(Frameworks framework, String version) {
+        private VersionedTestingFramework(Frameworks type, String version) {
             Preconditions.checkNotNull(version);
-            this.framework = framework;
+            this.type = type;
             this.version = version;
         }
     }
-    private final static TestingFramework NO_OPINION = new TestingFramework(Frameworks.NONE, "unset");
+    private final static VersionedTestingFramework NO_OPINION = new VersionedTestingFramework(Frameworks.NONE, "unset");
 
     private final ExtensiblePolymorphicDomainObjectContainer<JvmTestSuiteTarget> targets;
     private final SourceSet sourceSet;
@@ -101,7 +104,7 @@ private TestingFramework(Frameworks framework, String version) {
     private boolean attachedDependencies;
     private final Action<Void> attachDependencyAction;
 
-    protected abstract Property<TestingFramework> getTestingFramework();
+    protected abstract Property<VersionedTestingFramework> getVersionedTestingFramework();
 
     @Inject
     public DefaultJvmTestSuite(String name, ConfigurationContainer configurations, DependencyHandler dependencies, SourceSetContainer sourceSets) {
@@ -125,7 +128,7 @@ public DefaultJvmTestSuite(String name, ConfigurationContainer configurations, D
             // for the built-in test suite, we don't express an opinion, so we will not add any dependencies
             // if a user explicitly calls useJUnit or useJUnitJupiter, the built-in test suite will behave like a custom one
             // and add dependencies automatically.
-            getTestingFramework().convention(NO_OPINION);
+            getVersionedTestingFramework().convention(NO_OPINION);
         }
 
         this.targets = getObjectFactory().polymorphicDomainObjectContainer(JvmTestSuiteTarget.class);
@@ -135,21 +138,26 @@ 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> frameworkLookup = new HashMap<>(3);
+
         this.targets.withType(JvmTestSuiteTarget.class).configureEach(target -> {
             target.getTestTask().configure(task -> {
-                task.getTestFrameworkProperty().convention(getTestingFramework().map(framework -> {
-                    switch(framework.framework) {
+                task.getTestFrameworkProperty().convention(getVersionedTestingFramework().map(vtf -> {
+                    switch(vtf.type) {
                         case NONE: // fall-through
                         case JUNIT4: // fall-through
                         case KOTLIN_TEST:
-                            return new JUnitTestFramework(task, (DefaultTestFilter) task.getFilter());
+                            return frameworkLookup.computeIfAbsent(vtf.type, f -> new JUnitTestFramework(task, (DefaultTestFilter) task.getFilter()));
                         case JUNIT_JUPITER: // fall-through
                         case SPOCK:
-                            return new JUnitPlatformTestFramework((DefaultTestFilter) task.getFilter());
+                            return frameworkLookup.computeIfAbsent(vtf.type, f -> new JUnitPlatformTestFramework((DefaultTestFilter) task.getFilter()));
                         case TESTNG:
-                            return new TestNGTestFramework(task, task.getClasspath(), (DefaultTestFilter) task.getFilter(), getObjectFactory());
+                            return frameworkLookup.computeIfAbsent(vtf.type, f -> new TestNGTestFramework(task, task.getClasspath(), (DefaultTestFilter) task.getFilter(), getObjectFactory()));
                         default:
-                            throw new IllegalStateException("do not know how to handle " + framework);
+                            throw new IllegalStateException("do not know how to handle " + vtf);
                     }
                 }));
             });
@@ -158,14 +166,14 @@ public DefaultJvmTestSuite(String name, ConfigurationContainer configurations, D
 
     private void attachDependenciesForTestFramework(DependencyHandler dependencies, Configuration implementation) {
         if (!attachedDependencies) {
-            dependencies.addProvider(implementation.getName(), getTestingFramework().map(framework -> {
-                switch (framework.framework) {
+            dependencies.addProvider(implementation.getName(), getVersionedTestingFramework().map(framework -> {
+                switch (framework.type) {
                     case JUNIT4: // fall-through
                     case JUNIT_JUPITER: // fall-through
                     case SPOCK: // fall-through
                     case TESTNG: // fall-through
                     case KOTLIN_TEST:
-                        return framework.framework.getDependency(framework.version);
+                        return framework.type.getDependency(framework.version);
                     default:
                         throw new IllegalStateException("do not know how to handle " + framework);
                 }
@@ -211,7 +219,7 @@ public void useJUnit() {
 
     @Override
     public void useJUnit(String version) {
-        setFrameworkTo(new TestingFramework(Frameworks.JUNIT4, version));
+        setFrameworkTo(new VersionedTestingFramework(Frameworks.JUNIT4, version));
     }
 
     @Override
@@ -221,7 +229,7 @@ public void useJUnitJupiter() {
 
     @Override
     public void useJUnitJupiter(String version) {
-        setFrameworkTo(new TestingFramework(Frameworks.JUNIT_JUPITER, version));
+        setFrameworkTo(new VersionedTestingFramework(Frameworks.JUNIT_JUPITER, version));
     }
 
     @Override
@@ -231,7 +239,7 @@ public void useSpock() {
 
     @Override
     public void useSpock(String version) {
-        setFrameworkTo(new TestingFramework(Frameworks.SPOCK, version));
+        setFrameworkTo(new VersionedTestingFramework(Frameworks.SPOCK, version));
     }
 
     @Override
@@ -241,7 +249,7 @@ public void useKotlinTest() {
 
     @Override
     public void useKotlinTest(String version) {
-        setFrameworkTo(new TestingFramework(Frameworks.KOTLIN_TEST, version));
+        setFrameworkTo(new VersionedTestingFramework(Frameworks.KOTLIN_TEST, version));
     }
 
     @Override
@@ -251,11 +259,11 @@ public void useTestNG() {
 
     @Override
     public void useTestNG(String version) {
-        setFrameworkTo(new TestingFramework(Frameworks.TESTNG, version));
+        setFrameworkTo(new VersionedTestingFramework(Frameworks.TESTNG, version));
     }
 
-    private void setFrameworkTo(TestingFramework framework) {
-        getTestingFramework().set(framework);
+    private void setFrameworkTo(VersionedTestingFramework framework) {
+        getVersionedTestingFramework().set(framework);
         attachDependencyAction.execute(null);
     }
 

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

@@ -285,6 +285,149 @@ class TestTaskIntegrationTest extends JUnitMultiVersionIntegrationSpec {
         extraArgs << [[], ["--tests", "MyTest"]]
     }
 
+    def "options set prior to setting same test framework will warn and have no effect"() {
+        ignoreWhenJUnitPlatform()
+
+        given:
+        file('src/test/java/MyTest.java') << standaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """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
+                    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') << junitJupiterStandaloneTestClass()
+
+        settingsFile << "rootProject.name = 'Sample'"
+        buildFile << """apply plugin: 'java'
+
+            ${mavenCentralRepository()}
+            dependencies {
+                testImplementation 'org.junit.jupiter:junit-jupiter:${JUnitCoverage.LATEST_JUPITER_VERSION}'
+            }
+
+            test {
+                options {
+                    excludeCategories = ["Slow"]
+                }
+            }
+
+            test {
+                useJUnitPlatform()
+            }
+
+            tasks.register('verifyTestOptions') {
+                doLast {
+                    assert tasks.getByName("test").getOptions().getClass() == JUnitPlatformOptions
+                }
+            }
+        """.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') << junitJupiterStandaloneTestClass()
+
+        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
+                    assert tasks.getByName("test").getOptions().getClass() == JUnitPlatformOptions
+                }
+            }
+        """.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') << junitJupiterStandaloneTestClass()
+
+        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')
     }
@@ -303,6 +446,24 @@ class TestTaskIntegrationTest extends JUnitMultiVersionIntegrationSpec {
         """.stripIndent()
     }
 
+    private static String junitJupiterStandaloneTestClass() {
+        return junitJupiterTestClass('MyTest')
+    }
+
+    private static String junitJupiterTestClass(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/fixture/JUnitMultiVersionIntegrationSpec.groovy

@@ -108,6 +108,10 @@ abstract class JUnitMultiVersionIntegrationSpec extends MultiVersionIntegrationS
         Assume.assumeFalse(isJUnitPlatform())
     }
 
+    protected ignoreWhenJUnit4() {
+        Assume.assumeFalse(isJUnit4())
+    }
+
     static String getTestFramework() {
         isJUnitPlatform() ? "JUnitPlatform" : "JUnit"
     }