From 243dcd951218706fdbe2791daede015e061905db Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Mon, 1 Jun 2026 18:55:42 +0200 Subject: [PATCH] Refactor JUnit extensions to avoid using static fields. Static filed in Junit extension can impact on tests executed in parallel. For static methods preserver current test context in local thread. (cherry picked from commit 52c4d437379221ad860a0a48de96a962aa986c24) --- .../api/di/testing/MavenDIExtension.java | 50 +++++++++++------ .../api/plugin/testing/MojoExtension.java | 53 ++++++++++--------- .../maven/api/di/testing/SimpleDITest.java | 6 +-- 3 files changed, 62 insertions(+), 47 deletions(-) diff --git a/impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java b/impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java index 12de0178d137..113549277988 100644 --- a/impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java +++ b/impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java @@ -19,6 +19,7 @@ package org.apache.maven.api.di.testing; import java.io.File; +import java.util.Optional; import org.apache.maven.di.Injector; import org.apache.maven.di.Key; @@ -52,9 +53,11 @@ * */ public class MavenDIExtension implements BeforeEachCallback, AfterEachCallback { - protected static ExtensionContext context; - protected Injector injector; - protected static String basedir; + + private static final ExtensionContext.Namespace MAVEN_DI_EXTENSION = + ExtensionContext.Namespace.create("maven-di-extension"); + + private static final ThreadLocal EXTENSION_CONTEXT_THREAD_LOCAL = new ThreadLocal<>(); /** * Initializes the test environment before each test method execution. @@ -65,7 +68,6 @@ public class MavenDIExtension implements BeforeEachCallback, AfterEachCallback { */ @Override public void beforeEach(ExtensionContext context) throws Exception { - basedir = getBasedir(); setContext(context); getInjector().bindInstance((Class) context.getRequiredTestClass(), context.getRequiredTestInstance()); getInjector().injectInstance(context.getRequiredTestInstance()); @@ -77,7 +79,16 @@ public void beforeEach(ExtensionContext context) throws Exception { * @param context The extension context to store */ protected void setContext(ExtensionContext context) { - MavenDIExtension.context = context; + EXTENSION_CONTEXT_THREAD_LOCAL.set(context); + } + + /** + * Retrieves the extension context for the current thread. + * + * @return The extension context, or an empty Optional if not set + */ + protected static Optional getContext() { + return Optional.ofNullable(EXTENSION_CONTEXT_THREAD_LOCAL.get()); } /** @@ -87,7 +98,7 @@ protected void setContext(ExtensionContext context) { * @throws IllegalStateException if the ExtensionContext is null, the required test class is unavailable, * the required test instance is unavailable, or if container setup fails */ - protected void setupContainer() { + protected Injector setupContainer(ExtensionContext context) { if (context == null) { throw new IllegalStateException("ExtensionContext must not be null"); } @@ -101,11 +112,12 @@ protected void setupContainer() { } try { - injector = Injector.create(); + Injector injector = Injector.create(); injector.bindInstance(ExtensionContext.class, context); injector.discover(testClass.getClassLoader()); injector.bindInstance(Injector.class, injector); injector.bindInstance(testClass.asSubclass(Object.class), (Object) testInstance); // Safe generics handling + return injector; } catch (final Exception e) { throw new IllegalStateException( String.format( @@ -123,10 +135,14 @@ protected void setupContainer() { */ @Override public void afterEach(ExtensionContext context) throws Exception { - if (injector != null) { - // TODO: implement - // injector.dispose(); - injector = null; + try { + Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class); + if (injector != null) { + // TODO: implement + // injector.dispose(); + } + } finally { + EXTENSION_CONTEXT_THREAD_LOCAL.remove(); } } @@ -136,8 +152,12 @@ public void afterEach(ExtensionContext context) throws Exception { * @return The configured injector instance */ public Injector getInjector() { + ExtensionContext context = + getContext().orElseThrow(() -> new IllegalStateException("ExtensionContext must not be null")); + Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class); if (injector == null) { - setupContainer(); + injector = setupContainer(context); + context.getStore(MAVEN_DI_EXTENSION).put(Injector.class, injector); } return injector; } @@ -247,11 +267,7 @@ public static String getTestPath(String basedir, String path) { * @return The base directory path */ public static String getBasedir() { - if (basedir != null) { - return basedir; - } - - basedir = System.getProperty("basedir"); + String basedir = System.getProperty("basedir"); if (basedir == null) { basedir = new File("").getAbsolutePath(); diff --git a/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java b/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java index 1a9bfff9c59d..d6efe95459c8 100644 --- a/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java +++ b/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java @@ -165,11 +165,10 @@ */ public class MojoExtension extends MavenDIExtension implements ParameterResolver, BeforeEachCallback { - /** The base directory of the plugin being tested */ - protected static String pluginBasedir; + private static final ExtensionContext.Namespace MOJO_EXTENSION = + ExtensionContext.Namespace.create("mojo-extension"); - /** The base directory for test resources */ - protected static String basedir; + private static final String BASEDIR_KEY = "basedir"; /** * Gets the identifier for the current test method. @@ -178,6 +177,8 @@ public class MojoExtension extends MavenDIExtension implements ParameterResolver * @return the test identifier */ public static String getTestId() { + ExtensionContext context = + getContext().orElseThrow(() -> new IllegalStateException("ExtensionContext must not be null")); return context.getRequiredTestClass().getSimpleName() + "-" + context.getRequiredTestMethod().getName(); } @@ -190,7 +191,10 @@ public static String getTestId() { * @throws NullPointerException if neither basedir nor plugin basedir is set */ public static String getBasedir() { - return requireNonNull(basedir != null ? basedir : MavenDIExtension.basedir); + String basedir = getContext() + .map(context -> context.getStore(MOJO_EXTENSION).get(BASEDIR_KEY, String.class)) + .orElse(null); + return requireNonNull(basedir != null ? basedir : MavenDIExtension.getBasedir()); } /** @@ -200,7 +204,7 @@ public static String getBasedir() { * @throws NullPointerException if plugin basedir is not set */ public static String getPluginBasedir() { - return requireNonNull(pluginBasedir); + return MavenDIExtension.getBasedir(); } /** @@ -224,11 +228,9 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte throws ParameterResolutionException { try { Class holder = parameterContext.getTarget().orElseThrow().getClass(); - PluginDescriptor descriptor = extensionContext - .getStore(ExtensionContext.Namespace.GLOBAL) - .get(PluginDescriptor.class, PluginDescriptor.class); - Model model = - extensionContext.getStore(ExtensionContext.Namespace.GLOBAL).get(Model.class, Model.class); + PluginDescriptor descriptor = + extensionContext.getStore(MOJO_EXTENSION).get(PluginDescriptor.class, PluginDescriptor.class); + Model model = extensionContext.getStore(MOJO_EXTENSION).get(Model.class, Model.class); InjectMojo parameterInjectMojo = parameterContext.getAnnotatedElement().getAnnotation(InjectMojo.class); String goal; @@ -312,23 +314,22 @@ private static String getGoalFromMojoImplementationClass(Class cl) throws IOE @Override @SuppressWarnings("checkstyle:MethodLength") public void beforeEach(ExtensionContext context) throws Exception { - if (pluginBasedir == null) { - pluginBasedir = MavenDIExtension.getBasedir(); - } - basedir = AnnotationSupport.findAnnotation(context.getElement().orElseThrow(), Basedir.class) + setContext(context); + + String pluginBasedir = MavenDIExtension.getBasedir(); + + String basedir = AnnotationSupport.findAnnotation(context.getElement().orElseThrow(), Basedir.class) .map(Basedir::value) .orElse(pluginBasedir); - if (basedir != null) { - if (basedir.isEmpty()) { - basedir = pluginBasedir + "/target/tests/" - + context.getRequiredTestClass().getSimpleName() + "/" - + context.getRequiredTestMethod().getName(); - } else { - basedir = basedir.replace("${basedir}", pluginBasedir); - } + if (basedir.isEmpty()) { + basedir = pluginBasedir + "/target/tests/" + + context.getRequiredTestClass().getSimpleName() + "/" + + context.getRequiredTestMethod().getName(); + } else { + basedir = basedir.replace("${basedir}", pluginBasedir); } - setContext(context); + context.getStore(MOJO_EXTENSION).put(BASEDIR_KEY, basedir); /* binder.install(ProviderMethodsModule.forObject(context.getRequiredTestInstance())); @@ -419,7 +420,7 @@ public void beforeEach(ExtensionContext context) throws Exception { } tmodel = new DefaultModelPathTranslator(new DefaultPathTranslator()) .alignToBaseDirectory(tmodel, Paths.get(getBasedir()), null); - context.getStore(ExtensionContext.Namespace.GLOBAL).put(Model.class, tmodel); + context.getStore(MOJO_EXTENSION).put(Model.class, tmodel); // mojo execution // Map map = getInjector().getContext().getContextData(); @@ -432,7 +433,7 @@ public void beforeEach(ExtensionContext context) throws Exception { // new InterpolationFilterReader(reader, map, "${", "}"); pluginDescriptor = new PluginDescriptorStaxReader().read(reader); } - context.getStore(ExtensionContext.Namespace.GLOBAL).put(PluginDescriptor.class, pluginDescriptor); + context.getStore(MOJO_EXTENSION).put(PluginDescriptor.class, pluginDescriptor); // for (ComponentDescriptor desc : pluginDescriptor.getComponents()) { // getContainer().addComponentDescriptor(desc); // } diff --git a/impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java b/impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java index fbfa625a13cb..b2f7858508a6 100644 --- a/impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java +++ b/impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java @@ -55,8 +55,7 @@ Session createSession() { @Test void testSetupContainerWithNullContext() { MavenDIExtension extension = new MavenDIExtension(); - MavenDIExtension.context = null; - assertThrows(IllegalStateException.class, extension::setupContainer); + assertThrows(IllegalStateException.class, () -> extension.setupContainer(null)); } @Test @@ -65,10 +64,9 @@ void testSetupContainerWithNullTestClass() { final ExtensionContext context = mock(ExtensionContext.class); when(context.getRequiredTestClass()).thenReturn(null); // Mock null test class when(context.getRequiredTestInstance()).thenReturn(new TestClass()); // Valid instance - MavenDIExtension.context = context; assertThrows( IllegalStateException.class, - extension::setupContainer, + () -> extension.setupContainer(context), "Should throw IllegalStateException for null test class"); }