diff --git a/README.md b/README.md index 0782b0f07..3cee64a38 100644 --- a/README.md +++ b/README.md @@ -378,3 +378,5 @@ Configures some sensible defaults: 2. If Gradle detects you use JUnit 5 (i.e. you have a `testImplementation 'org:junit.jupiter:junit-jupiter'` dependency), it will automatically configure your `Test` tasks to run with `useJUnitPlatform()`, and configure all `@Test` methods to run in parallel by default. Many other languages take this stance by default - if some tests rely on static state then you can mark them as non-parallel. See more here: https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution + +The plugin also adds a `checkJUnitDependencies` to make the migration to JUnit5 safer. Specifically, it should prevent cases where the tests could silently not run due to misconfigured dependencies. diff --git a/changelog/@unreleased/pr-837.v2.yml b/changelog/@unreleased/pr-837.v2.yml new file mode 100644 index 000000000..7e0c12b3a --- /dev/null +++ b/changelog/@unreleased/pr-837.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: A new `checkJUnitDependencies` task detects misconfigured JUnit dependencies + which could result in some tests silently not running. + links: + - https://github.com/palantir/gradle-baseline/pull/837 diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineTesting.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineTesting.java index 7b70e6bac..a5b9cff39 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineTesting.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineTesting.java @@ -16,7 +16,9 @@ package com.palantir.baseline.plugins; +import com.palantir.baseline.tasks.CheckJUnitDependencies; import java.util.Objects; +import java.util.Optional; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.artifacts.Dependency; @@ -24,8 +26,8 @@ import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.specs.Spec; import org.gradle.api.tasks.SourceSet; +import org.gradle.api.tasks.TaskProvider; import org.gradle.api.tasks.testing.Test; -import org.gradle.api.tasks.testing.TestFrameworkOptions; import org.gradle.api.tasks.testing.junitplatform.JUnitPlatformOptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,34 +47,46 @@ public void apply(Project project) { } }); - project.getPlugins().withType(JavaPlugin.class, unused -> { + project.getPlugins().withType(JavaPlugin.class, unusedPlugin -> { // afterEvaluate necessary because the junit-jupiter dep might be added further down the build.gradle project.afterEvaluate(proj -> { proj.getConvention() .getPlugin(JavaPluginConvention.class) .getSourceSets() - .matching(ss -> hasCompileDependenciesMatching(proj, ss, this::isJunitJupiter)) + .matching(ss -> hasCompileDependenciesMatching(proj, ss, BaselineTesting::isJunitJupiter)) .forEach(ss -> { - String testTaskName = ss.getTaskName(null, "test"); - Test testTask = (Test) proj.getTasks().findByName(testTaskName); - if (testTask == null) { - // Fall back to the source set name, since that is what gradle-testsets-plugin does - testTask = (Test) proj.getTasks().findByName(ss.getName()); - if (testTask == null) { - log.warn( - "Detected 'org:junit.jupiter:junit-jupiter', but unable to find test task"); - return; - } + Optional maybeTestTask = getTestTaskForSourceSet(proj, ss); + if (!maybeTestTask.isPresent()) { + log.warn("Detected 'org:junit.jupiter:junit-jupiter', but unable to find test task"); + return; } log.info("Detected 'org:junit.jupiter:junit-jupiter', enabling useJUnitPlatform() on {}", - testTask.getName()); - enableJunit5ForTestTask(testTask); + maybeTestTask.get().getName()); + enableJunit5ForTestTask(maybeTestTask.get()); }); }); + + TaskProvider task = project.getTasks() + .register("checkJUnitDependencies", CheckJUnitDependencies.class); + + project.getTasks().findByName(JavaPlugin.TEST_TASK_NAME).dependsOn(task); }); } - private boolean hasCompileDependenciesMatching(Project project, SourceSet sourceSet, Spec spec) { + public static Optional getTestTaskForSourceSet(Project proj, SourceSet ss) { + String testTaskName = ss.getTaskName(null, "test"); + + Test task1 = (Test) proj.getTasks().findByName(testTaskName); + if (task1 != null) { + return Optional.of(task1); + } + + // unbroken dome does this + Test task2 = (Test) proj.getTasks().findByName(ss.getName()); + return Optional.ofNullable(task2); + } + + private static boolean hasCompileDependenciesMatching(Project project, SourceSet sourceSet, Spec spec) { return project.getConfigurations() .getByName(sourceSet.getCompileClasspathConfigurationName()) .getAllDependencies() @@ -82,14 +96,12 @@ private boolean hasCompileDependenciesMatching(Project project, SourceSet source .isPresent(); } - private boolean isJunitJupiter(Dependency dep) { - return Objects.equals(dep.getGroup(), "org.junit.jupiter") - && dep.getName().equals("junit-jupiter"); + private static boolean isJunitJupiter(Dependency dep) { + return Objects.equals(dep.getGroup(), "org.junit.jupiter") && dep.getName().equals("junit-jupiter"); } - private void enableJunit5ForTestTask(Test task) { - TestFrameworkOptions options = task.getOptions(); - if (!(options instanceof JUnitPlatformOptions)) { + private static void enableJunit5ForTestTask(Test task) { + if (!useJUnitPlatformEnabled(task)) { task.useJUnitPlatform(); } @@ -105,4 +117,8 @@ private void enableJunit5ForTestTask(Test task) { // provide some stdout feedback when tests fail task.testLogging(testLogging -> testLogging.events("failed")); } + + public static boolean useJUnitPlatformEnabled(Test task) { + return task.getOptions() instanceof JUnitPlatformOptions; + } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckJUnitDependencies.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckJUnitDependencies.java new file mode 100644 index 000000000..3b679425a --- /dev/null +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckJUnitDependencies.java @@ -0,0 +1,171 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * 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 com.palantir.baseline.tasks; + +import com.google.common.base.Preconditions; +import com.palantir.baseline.plugins.BaselineTesting; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.Optional; +import java.util.function.Predicate; +import java.util.stream.Stream; +import org.gradle.api.DefaultTask; +import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.gradle.api.plugins.JavaPluginConvention; +import org.gradle.api.tasks.SourceSet; +import org.gradle.api.tasks.TaskAction; +import org.gradle.api.tasks.testing.Test; + +public class CheckJUnitDependencies extends DefaultTask { + + public CheckJUnitDependencies() { + setGroup("Verification"); + setDescription("Ensures the correct JUnit4/5 dependencies are present, otherwise tests may silently not run"); + } + + @TaskAction + public final void validateDependencies() { + getProject().getConvention() + .getPlugin(JavaPluginConvention.class) + .getSourceSets() + .forEach(ss -> { + if (ss.getName().equals("main")) { + return; + } + + Optional maybeTestTask = BaselineTesting.getTestTaskForSourceSet(getProject(), ss); + if (!maybeTestTask.isPresent()) { + // source set doesn't have a test task, e.g. 'schema' + return; + } + Test task = maybeTestTask.get(); + + getProject().getLogger().info( + "Analyzing source set {} with task {}", + ss.getName(), task.getName()); + validateSourceSet(ss, task); + }); + } + + private void validateSourceSet(SourceSet ss, Test task) { + boolean junitJupiterIsPresent = hasDep( + ss.getRuntimeClasspathConfigurationName(), CheckJUnitDependencies::isJunitJupiter); + + // If some testing library happens to provide the junit-jupiter-api, then users might start using the + // org.junit.jupiter.api.Test annotation, but as JUnit4 knows nothing about these, they'll silently not run + // unless the user has wired up the dependency correctly. + if (sourceSetMentionsJUnit5Api(ss)) { + String implementation = ss.getImplementationConfigurationName(); + Preconditions.checkState( + BaselineTesting.useJUnitPlatformEnabled(task), + "Some tests mention JUnit5, but the '" + task.getName() + "' task does not have " + + "useJUnitPlatform() enabled. This means tests may be silently not running! Please " + + "add the following:\n\n" + + " " + implementation + " 'org.junit.jupiter:junit-jupiter'\n"); + } + + // When doing an incremental migration to JUnit5, a project may have some JUnit4 and some JUnit5 tests at the + // same time. It's crucial that they have the vintage engine set up correctly, otherwise tests may silently + // not run! + if (sourceSetMentionsJUnit4(ss)) { + if (BaselineTesting.useJUnitPlatformEnabled(task)) { // people might manually enable this + String testRuntimeOnly = ss.getRuntimeConfigurationName() + "Only"; + Preconditions.checkState( + junitJupiterIsPresent, + "Tests may be silently not running! Some tests still use JUnit4, but Gradle has " + + "been set to use JUnit Platform. " + + "To ensure your old JUnit4 tests still run, please add the following:\n\n" + + " " + testRuntimeOnly + " 'org.junit.jupiter:junit-jupiter'\n\n" + + "Otherwise they will silently not run."); + + boolean vintageEngineExists = hasDep( + ss.getRuntimeClasspathConfigurationName(), CheckJUnitDependencies::isVintageEngine); + Preconditions.checkState( + vintageEngineExists, + "Tests may be silently not running! Some tests still use JUnit4, but Gradle has " + + "been set to use JUnit Platform. " + + "To ensure your old JUnit4 tests still run, please add the following:\n\n" + + " " + testRuntimeOnly + " 'org.junit.vintage:junit-vintage-engine'\n\n" + + "Otherwise they will silently not run."); + } else { + Preconditions.checkState( + !junitJupiterIsPresent, + "Tests may be silently not running! Please remove " + + "'org.junit.jupiter:junit-jupiter' dependency " + + "because tests use JUnit4 and useJUnitPlatform() is not enabled."); + } + } else { + String compileClasspath = ss.getCompileClasspathConfigurationName(); + boolean compilingAgainstOldJunit = hasDep(compileClasspath, CheckJUnitDependencies::isJunit4); + Preconditions.checkState( + !compilingAgainstOldJunit, + "Extraneous dependency on JUnit4 (no test mentions JUnit4 classes). Please exclude " + + "this from compilation to ensure developers don't accidentally re-introduce it, e.g.\n\n" + + " configurations." + compileClasspath + ".exclude module: 'junit'\n\n"); + } + + // sourcesets might also contain Spock classes, but we don't have any special validation for these. + } + + private boolean hasDep(String configurationName, Predicate spec) { + return getProject().getConfigurations() + .getByName(configurationName) + .getIncoming() + .getResolutionResult() + .getAllComponents() + .stream() + .anyMatch(component -> spec.test(component.getModuleVersion())); + } + + private boolean sourceSetMentionsJUnit4(SourceSet ss) { + return !ss.getAllSource() + .filter(file -> fileContainsSubstring(file, l -> + l.contains("org.junit.Test") + || l.contains("org.junit.runner") + || l.contains("org.junit.ClassRule"))) + .isEmpty(); + } + + private boolean sourceSetMentionsJUnit5Api(SourceSet ss) { + return !ss.getAllSource() + .filter(file -> fileContainsSubstring(file, l -> l.contains("org.junit.jupiter.api."))) + .isEmpty(); + } + + private boolean fileContainsSubstring(File file, Predicate substring) { + try (Stream lines = Files.lines(file.toPath())) { + boolean hit = lines.anyMatch(substring::test); + getProject().getLogger().debug("[{}] {}", hit ? "hit" : "miss", file); + return hit; + } catch (IOException e) { + throw new RuntimeException("Unable to check file " + file, e); + } + } + + private static boolean isJunitJupiter(ModuleVersionIdentifier dep) { + return "org.junit.jupiter".equals(dep.getGroup()) && "junit-jupiter".equals(dep.getName()); + } + + private static boolean isVintageEngine(ModuleVersionIdentifier dep) { + return "org.junit.vintage".equals(dep.getGroup()) && "junit-vintage-engine".equals(dep.getName()); + } + + private static boolean isJunit4(ModuleVersionIdentifier dep) { + return "junit".equals(dep.getGroup()) && "junit".equals(dep.getName()); + } +} diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineTestingIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineTestingIntegrationTest.groovy index d41ee6205..69b6a0cd8 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineTestingIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineTestingIntegrationTest.groovy @@ -104,4 +104,63 @@ class BaselineTestingIntegrationTest extends AbstractPluginTest { result.task(':integrationTest').outcome == TaskOutcome.SUCCESS new File(projectDir, "build/reports/tests/integrationTest/classes/test.TestClass5.html").exists() } + + def 'checkJUnitDependencies ensures mixture of junit4 and 5 tests => legacy must be present'() { + when: + buildFile << ''' + plugins { + id 'org.unbroken-dome.test-sets' version '2.1.1' + } + '''.stripIndent() + buildFile << standardBuildFile + buildFile << ''' + testSets { + integrationTest + } + + dependencies { + integrationTestImplementation "org.junit.jupiter:junit-jupiter:5.4.2" + } + '''.stripIndent() + file('src/integrationTest/java/test/TestClass2.java') << junit4Test + file('src/integrationTest/java/test/TestClass5.java') << junit5Test + + then: + BuildResult result = with('checkJUnitDependencies', '--write-locks').buildAndFail() + result.output.contains 'Some tests still use JUnit4, but Gradle has been set to use JUnit Platform' + } + + def 'checkJUnitDependencies ensures mixture of junit4 and 5 tests => new must be present'() { + when: + buildFile << standardBuildFile + buildFile << ''' + dependencies { + testCompile "junit:junit:4.12" + } + '''.stripIndent() + file('src/test/java/test/TestClass2.java') << junit4Test + file('src/test/java/test/TestClass5.java') << junit5Test + + then: + BuildResult result = with('checkJUnitDependencies', '--write-locks').buildAndFail() + result.output.contains 'Some tests mention JUnit5, but the \'test\' task does not have useJUnitPlatform() enabled' + } + + def 'checkJUnitDependencies detects extraneous old junit'() { + when: + buildFile << standardBuildFile + buildFile << ''' + dependencies { + testCompile "junit:junit:4.12" + } + test { + useJUnitPlatform() + } + '''.stripIndent() + file('src/test/java/test/TestClass5.java') << junit5Test + + then: + BuildResult result = with('checkJUnitDependencies', '--write-locks').buildAndFail() + result.output.contains 'Extraneous dependency on JUnit4 (no test mentions JUnit4 classes)' + } }