Skip to content

Refactor JUnit extensions to avoid using static fields. #11829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slawekjaranowski wants to merge 3 commits into
base: master
Choose a base branch
from
+61 −46
Open

Refactor JUnit extensions to avoid using static fields. #11829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cbbaaa
Refactor JUnit extensions to avoid using static fields.
slawekjaranowski Mar 23, 2026
bdce42e
fix after review
slawekjaranowski Mar 24, 2026
17b3d54
Fix thread-local cleanup in afterEach method
slawekjaranowski Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,9 +53,11 @@
* </pre>
*/
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<ExtensionContext> EXTENSION_CONTEXT_THREAD_LOCAL = new ThreadLocal<>();

/**
* Initializes the test environment before each test method execution.
Expand All @@ -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<Object>) context.getRequiredTestClass(), context.getRequiredTestInstance());
getInjector().injectInstance(context.getRequiredTestInstance());
Expand All @@ -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<ExtensionContext> getContext() {
return Optional.ofNullable(EXTENSION_CONTEXT_THREAD_LOCAL.get());
}

/**
Expand All @@ -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");
}
Expand All @@ -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(
Expand All @@ -123,9 +135,13 @@ protected void setupContainer() {
*/
@Override
public void afterEach(ExtensionContext context) throws Exception {
if (injector != null) {
injector.dispose();
injector = null;
try {
Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class);
if (injector != null) {
injector.dispose();
}
} finally {
EXTENSION_CONTEXT_THREAD_LOCAL.remove();
}
}

Expand All @@ -135,8 +151,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;
}
Expand Down Expand Up @@ -246,11 +266,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();
Expand Down
53 changes: 27 additions & 26 deletions impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
}
Expand All @@ -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());
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -331,23 +333,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()));
Expand Down Expand Up @@ -438,7 +439,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<Object, Object> map = getInjector().getContext().getContextData();
Expand All @@ -451,7 +452,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);
// }
Expand Down
6 changes: 2 additions & 4 deletions impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}

Expand Down