From 4b1028ee9e9f98aa8856831176fe188b39c4eb36 Mon Sep 17 00:00:00 2001 From: Justin Van Dort Date: Tue, 14 Jun 2022 00:21:08 -0400 Subject: [PATCH] Remove --add-opens for test workers * Update ClassLoaderUtils to use public JDK APIs when listing a ClassLoader's package * Disable adding any --add-opens flags to test worker processes * Add tests to verify production code and test code behavior is equivilent * Verify users can opt back into previous behavior by adding --add-opens flags manually to test jvm args * Improved multi-jdk support for TestNG Tests * Add release notes and upgrade guide Addressed action items --- .../classloader/ClassLoaderUtils.java | 32 ++++-- .../gradle/api/ApplyPluginIntegSpec.groovy | 18 +++ ...rojectBuilderEndUserIntegrationTest.groovy | 16 +++ .../worker/DefaultWorkerProcessBuilder.java | 32 ++++-- .../worker/DefaultWorkerProcessFactory.java | 13 +-- .../internal/worker/WorkerProcessBuilder.java | 18 +++ ...lassLoaderWorkerImplementationFactory.java | 28 ++--- .../child/WorkerImplementationFactory.java | 31 ------ .../DefaultWorkerProcessBuilderSpec.groovy | 7 +- subprojects/docs/src/docs/release/notes.md | 8 ++ .../migration/upgrading_version_7.adoc | 48 ++++++++ .../groovy/task/build.gradle | 17 +++ .../kotlin/task/build.gradle.kts | 14 +++ .../customPlugin/groovy/plugin/build.gradle | 17 +++ .../kotlin/plugin/build.gradle.kts | 14 +++ .../CustomPluginIntegrationTest.groovy | 32 ++++++ ...ContextualMultiVersionTestInterceptor.java | 12 +- .../BaseGradleImplDepsIntegrationTest.groovy | 22 ++++ ...mplDepsShadingIssuesIntegrationTest.groovy | 4 +- ...leImplDepsVisibilityIntegrationTest.groovy | 2 +- .../devel/plugins/JavaGradlePluginPlugin.java | 35 +++++- ...pathInjectionEndUserIntegrationTest.groovy | 4 +- .../worker/ForkingTestClassProcessor.java | 5 +- .../ForkingTestClassProcessorTest.groovy | 1 + .../testing/fixture/TestNGCoverage.groovy | 46 ++++++-- ...bstractTestNGVersionIntegrationTest.groovy | 5 +- .../testng/TestNGClassIntegrationTest.groovy | 4 +- .../TestNGFilteringIntegrationTest.groovy | 2 +- ...stNGGroupByInstancesIntegrationTest.groovy | 2 +- .../testng/TestNGIntegrationTest.groovy | 2 +- ...LoggingOutputCaptureIntegrationTest.groovy | 49 +++++---- .../TestNGParallelSuiteIntegrationTest.groovy | 2 +- .../TestNGPreserveOrderIntegrationTest.groovy | 2 +- ...GSuiteInitialisationIntegrationTest.groovy | 13 +-- .../testng/TestNGSuiteIntegrationTest.groovy | 2 +- .../org/gradle/api/tasks/testing/Test.java | 3 +- ...ReflectionTestWorkerIntegrationTest.groovy | 104 ++++++++++++++++++ 37 files changed, 511 insertions(+), 155 deletions(-) delete mode 100644 subprojects/core/src/main/java/org/gradle/process/internal/worker/child/WorkerImplementationFactory.java create mode 100644 subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/JdkIllegalReflectionTestWorkerIntegrationTest.groovy diff --git a/subprojects/base-services/src/main/java/org/gradle/internal/classloader/ClassLoaderUtils.java b/subprojects/base-services/src/main/java/org/gradle/internal/classloader/ClassLoaderUtils.java index 291507c25067..80b997d44bc3 100644 --- a/subprojects/base-services/src/main/java/org/gradle/internal/classloader/ClassLoaderUtils.java +++ b/subprojects/base-services/src/main/java/org/gradle/internal/classloader/ClassLoaderUtils.java @@ -36,7 +36,7 @@ public abstract class ClassLoaderUtils { static { CLASS_DEFINER = JavaVersion.current().isJava9Compatible() ? new LookupClassDefiner() : new ReflectionClassDefiner(); - CLASS_LOADER_PACKAGES_FETCHER = JavaVersion.current().isJava9Compatible() ? new LookupPackagesFetcher() : new ReflectionPackagesFetcher(); + CLASS_LOADER_PACKAGES_FETCHER = JavaVersion.current().isJava9Compatible() ? new Java9PackagesFetcher() : new ReflectionPackagesFetcher(); } /** @@ -51,6 +51,8 @@ public static void tryClose(@Nullable ClassLoader classLoader) { CompositeStoppable.stoppable(classLoader).stop(); } + // Used by the Gradle Play Framework Plugin. See: + // https://github.com/gradle/playframework/blob/master/src/main/java/org/gradle/playframework/tools/internal/run/PlayWorkerServer.java#L72 public static void disableUrlConnectionCaching() { // fix problems in updating jar files by disabling default caching of URL connections. // URLConnection default caching should be disabled since it causes jar file locking issues and JVM crashes in updating jar files. @@ -69,6 +71,7 @@ static Package[] getPackages(ClassLoader classLoader) { return CLASS_LOADER_PACKAGES_FETCHER.getPackages(classLoader); } + @Nullable static Package getPackage(ClassLoader classLoader, String name) { return CLASS_LOADER_PACKAGES_FETCHER.getPackage(classLoader, name); } @@ -120,6 +123,7 @@ private interface ClassDefiner { private interface ClassLoaderPackagesFetcher { Package[] getPackages(ClassLoader classLoader); + @Nullable Package getPackage(ClassLoader classLoader, String name); } @@ -188,8 +192,8 @@ private static class LookupClassDefiner extends AbstractClassLoaderLookuper impl @SuppressWarnings("unchecked") public Class defineDecoratorClass(Class decoratedClass, ClassLoader classLoader, String className, byte[] classBytes) { try { - // Lookup.defineClass can only define a class into same classloader as the lookup object - // we have to use the fallback defineClass() if they're not same, which is the case of ManagedProxyClassGenerator + // Lookup.defineClass can only define a class into same classloader as the lookup object. + // We have to use the fallback defineClass() if they're not same, which is the case of ManagedProxyClassGenerator if (decoratedClass.getClassLoader() == classLoader) { MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(decoratedClass, baseLookup); return (Class) lookup.defineClass(classBytes); @@ -216,24 +220,36 @@ public Package[] getPackages(ClassLoader classLoader) { return GET_PACKAGES_METHOD.invoke(classLoader); } + @Nullable @Override public Package getPackage(ClassLoader classLoader, String name) { return GET_PACKAGE_METHOD.invoke(classLoader, name); } } - private static class LookupPackagesFetcher extends AbstractClassLoaderLookuper implements ClassLoaderPackagesFetcher { - private MethodType getPackagesMethodType = MethodType.methodType(Package[].class, new Class[]{}); - private MethodType getDefinedPackageMethodType = MethodType.methodType(Package.class, new Class[]{String.class}); + private static class Java9PackagesFetcher implements ClassLoaderPackagesFetcher { @Override public Package[] getPackages(ClassLoader classLoader) { - return invoke(classLoader, "getPackages", getPackagesMethodType); + return new ClassLoader(classLoader) { + @Override + protected Package[] getPackages() { + return super.getPackages(); + } + }.getPackages(); } + @Nullable @Override public Package getPackage(ClassLoader classLoader, String name) { - return invoke(classLoader, "getPackage", getDefinedPackageMethodType, name); + return new ClassLoader(classLoader) { + // TODO: This is deprecated for a reason. We should consider migrating if possible. + @Override + @SuppressWarnings("deprecation") + protected Package getPackage(String name) { + return super.getPackage(name); + } + }.getPackage(name); } } } diff --git a/subprojects/core/src/integTest/groovy/org/gradle/api/ApplyPluginIntegSpec.groovy b/subprojects/core/src/integTest/groovy/org/gradle/api/ApplyPluginIntegSpec.groovy index d678a1313ba2..2f8078d77125 100644 --- a/subprojects/core/src/integTest/groovy/org/gradle/api/ApplyPluginIntegSpec.groovy +++ b/subprojects/core/src/integTest/groovy/org/gradle/api/ApplyPluginIntegSpec.groovy @@ -107,6 +107,24 @@ class ApplyPluginIntegSpec extends AbstractIntegrationSpec { and: buildFile << junitBasedBuildScript() + buildFile << """ + // Needed when using ProjectBuilder + class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } + } + tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) + } + """ expect: executer.withArgument("--info") diff --git a/subprojects/core/src/integTest/groovy/org/gradle/testfixtures/ProjectBuilderEndUserIntegrationTest.groovy b/subprojects/core/src/integTest/groovy/org/gradle/testfixtures/ProjectBuilderEndUserIntegrationTest.groovy index 281d9efef727..36da28bf620b 100644 --- a/subprojects/core/src/integTest/groovy/org/gradle/testfixtures/ProjectBuilderEndUserIntegrationTest.groovy +++ b/subprojects/core/src/integTest/groovy/org/gradle/testfixtures/ProjectBuilderEndUserIntegrationTest.groovy @@ -44,6 +44,22 @@ class ProjectBuilderEndUserIntegrationTest extends AbstractIntegrationSpec { testLogging.exceptionFormat = 'full' } + // Needed when using ProjectBuilder + class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } + } + tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) + } """ } diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessBuilder.java b/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessBuilder.java index 82d595a15de9..ad809322ad01 100644 --- a/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessBuilder.java +++ b/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessBuilder.java @@ -19,7 +19,9 @@ import org.gradle.api.Action; import org.gradle.api.logging.LogLevel; import org.gradle.internal.id.IdGenerator; +import org.gradle.internal.jvm.JpmsConfiguration; import org.gradle.internal.jvm.Jvm; +import org.gradle.internal.jvm.inspection.JvmVersionDetector; import org.gradle.internal.logging.events.OutputEventListener; import org.gradle.internal.remote.Address; import org.gradle.internal.remote.ConnectionAcceptor; @@ -60,16 +62,19 @@ public class DefaultWorkerProcessBuilder implements WorkerProcessBuilder { private final Set applicationModulePath = new LinkedHashSet<>(); private final MemoryManager memoryManager; + private final JvmVersionDetector jvmVersionDetector; + private Action action; private LogLevel logLevel = LogLevel.LIFECYCLE; private String baseName = "Gradle Worker"; - private File gradleUserHomeDir; private int connectTimeoutSeconds; private List implementationClassPath; private List implementationModulePath; private boolean shouldPublishJvmMemoryInfo; - DefaultWorkerProcessBuilder(JavaExecHandleFactory execHandleFactory, MessagingServer server, IdGenerator idGenerator, ApplicationClassesInSystemClassLoaderWorkerImplementationFactory workerImplementationFactory, OutputEventListener outputEventListener, MemoryManager memoryManager) { + private boolean useLegacyAddOpens = true; + + DefaultWorkerProcessBuilder(JavaExecHandleFactory execHandleFactory, MessagingServer server, IdGenerator idGenerator, ApplicationClassesInSystemClassLoaderWorkerImplementationFactory workerImplementationFactory, OutputEventListener outputEventListener, MemoryManager memoryManager, JvmVersionDetector jvmVersionDetector) { this.javaCommand = execHandleFactory.newJavaExec(); this.javaCommand.setExecutable(Jvm.current().getJavaExecutable()); this.server = server; @@ -77,6 +82,7 @@ public class DefaultWorkerProcessBuilder implements WorkerProcessBuilder { this.workerImplementationFactory = workerImplementationFactory; this.outputEventListener = outputEventListener; this.memoryManager = memoryManager; + this.jvmVersionDetector = jvmVersionDetector; } public int getConnectTimeoutSeconds() { @@ -174,14 +180,6 @@ public WorkerProcessBuilder setLogLevel(LogLevel logLevel) { return this; } - public File getGradleUserHomeDir() { - return gradleUserHomeDir; - } - - public void setGradleUserHomeDir(File gradleUserHomeDir) { - this.gradleUserHomeDir = gradleUserHomeDir; - } - @Override public void setImplementationClasspath(List implementationClassPath) { this.implementationClassPath = implementationClassPath; @@ -197,6 +195,13 @@ public void enableJvmMemoryInfoPublishing(boolean shouldPublish) { this.shouldPublishJvmMemoryInfo = shouldPublish; } + @Deprecated + @Override + public WorkerProcessBuilder setUseLegacyAddOpens(boolean useLegacyAddOpens) { + this.useLegacyAddOpens = useLegacyAddOpens; + return this; + } + @Override public WorkerProcess build() { final WorkerJvmMemoryStatus memoryStatus = shouldPublishJvmMemoryInfo ? new WorkerJvmMemoryStatus() : null; @@ -234,7 +239,12 @@ public void run() { JavaExecHandleBuilder javaCommand = getJavaCommand(); javaCommand.setDisplayName(displayName); - workerImplementationFactory.prepareJavaCommand(id, displayName, this, implementationClassPath, implementationModulePath, localAddress, javaCommand, shouldPublishJvmMemoryInfo); + boolean java9Compatible = jvmVersionDetector.getJavaVersion(javaCommand.getExecutable()).isJava9Compatible(); + if (java9Compatible && useLegacyAddOpens) { + javaCommand.jvmArgs(JpmsConfiguration.GRADLE_WORKER_JPMS_ARGS); + } + + workerImplementationFactory.prepareJavaCommand(id, displayName, this, implementationClassPath, implementationModulePath, localAddress, javaCommand, shouldPublishJvmMemoryInfo, java9Compatible); javaCommand.args("'" + displayName + "'"); if (javaCommand.getMaxHeapSize() == null) { diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessFactory.java b/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessFactory.java index d248020639ec..fe97f5ca7695 100644 --- a/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessFactory.java +++ b/subprojects/core/src/main/java/org/gradle/process/internal/worker/DefaultWorkerProcessFactory.java @@ -36,8 +36,8 @@ public class DefaultWorkerProcessFactory implements WorkerProcessFactory { private final LoggingManager loggingManager; private final MessagingServer server; private final IdGenerator idGenerator; - private final File gradleUserHomeDir; private final JavaExecHandleFactory execHandleFactory; + private final JvmVersionDetector jvmVersionDetector; private final OutputEventListener outputEventListener; private final ApplicationClassesInSystemClassLoaderWorkerImplementationFactory workerImplementationFactory; private final MemoryManager memoryManager; @@ -49,10 +49,10 @@ public DefaultWorkerProcessFactory(LoggingManager loggingManager, MessagingServe this.loggingManager = loggingManager; this.server = server; this.idGenerator = idGenerator; - this.gradleUserHomeDir = gradleUserHomeDir; this.execHandleFactory = execHandleFactory; + this.jvmVersionDetector = jvmVersionDetector; this.outputEventListener = outputEventListener; - this.workerImplementationFactory = new ApplicationClassesInSystemClassLoaderWorkerImplementationFactory(classPathRegistry, temporaryFileProvider, jvmVersionDetector, gradleUserHomeDir); + this.workerImplementationFactory = new ApplicationClassesInSystemClassLoaderWorkerImplementationFactory(classPathRegistry, temporaryFileProvider, gradleUserHomeDir); this.memoryManager = memoryManager; } @@ -70,18 +70,17 @@ public WorkerProcessBuilder create(Action workerAc @Override public SingleRequestWorkerProcessBuilder singleRequestWorker(Class> workerImplementation) { - return new DefaultSingleRequestWorkerProcessBuilder(workerImplementation, newWorkerProcessBuilder(), outputEventListener); + return new DefaultSingleRequestWorkerProcessBuilder<>(workerImplementation, newWorkerProcessBuilder(), outputEventListener); } @Override public MultiRequestWorkerProcessBuilder multiRequestWorker(Class> workerImplementation) { - return new DefaultMultiRequestWorkerProcessBuilder(workerImplementation, newWorkerProcessBuilder(), outputEventListener); + return new DefaultMultiRequestWorkerProcessBuilder<>(workerImplementation, newWorkerProcessBuilder(), outputEventListener); } private DefaultWorkerProcessBuilder newWorkerProcessBuilder() { - DefaultWorkerProcessBuilder builder = new DefaultWorkerProcessBuilder(execHandleFactory, server, idGenerator, workerImplementationFactory, outputEventListener, memoryManager); + DefaultWorkerProcessBuilder builder = new DefaultWorkerProcessBuilder(execHandleFactory, server, idGenerator, workerImplementationFactory, outputEventListener, memoryManager, jvmVersionDetector); builder.setLogLevel(loggingManager.getLevel()); - builder.setGradleUserHomeDir(gradleUserHomeDir); builder.setConnectTimeoutSeconds(connectTimeoutSeconds); return builder; } diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/worker/WorkerProcessBuilder.java b/subprojects/core/src/main/java/org/gradle/process/internal/worker/WorkerProcessBuilder.java index c15643321123..db5ab8d6d121 100644 --- a/subprojects/core/src/main/java/org/gradle/process/internal/worker/WorkerProcessBuilder.java +++ b/subprojects/core/src/main/java/org/gradle/process/internal/worker/WorkerProcessBuilder.java @@ -59,6 +59,24 @@ public interface WorkerProcessBuilder extends WorkerProcessSettings { void enableJvmMemoryInfoPublishing(boolean shouldPublish); + /** + * Specify whether the {@code --add-opens} command line args specified by + * {@link org.gradle.internal.jvm.JpmsConfiguration#GRADLE_WORKER_JPMS_ARGS} + * should be used for the process-to-build. + *

+ * Note: This option will be removed in Gradle 8.0 so that no workers use + * these args by default. We are leaving these args enabled by default for + * most worker types in order to reduce breaking changes for those using + * reflection already in their workers already. In 8.0, we will remove these + * args entirely from all workers. + * + * @see 8.0 Removal Issue + * + * @param useLegacyAddOpens Whether to use the add-opens args. + */ + @Deprecated + WorkerProcessBuilder setUseLegacyAddOpens(boolean useLegacyAddOpens); + /** * Creates the worker process. The process is not started until {@link WorkerProcess#start()} is called. * diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/ApplicationClassesInSystemClassLoaderWorkerImplementationFactory.java b/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/ApplicationClassesInSystemClassLoaderWorkerImplementationFactory.java index a381709e23e4..6ff828a5b0e4 100755 --- a/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/ApplicationClassesInSystemClassLoaderWorkerImplementationFactory.java +++ b/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/ApplicationClassesInSystemClassLoaderWorkerImplementationFactory.java @@ -17,14 +17,11 @@ package org.gradle.process.internal.worker.child; import com.google.common.base.Joiner; -import org.gradle.api.JavaVersion; import org.gradle.api.UncheckedIOException; import org.gradle.api.internal.ClassPathRegistry; import org.gradle.api.internal.file.temp.TemporaryFileProvider; import org.gradle.api.logging.LogLevel; import org.gradle.internal.io.StreamByteBuffer; -import org.gradle.internal.jvm.JpmsConfiguration; -import org.gradle.internal.jvm.inspection.JvmVersionDetector; import org.gradle.internal.process.ArgWriter; import org.gradle.internal.remote.Address; import org.gradle.internal.remote.internal.inet.MultiChoiceAddress; @@ -69,22 +66,25 @@ * (ActionExecutionWorker + worker action implementation) * */ -public class ApplicationClassesInSystemClassLoaderWorkerImplementationFactory implements WorkerImplementationFactory { +public class ApplicationClassesInSystemClassLoaderWorkerImplementationFactory { private final ClassPathRegistry classPathRegistry; private final TemporaryFileProvider temporaryFileProvider; - private final JvmVersionDetector jvmVersionDetector; private final File gradleUserHomeDir; - public ApplicationClassesInSystemClassLoaderWorkerImplementationFactory(ClassPathRegistry classPathRegistry, TemporaryFileProvider temporaryFileProvider, - JvmVersionDetector jvmVersionDetector, File gradleUserHomeDir) { + public ApplicationClassesInSystemClassLoaderWorkerImplementationFactory( + ClassPathRegistry classPathRegistry, + TemporaryFileProvider temporaryFileProvider, + File gradleUserHomeDir + ) { this.classPathRegistry = classPathRegistry; this.temporaryFileProvider = temporaryFileProvider; - this.jvmVersionDetector = jvmVersionDetector; this.gradleUserHomeDir = gradleUserHomeDir; } - @Override - public void prepareJavaCommand(long workerId, String displayName, WorkerProcessBuilder processBuilder, List implementationClassPath, List implementationModulePath, Address serverAddress, JavaExecHandleBuilder execSpec, boolean publishProcessInfo) { + /** + * Configures the Java command that will be used to launch the child process. + */ + public void prepareJavaCommand(long workerId, String displayName, WorkerProcessBuilder processBuilder, List implementationClassPath, List implementationModulePath, Address serverAddress, JavaExecHandleBuilder execSpec, boolean publishProcessInfo, boolean useOptionsFile) { Collection applicationClasspath = processBuilder.getApplicationClasspath(); Set applicationModulePath = processBuilder.getApplicationModulePath(); LogLevel logLevel = processBuilder.getLogLevel(); @@ -98,8 +98,6 @@ public void prepareJavaCommand(long workerId, String displayName, WorkerProcessB execSpec.getMainModule().set("gradle.worker"); } execSpec.getMainClass().set("worker." + GradleWorkerMain.class.getName()); - - boolean useOptionsFile = shouldUseOptionsFile(execSpec); if (useOptionsFile) { // Use an options file to pass across application classpath File optionsFile = temporaryFileProvider.createTemporaryFile("gradle-worker-classpath", "txt"); @@ -170,11 +168,6 @@ public void prepareJavaCommand(long workerId, String displayName, WorkerProcessB execSpec.setStandardInput(buffer.getInputStream()); } - private boolean shouldUseOptionsFile(JavaExecHandleBuilder execSpec) { - JavaVersion executableVersion = jvmVersionDetector.getJavaVersion(execSpec.getExecutable()); - return executableVersion != null && executableVersion.isJava9Compatible(); - } - private List writeOptionsFile(boolean runAsModule, Collection workerMainClassPath, Collection implementationModulePath, Collection applicationClasspath, Set applicationModulePath, File optionsFile) { List classpath = new ArrayList<>(); List modulePath = new ArrayList<>(); @@ -205,7 +198,6 @@ private List writeOptionsFile(boolean runAsModule, Collection work if (!classpath.isEmpty()) { argumentList.addAll(Arrays.asList("-cp", Joiner.on(File.pathSeparator).join(classpath))); } - argumentList.addAll(JpmsConfiguration.GRADLE_WORKER_JPMS_ARGS); return ArgWriter.argsFileGenerator(optionsFile, ArgWriter.javaStyleFactory()).transform(argumentList); } } diff --git a/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/WorkerImplementationFactory.java b/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/WorkerImplementationFactory.java deleted file mode 100644 index 9391c62380cd..000000000000 --- a/subprojects/core/src/main/java/org/gradle/process/internal/worker/child/WorkerImplementationFactory.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2010 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.process.internal.worker.child; - -import org.gradle.internal.remote.Address; -import org.gradle.process.internal.JavaExecHandleBuilder; -import org.gradle.process.internal.worker.WorkerProcessBuilder; - -import java.net.URL; -import java.util.List; - -public interface WorkerImplementationFactory { - /** - * Configures the Java command that will be used to launch the child process. - */ - void prepareJavaCommand(long workerId, String displayName, WorkerProcessBuilder processBuilder, List implementationClassPath, List implementationModulePath, Address serverAddress, JavaExecHandleBuilder execSpec, boolean publishProcessInfo); -} diff --git a/subprojects/core/src/test/groovy/org/gradle/process/internal/worker/DefaultWorkerProcessBuilderSpec.groovy b/subprojects/core/src/test/groovy/org/gradle/process/internal/worker/DefaultWorkerProcessBuilderSpec.groovy index 73b5fe47092f..378b08cb147b 100644 --- a/subprojects/core/src/test/groovy/org/gradle/process/internal/worker/DefaultWorkerProcessBuilderSpec.groovy +++ b/subprojects/core/src/test/groovy/org/gradle/process/internal/worker/DefaultWorkerProcessBuilderSpec.groovy @@ -16,12 +16,13 @@ package org.gradle.process.internal.worker import org.gradle.internal.id.IdGenerator +import org.gradle.internal.jvm.inspection.JvmVersionDetector import org.gradle.internal.logging.events.OutputEventListener import org.gradle.internal.remote.MessagingServer +import org.gradle.process.internal.JavaExecHandleBuilder import org.gradle.process.internal.JavaExecHandleFactory import org.gradle.process.internal.health.memory.MemoryManager import org.gradle.process.internal.worker.child.ApplicationClassesInSystemClassLoaderWorkerImplementationFactory -import org.gradle.process.internal.JavaExecHandleBuilder import spock.lang.Specification import static org.junit.Assert.assertTrue @@ -39,12 +40,14 @@ class DefaultWorkerProcessBuilderSpec extends Specification { def applicationClassesInSystemClassLoaderWorkerImplementationFactory = Mock(ApplicationClassesInSystemClassLoaderWorkerImplementationFactory) def outputEventListener = Mock(OutputEventListener) def memoryManager = Mock(MemoryManager) + def versionDetector = Mock(JvmVersionDetector) DefaultWorkerProcessBuilder builder = new DefaultWorkerProcessBuilder(javaExecHandleFactory, messagingServer, idGenerator, applicationClassesInSystemClassLoaderWorkerImplementationFactory, outputEventListener, - memoryManager) + memoryManager, + versionDetector) def "validate entries in classpath"() { diff --git a/subprojects/docs/src/docs/release/notes.md b/subprojects/docs/src/docs/release/notes.md index 0329a45abf67..8ae4e3378aa1 100644 --- a/subprojects/docs/src/docs/release/notes.md +++ b/subprojects/docs/src/docs/release/notes.md @@ -287,6 +287,14 @@ This allows configuration of PMD to run its analysis on more than one thread. See the [documentation](userguide/pmd_plugin.html#sec:pmd_conf_threads) for more information. +### Better test compatibility with Java 9+ + +When running on Java 9+, Gradle no longer opens the `java.base/java.util` and `java.base/java.lang` JDK modules for all `Test` tasks. In some cases, this would cause code to pass during testing but fail at runtime. + +This change may cause new test failures and warnings. When running on Java 16+, code performing reflection on JDK internals will now fail tests. When running on Java 9-15, illegal access warnings will appear in logs. While this change may break some existing builds, most failures are likely to uncover suppressed issues which would have only been detected at runtime. + +For a detailed description on how to mitigate this change, please see the [upgrade guide for details](userguide/upgrading_version_7.html#removes_implicit_add_opens_for_test_workers). + ## Promoted features Promoted features are features that were incubating in previous versions of Gradle but are now supported and subject to backwards compatibility. See the User Manual section on the “[Feature Lifecycle](userguide/feature_lifecycle.html)” for more information. diff --git a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc index d28af8d376d0..6c2a4a98f4fd 100644 --- a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc +++ b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc @@ -154,6 +154,54 @@ It takes into account which methods are being used and which have changed, which Zinc version has been updated to the newest available one in order to benefit from all the recent bugfixes. Due to that, if you use `zincVersion` setting it's advised to remove it and only use the default version, because Gradle will only be able to compile Scala code with Zinc versions set to 1.6.x or higher. +==== Removes implicit `--add-opens` for test workers + +Prior to Gradle 7.5, JDK modules `java.base/java.util` and `java.base/java.lang` were automatically opened in test workers on JDK9+ by passing `--add-opens` CLI arguments. This meant any tests were able to perform deep reflection on JDK internals without warning or failing. This caused tests to be unreliable by allowing code to pass when it would otherwise fail in a production environment. + +These implicit arguments have been removed and are no longer added by default. If your code or any of your dependencies are performing deep refection into JDK internals during test execution, you may see the following behavior changes: + +Before Java 16, new build warnings are shown. These new warnings are printed to stderr and will not fail the build: +``` +WARNING: An illegal reflective access operation has occurred +WARNING: Illegal reflective access by com.google.inject.internal.cglib.core.ReflectUtils$2 (file:/.../testng-5.12.1.jar) to +WARNING: Please consider reporting this to the maintainers of com.google.inject.internal.cglib.core.ReflectUtils$2 +WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations +WARNING: All illegal access operations will be denied in a future release +``` + +With Java 16 or higher, exceptions are thrown that fail the build: +``` +// Thrown by TestNG +java.lang.reflect.InaccessibleObjectException: Unable to make accessible: module java.base does not "opens java.lang" to unnamed module @1e92bd61 + at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354) + at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297) + at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199) + at java.base/java.lang.reflect.Method.setAccessible(Method.java:193) + ... + +// Thrown by ProjectBuilder +org.gradle.api.GradleException: Could not inject synthetic classes. + at org.gradle.initialization.DefaultLegacyTypesSupport.injectEmptyInterfacesIntoClassLoader(DefaultLegacyTypesSupport.java:91) + at org.gradle.testfixtures.internal.ProjectBuilderImpl.getGlobalServices(ProjectBuilderImpl.java:182) + at org.gradle.testfixtures.internal.ProjectBuilderImpl.createProject(ProjectBuilderImpl.java:111) + at org.gradle.testfixtures.ProjectBuilder.build(ProjectBuilder.java:120) + ... +Caused by: java.lang.RuntimeException: java.lang.IllegalAccessException: module java.base does not open java.lang to unnamed module @1e92bd61 +``` + +In most cases, these errors can be resolved by updating the code or dependency performing the illegal access. If the code-under-test or the newest version of the dependency in question performs illegal access by design, the old behavior can be restored by opening the `java.base/java.lang` and `java.base/java.util` modules manually with `--add-opens`: + +``` +tasks.withType(Test).configureEach { + jvmArgs(["--add-opens=java.base/java.lang=ALL-UNNAMED", + "--add-opens=java.base/java.util=ALL-UNNAMED"] +} +``` + +If you are developing Gradle plugins, `ProjectBuilder` relies on reflection in the `java.base/java.lang` module. Gradle will automatically add the appropriate `--add-opens` flag to tests when the `java-gradle-plugin` plugin is applied. + +If you are using TestNG, versions prior to `5.14.6` perform illegal reflection. Updating to at least `5.14.6` should fix the incompatibility. + === Deprecations [[file_collection_to_classpath]] diff --git a/subprojects/docs/src/snippets/base/customExternalTask/groovy/task/build.gradle b/subprojects/docs/src/snippets/base/customExternalTask/groovy/task/build.gradle index 4bbdb29ba193..e43edcdf9220 100644 --- a/subprojects/docs/src/snippets/base/customExternalTask/groovy/task/build.gradle +++ b/subprojects/docs/src/snippets/base/customExternalTask/groovy/task/build.gradle @@ -34,3 +34,20 @@ publishing { } } } + +// Needed when using ProjectBuilder +class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } +} +tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) +} diff --git a/subprojects/docs/src/snippets/base/customExternalTask/kotlin/task/build.gradle.kts b/subprojects/docs/src/snippets/base/customExternalTask/kotlin/task/build.gradle.kts index 03a062e3851c..911c7afcd9b8 100644 --- a/subprojects/docs/src/snippets/base/customExternalTask/kotlin/task/build.gradle.kts +++ b/subprojects/docs/src/snippets/base/customExternalTask/kotlin/task/build.gradle.kts @@ -34,3 +34,17 @@ publishing { } } } + +// Needed when using ProjectBuilder +class AddOpensArgProvider(private val test: Test) : CommandLineArgumentProvider { + override fun asArguments() : Iterable { + return if (test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9)) { + listOf("--add-opens=java.base/java.lang=ALL-UNNAMED") + } else { + emptyList() + } + } +} +tasks.withType().configureEach { + jvmArgumentProviders.add(AddOpensArgProvider(this)) +} diff --git a/subprojects/docs/src/snippets/plugins/customPlugin/groovy/plugin/build.gradle b/subprojects/docs/src/snippets/plugins/customPlugin/groovy/plugin/build.gradle index 350232db5bb0..564a69cc0201 100644 --- a/subprojects/docs/src/snippets/plugins/customPlugin/groovy/plugin/build.gradle +++ b/subprojects/docs/src/snippets/plugins/customPlugin/groovy/plugin/build.gradle @@ -39,3 +39,20 @@ publishing { } } } + +// Needed when using ProjectBuilder +class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } +} +tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) +} diff --git a/subprojects/docs/src/snippets/plugins/customPlugin/kotlin/plugin/build.gradle.kts b/subprojects/docs/src/snippets/plugins/customPlugin/kotlin/plugin/build.gradle.kts index 47367fd4a2c4..ff6d0148bc12 100644 --- a/subprojects/docs/src/snippets/plugins/customPlugin/kotlin/plugin/build.gradle.kts +++ b/subprojects/docs/src/snippets/plugins/customPlugin/kotlin/plugin/build.gradle.kts @@ -39,3 +39,17 @@ publishing { } } } + +// Needed when using ProjectBuilder +class AddOpensArgProvider(private val test: Test) : CommandLineArgumentProvider { + override fun asArguments() : Iterable { + return if (test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9)) { + listOf("--add-opens=java.base/java.lang=ALL-UNNAMED") + } else { + emptyList() + } + } +} +tasks.withType().configureEach { + jvmArgumentProviders.add(AddOpensArgProvider(this)) +} diff --git a/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/CustomPluginIntegrationTest.groovy b/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/CustomPluginIntegrationTest.groovy index 21619ecad10e..3254d0427309 100755 --- a/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/CustomPluginIntegrationTest.groovy +++ b/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/CustomPluginIntegrationTest.groovy @@ -167,6 +167,22 @@ dependencies { implementation localGroovy() testImplementation 'junit:junit:4.13' } +// Needed when using ProjectBuilder +class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } +} +tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) +} """ expect: @@ -209,6 +225,22 @@ dependencies { implementation localGroovy() testImplementation 'junit:junit:4.13' } +// Needed when using ProjectBuilder +class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } +} +tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) +} """ expect: diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/compatibility/AbstractContextualMultiVersionTestInterceptor.java b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/compatibility/AbstractContextualMultiVersionTestInterceptor.java index e27b8ef38b0c..8fc11854f440 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/compatibility/AbstractContextualMultiVersionTestInterceptor.java +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/compatibility/AbstractContextualMultiVersionTestInterceptor.java @@ -18,15 +18,14 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import org.gradle.api.specs.Spec; import org.gradle.integtests.fixtures.VersionedTool; import org.gradle.integtests.fixtures.extensions.AbstractMultiTestInterceptor; -import org.gradle.util.internal.CollectionUtils; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import static com.google.common.collect.Iterators.getLast; @@ -76,14 +75,7 @@ protected Collection getPartialVersions() { } private Collection getAvailableVersions() { - Set allAvailable = Sets.newHashSet(); - CollectionUtils.filter(getAllVersions(), allAvailable, new Spec() { - @Override - public boolean isSatisfiedBy(T version) { - return isAvailable(version); - } - }); - return allAvailable; + return getAllVersions().stream().filter(this::isAvailable).collect(Collectors.toSet()); } private T getFirstAvailable(Collection versions) { diff --git a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/BaseGradleImplDepsIntegrationTest.groovy b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/BaseGradleImplDepsIntegrationTest.groovy index da23eba93c6a..033884c55a87 100644 --- a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/BaseGradleImplDepsIntegrationTest.groovy +++ b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/BaseGradleImplDepsIntegrationTest.groovy @@ -98,6 +98,28 @@ abstract class BaseGradleImplDepsIntegrationTest extends AbstractIntegrationSpec buildFile.toString() } + static String testablePluginProjectWithAddOpens(String appliedLanguagePlugin = applyGroovyPlugin()) { + return """ + ${testablePluginProject(appliedLanguagePlugin)} + // Needed when using ProjectBuilder + class AddOpensArgProvider implements CommandLineArgumentProvider { + private final Test test; + public AddOpensArgProvider(Test test) { + this.test = test; + } + @Override + Iterable asArguments() { + return test.javaVersion.isCompatibleWith(JavaVersion.VERSION_1_9) + ? ["--add-opens=java.base/java.lang=ALL-UNNAMED"] + : [] + } + } + tasks.withType(Test).configureEach { + jvmArgumentProviders.add(new AddOpensArgProvider(it)) + } + """ + } + static void assertSingleGenerationOutput(String output, String regex) { def pattern = /\b${regex}\b/ def matcher = output =~ pattern diff --git a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsShadingIssuesIntegrationTest.groovy b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsShadingIssuesIntegrationTest.groovy index 93c03fd03876..bb4413f8f8c8 100644 --- a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsShadingIssuesIntegrationTest.groovy +++ b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsShadingIssuesIntegrationTest.groovy @@ -28,7 +28,7 @@ class GradleImplDepsShadingIssuesIntegrationTest extends BaseGradleImplDepsInteg def "doesn't fail when using Ivy in a plugin"() { when: - buildFile << testablePluginProject() + buildFile << testablePluginProjectWithAddOpens() file('src/main/groovy/MyPlugin.groovy') << """ import org.gradle.api.Plugin import org.gradle.api.Project @@ -68,7 +68,7 @@ class GradleImplDepsShadingIssuesIntegrationTest extends BaseGradleImplDepsInteg def "can read resources both with relative and absolute path in relocated and original path"() { when: - buildFile << testablePluginProject() + buildFile << testablePluginProjectWithAddOpens() file('src/main/groovy/MyPlugin.groovy') << ''' import org.gradle.api.Plugin import org.gradle.api.Project diff --git a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsVisibilityIntegrationTest.groovy b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsVisibilityIntegrationTest.groovy index 1de6d9aba87e..1387ca84b6c1 100644 --- a/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsVisibilityIntegrationTest.groovy +++ b/subprojects/plugin-development/src/integTest/groovy/org/gradle/plugin/devel/impldeps/GradleImplDepsVisibilityIntegrationTest.groovy @@ -106,7 +106,7 @@ class GradleImplDepsVisibilityIntegrationTest extends BaseGradleImplDepsIntegrat def "can use ProjectBuilder to unit test a plugin"() { when: - buildFile << testablePluginProject() + buildFile << testablePluginProjectWithAddOpens() file('src/main/groovy/MyPlugin.groovy') << customGroovyPlugin() diff --git a/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java b/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java index a4308dd74c51..8f31ca1b64e1 100644 --- a/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java +++ b/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java @@ -18,6 +18,7 @@ import com.google.common.collect.Sets; import org.gradle.api.Action; +import org.gradle.api.JavaVersion; import org.gradle.api.NonNullApi; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -57,6 +58,7 @@ import org.gradle.plugin.use.PluginId; import org.gradle.plugin.use.internal.DefaultPluginId; import org.gradle.plugin.use.resolve.internal.local.PluginPublication; +import org.gradle.process.CommandLineArgumentProvider; import javax.annotation.Nullable; import java.io.File; @@ -65,6 +67,7 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -373,10 +376,14 @@ public void execute(Project project) { Set testSourceSets = extension.getTestSourceSets(); project.getNormalization().getRuntimeClasspath().ignore(PluginUnderTestMetadata.METADATA_FILE_NAME); - project.getTasks().withType(Test.class).configureEach(test -> test.getInputs() - .files(pluginClasspathTask.get().getPluginClasspath()) - .withPropertyName("pluginClasspath") - .withNormalizer(ClasspathNormalizer.class)); + project.getTasks().withType(Test.class).configureEach(test -> { + test.getInputs() + .files(pluginClasspathTask.get().getPluginClasspath()) + .withPropertyName("pluginClasspath") + .withNormalizer(ClasspathNormalizer.class); + + test.getJvmArgumentProviders().add(new AddOpensCommandLineArgumentProvider(test)); + }); for (SourceSet testSourceSet : testSourceSets) { String implementationConfigurationName = testSourceSet.getImplementationConfigurationName(); @@ -387,6 +394,26 @@ public void execute(Project project) { } } + /** + * Provides an {@code --add-opens} flag for {@code java.base/java.lang} if the JVM version + * a given test task is running does not allow reflection of JDK internals by default. + * Needed when using ProjectBuilder in tests. + */ + private static class AddOpensCommandLineArgumentProvider implements CommandLineArgumentProvider { + private final Test test; + + private AddOpensCommandLineArgumentProvider(Test test) { + this.test = test; + } + + @Override + public Iterable asArguments() { + return test.getJavaVersion().isCompatibleWith(JavaVersion.VERSION_16) + ? Collections.singletonList("--add-opens=java.base/java.lang=ALL-UNNAMED") + : Collections.emptyList(); + } + } + private static class LocalPluginPublication implements PluginPublication { private final PluginDeclaration pluginDeclaration; diff --git a/subprojects/test-kit/src/integTest/groovy/org/gradle/testkit/runner/enduser/GradleRunnerPluginClasspathInjectionEndUserIntegrationTest.groovy b/subprojects/test-kit/src/integTest/groovy/org/gradle/testkit/runner/enduser/GradleRunnerPluginClasspathInjectionEndUserIntegrationTest.groovy index 118a178a9531..a2e83bab6c6d 100644 --- a/subprojects/test-kit/src/integTest/groovy/org/gradle/testkit/runner/enduser/GradleRunnerPluginClasspathInjectionEndUserIntegrationTest.groovy +++ b/subprojects/test-kit/src/integTest/groovy/org/gradle/testkit/runner/enduser/GradleRunnerPluginClasspathInjectionEndUserIntegrationTest.groovy @@ -73,7 +73,7 @@ class GradleRunnerPluginClasspathInjectionEndUserIntegrationTest extends BaseTes def setup() { new File(testProjectDir, 'settings.gradle') << "rootProject.name = 'test'" buildFile = new File(testProjectDir, 'build.gradle') - def pluginClasspath = getClass().classLoader.findResource("plugin-classpath.txt") + def pluginClasspath = getClass().classLoader.getResource("plugin-classpath.txt") .readLines() .collect { it.replace('\\\\', '\\\\\\\\') } // escape backslashes in Windows paths .collect { "'\$it'" } @@ -127,7 +127,7 @@ class GradleRunnerPluginClasspathInjectionEndUserIntegrationTest extends BaseTes def setup() { new File(testProjectDir, 'settings.gradle') << "rootProject.name = 'test'" buildFile = new File(testProjectDir, 'build.gradle') - pluginClasspath = getClass().classLoader.findResource("plugin-classpath.txt") + pluginClasspath = getClass().classLoader.getResource("plugin-classpath.txt") .readLines() .collect { new File(it) } } diff --git a/subprojects/testing-base/src/main/java/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessor.java b/subprojects/testing-base/src/main/java/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessor.java index a2301e0c6524..9a9fc324c677 100755 --- a/subprojects/testing-base/src/main/java/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessor.java +++ b/subprojects/testing-base/src/main/java/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessor.java @@ -106,7 +106,10 @@ public void processTestClass(TestClassRunInfo testClass) { } RemoteTestClassProcessor forkProcess() { - WorkerProcessBuilder builder = workerFactory.create(new TestWorker(processorFactory)); + @SuppressWarnings("deprecation") // WorkerProcessBuilder#useLegacyAddOpens + WorkerProcessBuilder builder = + workerFactory.create(new TestWorker(processorFactory)) + .setUseLegacyAddOpens(false); builder.setBaseName("Gradle Test Executor"); builder.setImplementationClasspath(getTestWorkerImplementationClasspath()); builder.setImplementationModulePath(getTestWorkerImplementationModulePath()); diff --git a/subprojects/testing-base/src/test/groovy/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessorTest.groovy b/subprojects/testing-base/src/test/groovy/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessorTest.groovy index 2f0be38187a0..1afbe095a69c 100755 --- a/subprojects/testing-base/src/test/groovy/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessorTest.groovy +++ b/subprojects/testing-base/src/test/groovy/org/gradle/api/internal/tasks/testing/worker/ForkingTestClassProcessorTest.groovy @@ -55,6 +55,7 @@ class ForkingTestClassProcessorTest extends Specification { workerProcessBuilder.build() >> workerProcess workerProcessFactory.create(_) >> workerProcessBuilder workerProcessBuilder.getJavaCommand() >> Stub(JavaExecHandleBuilder) + workerProcessBuilder.setUseLegacyAddOpens(_) >> workerProcessBuilder } def "acquires worker lease and starts worker process on first test"() { diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/TestNGCoverage.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/TestNGCoverage.groovy index ef5c273d2c97..d3e4c5a5abd7 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/TestNGCoverage.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/TestNGCoverage.groovy @@ -16,20 +16,46 @@ package org.gradle.testing.fixture -import com.google.common.collect.ObjectArrays +import org.gradle.api.JavaVersion import org.gradle.integtests.fixtures.RepoScriptBlockUtil -import org.gradle.internal.jvm.Jvm +import org.gradle.util.internal.VersionNumber class TestNGCoverage { final static String NEWEST = '7.5' - final static String INITIAL_BROKEN_ICLASS_LISTENER = '6.9.10' // introduces initial, buggy IClassListener - final static String FIXED_ICLASS_LISTENER = '6.9.13.3' // introduces fixed IClassListener - final static String[] STANDARD_COVERAGE = ['5.14.10', '6.2', '6.8.7', '6.9.13.6', NEWEST] - final static String[] STANDARD_COVERAGE_WITH_INITIAL_ICLASS_LISTENER = ObjectArrays.concat(INITIAL_BROKEN_ICLASS_LISTENER, STANDARD_COVERAGE) - final static String LEGACY = '5.12.1' // lacks TestNG#setConfigFailurePolicy method - final static String[] STANDARD_COVERAGE_WITH_LEGACY = ObjectArrays.concat(LEGACY, STANDARD_COVERAGE_WITH_INITIAL_ICLASS_LISTENER) - final static String[] PRESERVE_ORDER = Jvm.current().javaVersion.java7Compatible ? ['5.14.6', '6.1.1', '6.9.4', NEWEST] : ['5.14.6', '6.1.1'] // skipped NEWEST (6.8.7) because of cbeust/testng#639 - final static String[] GROUP_BY_INSTANCES = ['6.1', '6.8.7', NEWEST] + + private static final String FIXED_ILLEGAL_ACCESS = '5.14.6' // Oldest version to support JDK 16+ without explicit --add-opens + private static final String FIXED_ICLASS_LISTENER = '6.9.13.3' // Introduces fixed IClassListener + + private static final Set VERSIONS = [ + '5.12.1', // Newest version without TestNG#setConfigFailurePolicy method (Added in 5.13) + FIXED_ILLEGAL_ACCESS, + '6.1.1', // Fixes NPE when using preserve-order and factories. + '6.8.21', // Newest version with cbeust/testng#639 bug + '6.9.4', // Fixes cbeust/testng#639 for preserve-order + '6.9.10', // Introduces initial, buggy IClassListener + FIXED_ICLASS_LISTENER, + NEWEST + ] + + static final Set SUPPORTED_BY_JDK = testNgVersionsSupportedByJdk(VERSIONS, JavaVersion.current()) + static final Set SUPPORTS_PRESERVE_ORDER = SUPPORTED_BY_JDK.findAll { VersionNumber.parse(it) >= VersionNumber.parse('5.14.5') } + static final Set SUPPORTS_GROUP_BY_INSTANCES = SUPPORTED_BY_JDK.findAll { VersionNumber.parse(it) >= VersionNumber.parse('6.1') } + static final Set SUPPORTS_ICLASS_LISTENER = SUPPORTED_BY_JDK.findAll { VersionNumber.parse(it) >= VersionNumber.parse(FIXED_ICLASS_LISTENER) } + + static boolean providesClassListener(Object version) { + VersionNumber.parse(version.toString()) >= VersionNumber.parse(FIXED_ICLASS_LISTENER) + } + + private static Set testNgVersionsSupportedByJdk(Set versions, JavaVersion javaVersion) { + if (javaVersion >= JavaVersion.VERSION_16) { + return versions.findAll { VersionNumber.parse(it) >= VersionNumber.parse(FIXED_ILLEGAL_ACCESS) } + } else if (javaVersion < JavaVersion.VERSION_1_7) { + // 6.8.21 was the last version to compile to JDK 5 bytecode. Afterwards (6.9.4) TestNG compiled to JDK 7 bytecode. + return versions.findAll { VersionNumber.parse(it) <= VersionNumber.parse('6.8.21')} + } else { + return versions + } + } /** * Adds java plugin and configures TestNG support in given build script file. diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/AbstractTestNGVersionIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/AbstractTestNGVersionIntegrationTest.groovy index d767ce0291c9..afd0690b7fb8 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/AbstractTestNGVersionIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/AbstractTestNGVersionIntegrationTest.groovy @@ -18,11 +18,10 @@ package org.gradle.testing.testng import org.gradle.integtests.fixtures.MultiVersionIntegrationSpec import org.gradle.integtests.fixtures.TargetCoverage +import org.gradle.testing.fixture.TestNGCoverage import org.gradle.util.internal.VersionNumber -import static org.gradle.testing.fixture.TestNGCoverage.* - -@TargetCoverage({ STANDARD_COVERAGE_WITH_LEGACY }) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) class AbstractTestNGVersionIntegrationTest extends MultiVersionIntegrationSpec { static boolean supportConfigFailurePolicy() { diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGClassIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGClassIntegrationTest.groovy index c79ca5f21b77..282792e763c2 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGClassIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGClassIntegrationTest.groovy @@ -24,11 +24,9 @@ import org.gradle.integtests.fixtures.MultiVersionIntegrationSpec import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.testing.fixture.TestNGCoverage -import static org.gradle.testing.fixture.TestNGCoverage.FIXED_ICLASS_LISTENER -import static org.gradle.testing.fixture.TestNGCoverage.NEWEST import static org.gradle.util.internal.TextUtil.normaliseFileSeparators -@TargetCoverage({ [FIXED_ICLASS_LISTENER, NEWEST] }) +@TargetCoverage({ TestNGCoverage.SUPPORTS_ICLASS_LISTENER }) class TestNGClassIntegrationTest extends MultiVersionIntegrationSpec { private final static String STARTED = 'Started' diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGFilteringIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGFilteringIntegrationTest.groovy index dcc9a74d9238..62bc4e592fe1 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGFilteringIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGFilteringIntegrationTest.groovy @@ -25,7 +25,7 @@ import spock.lang.Issue import static org.gradle.testing.fixture.JUnitMultiVersionIntegrationSpec.* -@TargetCoverage({TestNGCoverage.STANDARD_COVERAGE}) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) class TestNGFilteringIntegrationTest extends AbstractTestFilteringIntegrationTest { String imports = "org.testng.annotations.*" diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGGroupByInstancesIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGGroupByInstancesIntegrationTest.groovy index 23ccb266c8e3..14c2b4c593e6 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGGroupByInstancesIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGGroupByInstancesIntegrationTest.groovy @@ -20,7 +20,7 @@ import org.gradle.integtests.fixtures.MultiVersionIntegrationSpec import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.testing.fixture.TestNGCoverage -@TargetCoverage({TestNGCoverage.GROUP_BY_INSTANCES}) +@TargetCoverage({ TestNGCoverage.SUPPORTS_GROUP_BY_INSTANCES }) public class TestNGGroupByInstancesIntegrationTest extends MultiVersionIntegrationSpec { def "run tests using groupByInstances"() { diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGIntegrationTest.groovy index e042815ccb7e..3ae873b75757 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGIntegrationTest.groovy @@ -26,7 +26,7 @@ import spock.lang.Issue import static org.hamcrest.CoreMatchers.containsString import static org.hamcrest.CoreMatchers.not -@TargetCoverage({ TestNGCoverage.STANDARD_COVERAGE_WITH_INITIAL_ICLASS_LISTENER }) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) class TestNGIntegrationTest extends MultiVersionIntegrationSpec { def setup() { diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGLoggingOutputCaptureIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGLoggingOutputCaptureIntegrationTest.groovy index 2fc41dd61568..7bb602d6c2bc 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGLoggingOutputCaptureIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGLoggingOutputCaptureIntegrationTest.groovy @@ -16,21 +16,20 @@ package org.gradle.testing.testng - import org.gradle.integtests.fixtures.HtmlTestExecutionResult import org.gradle.integtests.fixtures.JUnitXmlTestExecutionResult import org.gradle.integtests.fixtures.MultiVersionIntegrationSpec import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.integtests.fixtures.TestClassExecutionResult -import org.gradle.util.GradleVersion +import org.gradle.testing.fixture.TestNGCoverage import org.gradle.util.Requires import org.gradle.util.TestPrecondition +import org.gradle.util.internal.VersionNumber -import static org.gradle.testing.fixture.TestNGCoverage.FIXED_ICLASS_LISTENER -import static org.gradle.testing.fixture.TestNGCoverage.STANDARD_COVERAGE +import static org.gradle.testing.fixture.TestNGCoverage.providesClassListener import static org.hamcrest.CoreMatchers.is -@TargetCoverage({STANDARD_COVERAGE}) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) @Requires(TestPrecondition.SUPPORTS_UTF8_STDOUT) class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationSpec { @@ -106,12 +105,15 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS when: succeeds "test" then: - containsLinesThatMatch(result.output, - "Gradle Test Executor \\d+ -> static out", - "Gradle Test Executor \\d+ -> static err", - "Gradle Test Executor \\d+ -> constructor out", - "Gradle Test Executor \\d+ -> constructor err" - ) + if (VersionNumber.parse(version.toString()) > VersionNumber.parse('5.12.1')) { + // Broken in 5.12.1, fixed in 5.13 + assert containsLinesThatMatch(result.output, + "Gradle Test Executor \\d+ -> static out", + "Gradle Test Executor \\d+ -> static err", + "Gradle Test Executor \\d+ -> constructor out", + "Gradle Test Executor \\d+ -> constructor err" + ) + } outputContains expectedOutput('The Foo Test') @@ -133,12 +135,15 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS when: succeeds "test" then: - containsLinesThatMatch(result.output, - "Gradle Test Executor \\d+ -> static out", - "Gradle Test Executor \\d+ -> static err", - "Gradle Test Executor \\d+ -> constructor out", - "Gradle Test Executor \\d+ -> constructor err" - ) + if (VersionNumber.parse(version.toString()) > VersionNumber.parse('5.12.1')) { + // Broken in 5.12.1, fixed in 5.13 + assert containsLinesThatMatch(result.output, + "Gradle Test Executor \\d+ -> static out", + "Gradle Test Executor \\d+ -> static err", + "Gradle Test Executor \\d+ -> constructor out", + "Gradle Test Executor \\d+ -> constructor err" + ) + } outputContains expectedOutput('Gradle test') @@ -163,7 +168,7 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS } private String expectedOutput(String testSuiteName) { - providesClassListener() ? expectedEventOutputWithTestClassListener(testSuiteName) : expectedEventOutputWithoutTestClassListener(testSuiteName) + providesClassListener(version) ? expectedEventOutputWithTestClassListener(testSuiteName) : expectedEventOutputWithoutTestClassListener(testSuiteName) } private void assertTestClassExecutionResultOutput(TestClassExecutionResult classResult) { @@ -172,7 +177,7 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS classResult.assertTestCaseStdout("m1", is("m1: \u03b1\n")) classResult.assertTestCaseStdout("m2", is("m2 out\n")) - if (providesClassListener()) { + if (providesClassListener(version)) { classResult.assertStderr(is("beforeClass err\n")) classResult.assertStdout(is("beforeClass out\n")) } else { @@ -182,7 +187,7 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS } private void assertTestClassExecutionResultReport(TestClassExecutionResult classReport) { - if (providesClassListener()) { + if (providesClassListener(version)) { classReport.assertStdout(is("beforeClass out\nm1: \u03b1\nm2 out\n")) classReport.assertStderr(is("beforeClass err\nm1 err\nm2 err\n")) } else { @@ -191,10 +196,6 @@ class TestNGLoggingOutputCaptureIntegrationTest extends MultiVersionIntegrationS } } - static boolean providesClassListener() { - GradleVersion.version(version).compareTo(GradleVersion.version(FIXED_ICLASS_LISTENER)) == 1 - } - static String expectedEventOutputWithoutTestClassListener(String testSuiteName) { """Test suite '$testSuiteName' -> beforeTest out Test suite '$testSuiteName' -> beforeTest err diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGParallelSuiteIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGParallelSuiteIntegrationTest.groovy index d18c82112dd3..937d4de4d8dd 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGParallelSuiteIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGParallelSuiteIntegrationTest.groovy @@ -23,7 +23,7 @@ import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.testing.fixture.TestNGCoverage import spock.lang.Issue -@TargetCoverage({ TestNGCoverage.STANDARD_COVERAGE }) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) class TestNGParallelSuiteIntegrationTest extends MultiVersionIntegrationSpec { def setup() { buildFile << """ diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGPreserveOrderIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGPreserveOrderIntegrationTest.groovy index e955c82335ee..0d870e7eb2df 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGPreserveOrderIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGPreserveOrderIntegrationTest.groovy @@ -20,7 +20,7 @@ import org.gradle.integtests.fixtures.MultiVersionIntegrationSpec import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.testing.fixture.TestNGCoverage -@TargetCoverage({TestNGCoverage.PRESERVE_ORDER}) +@TargetCoverage({ TestNGCoverage.SUPPORTS_PRESERVE_ORDER }) public class TestNGPreserveOrderIntegrationTest extends MultiVersionIntegrationSpec { def "run tests using preserveOrder"() { diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteInitialisationIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteInitialisationIntegrationTest.groovy index 84cfa7c959e0..be7ca56aabcf 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteInitialisationIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteInitialisationIntegrationTest.groovy @@ -18,6 +18,7 @@ package org.gradle.testing.testng import org.gradle.integtests.fixtures.AbstractIntegrationSpec import org.gradle.integtests.fixtures.DefaultTestExecutionResult +import org.gradle.testing.fixture.TestNGCoverage import spock.lang.Issue import static org.gradle.integtests.fixtures.TestExecutionResult.EXECUTION_FAILURE @@ -27,17 +28,7 @@ class TestNGSuiteInitialisationIntegrationTest extends AbstractIntegrationSpec { @Issue("GRADLE-1710") def "reports suite fatal failure"() { - buildFile << """ - apply plugin: 'java' - ${mavenCentralRepository()} - testing { - suites { - test { - useTestNG('6.3.1') - } - } - } - """ + TestNGCoverage.enableTestNG(buildFile, '6.3.1') file("src/test/java/FooTest.java") << """ import org.testng.annotations.*; diff --git a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteIntegrationTest.groovy b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteIntegrationTest.groovy index 24a1fde40931..3995ed095901 100644 --- a/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteIntegrationTest.groovy +++ b/subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGSuiteIntegrationTest.groovy @@ -23,7 +23,7 @@ import org.gradle.integtests.fixtures.TargetCoverage import org.gradle.testing.fixture.TestNGCoverage import spock.lang.Issue -@TargetCoverage({TestNGCoverage.STANDARD_COVERAGE}) +@TargetCoverage({ TestNGCoverage.SUPPORTED_BY_JDK }) class TestNGSuiteIntegrationTest extends MultiVersionIntegrationSpec { /** 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 c53dc1f74fe4..91a2da4353f6 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 @@ -266,7 +266,8 @@ public Test workingDir(Object dir) { } /** - * Returns the version of Java used to run the tests based on the executable specified by {@link #getExecutable()}. + * Returns the version of Java used to run the tests based on the {@link JavaLauncher} specified by {@link #getJavaLauncher()}, + * or the executable specified by {@link #getExecutable()} if the {@code JavaLauncher} is not present. * * @since 3.3 */ diff --git a/subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/JdkIllegalReflectionTestWorkerIntegrationTest.groovy b/subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/JdkIllegalReflectionTestWorkerIntegrationTest.groovy new file mode 100644 index 000000000000..46affc84ec62 --- /dev/null +++ b/subprojects/workers/src/integTest/groovy/org/gradle/workers/internal/JdkIllegalReflectionTestWorkerIntegrationTest.groovy @@ -0,0 +1,104 @@ +/* + * Copyright 2022 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.workers.internal + +import org.gradle.integtests.fixtures.AbstractIntegrationSpec +import org.gradle.integtests.fixtures.DefaultTestExecutionResult +import org.gradle.util.Requires +import org.gradle.util.TestPrecondition +import spock.lang.Issue + +import static org.hamcrest.CoreMatchers.containsString + +/** + * Ensures test behavior and actual application behavior are equivalent when + * production code attempts to perform reflection on JDK internals. + */ +class JdkIllegalReflectionTestWorkerIntegrationTest extends AbstractIntegrationSpec { + + def setup() { + buildFile << """ + plugins { + id 'application' + } + + ${mavenCentralRepository()} + + testing.suites.test.useJUnit() + + application { + mainClass = 'example.Main' + } + """ + + file("src/main/java/example/Main.java") << """ + package example; + + import java.lang.invoke.MethodHandles; + + public class Main { + public static void main(String[] args) throws Exception { + MethodHandles.privateLookupIn(java.lang.ClassLoader.class, MethodHandles.lookup()); + } + } + """ + + file("src/test/java/example/MainTest.java") << """ + package example; + + import org.junit.Test; + + public class MainTest { + @Test + public void runTest() throws Exception { + Main.main(new String[0]); + } + } + """ + } + + @Requires(TestPrecondition.JDK16_OR_LATER) + @Issue("https://github.com/gradle/gradle/issues/19771") + def "both tests and production code fail when application uses illegal reflection"() { + when: + executer.withStackTraceChecksDisabled() + + then: + fails "run" + result.assertHasErrorOutput("module java.base does not open java.lang to unnamed module") + + expect: + fails "test" + def results = new DefaultTestExecutionResult(file(".")) + results.assertTestClassesExecuted("example.MainTest") + results.testClass("example.MainTest").assertTestFailed("runTest", containsString('module java.base does not open java.lang to unnamed module')) + } + + @Requires(TestPrecondition.JDK16_OR_LATER) + @Issue("https://github.com/gradle/gradle/issues/19771") + def "can add --add-opens flag in test to permit reflection"() { + given: + buildFile << """ + tasks.withType(Test).configureEach { + jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED") + } + """ + + expect: + succeeds "test" + } +}