diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java index c6fc8c85c59d..333ec87c342b 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java +++ b/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 condition) { + final Optional firstMatch = expectedDeprecationWarnings.stream().filter(condition).findFirst(); + if (firstMatch.isPresent()) { + return expectedDeprecationWarnings.remove(firstMatch.get()); + } else { + return false; + } + } + private int skipStackTrace(List lines, int i) { while (i < lines.size() && STACK_TRACE_ELEMENT.matcher(lines.get(i)).matches()) { i++; diff --git a/subprojects/plugins/src/main/java/org/gradle/api/plugins/jvm/internal/DefaultJvmTestSuite.java b/subprojects/plugins/src/main/java/org/gradle/api/plugins/jvm/internal/DefaultJvmTestSuite.java index 5f1bf1c34b9f..4dc4cb5666ff 100644 --- a/subprojects/plugins/src/main/java/org/gradle/api/plugins/jvm/internal/DefaultJvmTestSuite.java +++ b/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,6 +138,11 @@ 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 testFrameworkMap = new HashMap<>(3); + this.targets.withType(JvmTestSuiteTarget.class).configureEach(target -> { target.getTestTask().configure(task -> { task.getTestFrameworkProperty().convention(getTestingFramework().map(framework -> { @@ -142,12 +150,12 @@ public DefaultJvmTestSuite(String name, ConfigurationContainer configurations, D 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); } diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/TestTaskIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/TestTaskIntegrationTest.groovy index eb49e6a29ab8..6f82ebc6f735 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/TestTaskIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/TestTaskIntegrationTest.groovy @@ -282,6 +282,155 @@ 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 << """ + 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') << 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 { + options { + excludeCategories = ["Slow"] + } + } + + test { + 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') << 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.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') << 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') } @@ -300,6 +449,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' diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/JUnitMultiVersionIntegrationSpec.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/JUnitMultiVersionIntegrationSpec.groovy index 302689040f16..cd5c523bea00 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/JUnitMultiVersionIntegrationSpec.groovy +++ b/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" } diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec.groovy index cf61ab33b111..a6b2c2cca98c 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec.groovy +++ b/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' diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGUpToDateCheckIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGUpToDateCheckIntegrationTest.groovy index e9658276802d..f86756fc5d03 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGUpToDateCheckIntegrationTest.groovy +++ b/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' diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testsuites/TestSuitesIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testsuites/TestSuitesIntegrationTest.groovy index 4aa829ce8172..54f23a111b29 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testsuites/TestSuitesIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testsuites/TestSuitesIntegrationTest.groovy @@ -401,7 +401,8 @@ class TestSuitesIntegrationTest extends AbstractIntegrationSpec { succeeds("checkConfiguration") } - def "test framework may not be changed once options have been used with test suites"() { + // TODO: Test Framework Selection - Revert this to may NOT in Gradle 8 + def "test framework MAY be changed once options have been used with test suites"() { buildFile << """ plugins { id 'java' @@ -433,10 +434,10 @@ class TestSuitesIntegrationTest extends AbstractIntegrationSpec { check.dependsOn testing.suites """ - when: - fails("check") - then: - failure.assertHasCause("The value for task ':integrationTest' property 'testFrameworkProperty' is final and cannot be changed any further.") + executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated. This is scheduled to be removed in Gradle 8.0.") + + expect: + succeeds("check") } // This checks for backwards compatibility with builds that may rely on this @@ -472,8 +473,11 @@ class TestSuitesIntegrationTest extends AbstractIntegrationSpec { } """ + executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated. This is scheduled to be removed in Gradle 8.0.") + executer.expectDocumentedDeprecationWarning("Accessing test options prior to setting test framework has been deprecated. This is scheduled to be removed in Gradle 8.0.") + when: - run "test" + succeeds( "test") then: executedAndNotSkipped(":test") @@ -592,4 +596,78 @@ class TestSuitesIntegrationTest extends AbstractIntegrationSpec { expect: succeeds("mytest", "assertNoTestClasses") } + + def "multiple getTestingFramework() calls on a test suite return same instance"() { + given: + buildFile << """ + plugins { + id 'java' + } + + def first = testing.suites.test.getTestingFramework() + def second = testing.suites.test.getTestingFramework() + + tasks.register('assertSameFrameworkInstance') { + doLast { + assert first === second + } + }""".stripIndent() + + expect: + succeeds("assertSameFrameworkInstance") + } + + def "multiple getTestingFramework() calls on a test suite return same instance even when calling useJUnit"() { + given: + buildFile << """ + plugins { + id 'java' + } + + def first = testing.suites.test.getTestingFramework() + + testing { + suites { + test { + useJUnit() + } + } + } + + def second = testing.suites.test.getTestingFramework() + + tasks.register('assertSameFrameworkInstance') { + doLast { + assert first === second + } + }""".stripIndent() + + expect: + succeeds("assertSameFrameworkInstance") + } + + def "multiple getTestingFramework() calls on a test suite return same instance even after toggling testing framework"() { + given: + buildFile << """ + plugins { + id 'java' + } + + def first = testing.suites.test.getTestingFramework() + + testing.suites.test.useJUnit() + testing.suites.test.useTestNG() + testing.suites.test.useJUnit() + + def second = testing.suites.test.getTestingFramework() + + tasks.register('assertSameFrameworkInstance') { + doLast { + assert first === second + } + }""".stripIndent() + + expect: + succeeds("assertSameFrameworkInstance") + } } diff --git a/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/build.gradle b/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/build.gradle index 5561ed9f1442..77acb152a3a2 100644 --- a/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/build.gradle +++ b/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/build.gradle @@ -19,11 +19,3 @@ apply plugin: "java" repositories { mavenCentral() } - -testing { - suites { - test { - useJUnit() - } - } -} diff --git a/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/src/test/java/org/gradle/CategoryB.java b/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/src/test/java/org/gradle/CategoryB.java new file mode 100644 index 000000000000..4c09973ad3ea --- /dev/null +++ b/subprojects/testing-jvm/src/integTest/resources/org/gradle/testing/junit/JUnitCategoriesIntegrationSpec/reExecutesWhenPropertyIsChanged/src/test/java/org/gradle/CategoryB.java @@ -0,0 +1,20 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle; + +public interface CategoryB { +} diff --git a/subprojects/testing-jvm/src/main/java/org/gradle/api/tasks/testing/Test.java b/subprojects/testing-jvm/src/main/java/org/gradle/api/tasks/testing/Test.java index 1193761e1eef..c53dc1f74fe4 100644 --- a/subprojects/testing-jvm/src/main/java/org/gradle/api/tasks/testing/Test.java +++ b/subprojects/testing-jvm/src/main/java/org/gradle/api/tasks/testing/Test.java @@ -67,6 +67,7 @@ import org.gradle.internal.Factory; import org.gradle.internal.actor.ActorFactory; import org.gradle.internal.concurrent.CompositeStoppable; +import org.gradle.internal.deprecation.DeprecationLogger; import org.gradle.internal.jvm.DefaultModularitySpec; import org.gradle.internal.jvm.JavaModuleDetector; import org.gradle.internal.jvm.Jvm; @@ -169,6 +170,7 @@ public class Test extends AbstractTestTask implements JavaForkOptions, PatternFi private final ConfigurableFileCollection stableClasspath; private final Property testFramework; private boolean userHasConfiguredTestFramework; + private boolean optionsAccessed; private boolean scanForTestClasses = true; private long forkEvery; @@ -918,8 +920,9 @@ public TestFramework testFramework(@Nullable Closure testFrameworkConfigure) { // multiple times for a single task--either switching between frameworks or overwriting // the existing configuration for a test framework--we need to keep track if the user has // explicitly set a test framework - // With test suites, users should never need to call the useXXX methods, so we can forbid - // them from doing something like this from the beginning. + // With test suites, users should never need to call the useXXX methods, so we can warn if + // them from doing something like this (for now, in order to preserve existing builds). + // This behavior should be restored to fail fast once again with the next major version. // // testTask.configure { // options { @@ -933,9 +936,11 @@ public TestFramework testFramework(@Nullable Closure testFrameworkConfigure) { // } // } - if (!userHasConfiguredTestFramework) { - testFramework.finalizeValue(); - } + // TODO: Test Framework Selection - Restore this to re-enable fail-fast behavior for Gradle 8 +// if (!userHasConfiguredTestFramework) { +// testFramework.finalizeValue(); +// } + return testFramework.get(); } @@ -944,13 +949,17 @@ public TestFramework testFramework(@Nullable Closure testFrameworkConfigure) { * * @return The test framework options. */ - @Internal + @Nested public TestFrameworkOptions getOptions() { + optionsAccessed = true; return getTestFramework().getOptions(); } /** * Configures test framework specific options. Make sure to call {@link #useJUnit()}, {@link #useJUnitPlatform()} or {@link #useTestNG()} before using this method. + *

+ * Any previous option configuration will be DISCARDED upon changing the test framework. Accessing options prior to setting the test framework will be + * deprecated in Gradle 8. * * @return The test framework options. */ @@ -975,6 +984,18 @@ TestFramework useTestFramework(TestFramework testFramework) { } private TestFramework useTestFramework(TestFramework testFramework, @Nullable Action testFrameworkConfigure) { + if (optionsAccessed) { + DeprecationLogger.deprecateAction("Accessing test options prior to setting test framework") + .withContext("\nTest options have already been accessed for task: '" + getProject().getName() + ":" + getName() + "' prior to setting the test framework to: '" + testFramework.getClass().getSimpleName() + "'. Any previous configuration will be discarded.\n") + .willBeRemovedInGradle8() + .withDslReference(Test.class, "options") + .nagUser(); + + if (!this.testFramework.get().getClass().equals(testFramework.getClass())) { + getLogger().warn("Test framework is changing from '{}', previous option configuration would not be applicable.", this.testFramework.get().getClass().getSimpleName()); + } + } + userHasConfiguredTestFramework = true; this.testFramework.set(testFramework);