From 7cbbaaa205ff1533b67b952ea98473e18e82dc3e Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Mon, 23 Mar 2026 21:37:25 +0100 Subject: [PATCH 1/3] 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. --- .../api/di/testing/MavenDIExtension.java | 37 +++++++----- .../api/plugin/testing/MojoExtension.java | 58 ++++++++++--------- .../maven/api/di/testing/SimpleDITest.java | 6 +- 3 files changed, 54 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 67e2c714862d..2085600cff03 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()); @@ -76,10 +78,13 @@ public void beforeEach(ExtensionContext context) throws Exception { * * @param context The extension context to store */ - protected void setContext(ExtensionContext context) { - MavenDIExtension.context = context; + protected static void setContext(ExtensionContext context) { + EXTENSION_CONTEXT_THREAD_LOCAL.set(context); } + protected static Optional getContext() { + return Optional.ofNullable(EXTENSION_CONTEXT_THREAD_LOCAL.get()); + } /** * Creates and configures the DI container for test execution. * Performs component discovery and sets up basic bindings. @@ -87,7 +92,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 +106,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 +129,11 @@ protected void setupContainer() { */ @Override public void afterEach(ExtensionContext context) throws Exception { + Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class); if (injector != null) { injector.dispose(); - injector = null; } + EXTENSION_CONTEXT_THREAD_LOCAL.remove(); } /** @@ -135,8 +142,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; } @@ -246,11 +257,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 0bc054aebe3d..4fa5c3cadef1 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 @@ -168,11 +168,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. @@ -181,6 +180,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(); } @@ -193,7 +194,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()); } /** @@ -203,7 +207,7 @@ public static String getBasedir() { * @throws NullPointerException if plugin basedir is not set */ public static String getPluginBasedir() { - return requireNonNull(pluginBasedir); + return MavenDIExtension.getBasedir(); } /** @@ -227,11 +231,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; @@ -331,23 +333,23 @@ 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); - } + .orElse(null); + + if (basedir == null || 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())); @@ -437,8 +439,8 @@ public void beforeEach(ExtensionContext context) throws Exception { model = new MavenMerger().merge(tmodel, defaultModel, false, null); } tmodel = new DefaultModelPathTranslator(new DefaultPathTranslator()) - .alignToBaseDirectory(tmodel, Paths.get(getBasedir()), null); - context.getStore(ExtensionContext.Namespace.GLOBAL).put(Model.class, tmodel); + .alignToBaseDirectory(model, Paths.get(getBasedir()), null); + context.getStore(MOJO_EXTENSION).put(Model.class, tmodel); // mojo execution // Map map = getInjector().getContext().getContextData(); @@ -451,7 +453,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"); } From bdce42e7ffa370ca94382840b7c838ed2d47b8d9 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Tue, 24 Mar 2026 20:50:08 +0100 Subject: [PATCH 2/3] fix after review --- .../org/apache/maven/api/di/testing/MavenDIExtension.java | 8 +++++++- .../apache/maven/api/plugin/testing/MojoExtension.java | 7 +++---- 2 files changed, 10 insertions(+), 5 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 2085600cff03..416d0c24834e 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 @@ -78,13 +78,19 @@ public void beforeEach(ExtensionContext context) throws Exception { * * @param context The extension context to store */ - protected static void setContext(ExtensionContext context) { + protected void setContext(ExtensionContext 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()); } + /** * Creates and configures the DI container for test execution. * Performs component discovery and sets up basic bindings. 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 4fa5c3cadef1..8824a61d5462 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 @@ -339,9 +339,8 @@ public void beforeEach(ExtensionContext context) throws Exception { String basedir = AnnotationSupport.findAnnotation(context.getElement().orElseThrow(), Basedir.class) .map(Basedir::value) - .orElse(null); - - if (basedir == null || basedir.isEmpty()) { + .orElse(pluginBasedir); + if (basedir.isEmpty()) { basedir = pluginBasedir + "/target/tests/" + context.getRequiredTestClass().getSimpleName() + "/" + context.getRequiredTestMethod().getName(); @@ -439,7 +438,7 @@ public void beforeEach(ExtensionContext context) throws Exception { model = new MavenMerger().merge(tmodel, defaultModel, false, null); } tmodel = new DefaultModelPathTranslator(new DefaultPathTranslator()) - .alignToBaseDirectory(model, Paths.get(getBasedir()), null); + .alignToBaseDirectory(tmodel, Paths.get(getBasedir()), null); context.getStore(MOJO_EXTENSION).put(Model.class, tmodel); // mojo execution From 17b3d54b9c8d5f114ff62b70559fd790f5e87edc Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Thu, 26 Mar 2026 21:12:28 +0100 Subject: [PATCH 3/3] Fix thread-local cleanup in afterEach method --- .../apache/maven/api/di/testing/MavenDIExtension.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 416d0c24834e..8b43165de7c4 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 @@ -135,11 +135,14 @@ protected Injector setupContainer(ExtensionContext context) { */ @Override public void afterEach(ExtensionContext context) throws Exception { - Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class); - if (injector != null) { - injector.dispose(); + try { + Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class); + if (injector != null) { + injector.dispose(); + } + } finally { + EXTENSION_CONTEXT_THREAD_LOCAL.remove(); } - EXTENSION_CONTEXT_THREAD_LOCAL.remove(); } /**