Skip to content

Commit 52c4d43

Browse files
slawekjaranowskislawekjaranowski
authored
Refactor JUnit extensions to avoid using static fields. (#11829)
Static filed in Junit extension can impact on tests executed in parallel. For static methods preserver current test context in local thread.
1 parent 15d19f2 commit 52c4d43

3 files changed

Lines changed: 61 additions & 46 deletions

File tree

‎impl/maven-testing/src/main/java/org/apache/maven/api/di/testing/MavenDIExtension.java‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.maven.api.di.testing;
2020

2121
import java.io.File;
22+
import java.util.Optional;
2223

2324
import org.apache.maven.di.Injector;
2425
import org.apache.maven.di.Key;
@@ -52,9 +53,11 @@
5253
* </pre>
5354
*/
5455
public class MavenDIExtension implements BeforeEachCallback, AfterEachCallback {
55-
protected static ExtensionContext context;
56-
protected Injector injector;
57-
protected static String basedir;
56+
57+
private static final ExtensionContext.Namespace MAVEN_DI_EXTENSION =
58+
ExtensionContext.Namespace.create("maven-di-extension");
59+
60+
private static final ThreadLocal<ExtensionContext> EXTENSION_CONTEXT_THREAD_LOCAL = new ThreadLocal<>();
5861

5962
/**
6063
* Initializes the test environment before each test method execution.
@@ -65,7 +68,6 @@ public class MavenDIExtension implements BeforeEachCallback, AfterEachCallback {
6568
*/
6669
@Override
6770
public void beforeEach(ExtensionContext context) throws Exception {
68-
basedir = getBasedir();
6971
setContext(context);
7072
getInjector().bindInstance((Class<Object>) context.getRequiredTestClass(), context.getRequiredTestInstance());
7173
getInjector().injectInstance(context.getRequiredTestInstance());
@@ -77,7 +79,16 @@ public void beforeEach(ExtensionContext context) throws Exception {
7779
* @param context The extension context to store
7880
*/
7981
protected void setContext(ExtensionContext context) {
80-
MavenDIExtension.context = context;
82+
EXTENSION_CONTEXT_THREAD_LOCAL.set(context);
83+
}
84+
85+
/**
86+
* Retrieves the extension context for the current thread.
87+
*
88+
* @return The extension context, or an empty Optional if not set
89+
*/
90+
protected static Optional<ExtensionContext> getContext() {
91+
return Optional.ofNullable(EXTENSION_CONTEXT_THREAD_LOCAL.get());
8192
}
8293

8394
/**
@@ -87,7 +98,7 @@ protected void setContext(ExtensionContext context) {
8798
* @throws IllegalStateException if the ExtensionContext is null, the required test class is unavailable,
8899
* the required test instance is unavailable, or if container setup fails
89100
*/
90-
protected void setupContainer() {
101+
protected Injector setupContainer(ExtensionContext context) {
91102
if (context == null) {
92103
throw new IllegalStateException("ExtensionContext must not be null");
93104
}
@@ -101,11 +112,12 @@ protected void setupContainer() {
101112
}
102113

103114
try {
104-
injector = Injector.create();
115+
Injector injector = Injector.create();
105116
injector.bindInstance(ExtensionContext.class, context);
106117
injector.discover(testClass.getClassLoader());
107118
injector.bindInstance(Injector.class, injector);
108119
injector.bindInstance(testClass.asSubclass(Object.class), (Object) testInstance); // Safe generics handling
120+
return injector;
109121
} catch (final Exception e) {
110122
throw new IllegalStateException(
111123
String.format(
@@ -123,9 +135,13 @@ protected void setupContainer() {
123135
*/
124136
@Override
125137
public void afterEach(ExtensionContext context) throws Exception {
126-
if (injector != null) {
127-
injector.dispose();
128-
injector = null;
138+
try {
139+
Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class);
140+
if (injector != null) {
141+
injector.dispose();
142+
}
143+
} finally {
144+
EXTENSION_CONTEXT_THREAD_LOCAL.remove();
129145
}
130146
}
131147

@@ -135,8 +151,12 @@ public void afterEach(ExtensionContext context) throws Exception {
135151
* @return The configured injector instance
136152
*/
137153
public Injector getInjector() {
154+
ExtensionContext context =
155+
getContext().orElseThrow(() -> new IllegalStateException("ExtensionContext must not be null"));
156+
Injector injector = context.getStore(MAVEN_DI_EXTENSION).get(Injector.class, Injector.class);
138157
if (injector == null) {
139-
setupContainer();
158+
injector = setupContainer(context);
159+
context.getStore(MAVEN_DI_EXTENSION).put(Injector.class, injector);
140160
}
141161
return injector;
142162
}
@@ -246,11 +266,7 @@ public static String getTestPath(String basedir, String path) {
246266
* @return The base directory path
247267
*/
248268
public static String getBasedir() {
249-
if (basedir != null) {
250-
return basedir;
251-
}
252-
253-
basedir = System.getProperty("basedir");
269+
String basedir = System.getProperty("basedir");
254270

255271
if (basedir == null) {
256272
basedir = new File("").getAbsolutePath();

‎impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/MojoExtension.java‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,10 @@
168168
*/
169169
public class MojoExtension extends MavenDIExtension implements ParameterResolver, BeforeEachCallback {
170170

171-
/** The base directory of the plugin being tested */
172-
protected static String pluginBasedir;
171+
private static final ExtensionContext.Namespace MOJO_EXTENSION =
172+
ExtensionContext.Namespace.create("mojo-extension");
173173

174-
/** The base directory for test resources */
175-
protected static String basedir;
174+
private static final String BASEDIR_KEY = "basedir";
176175

177176
/**
178177
* Gets the identifier for the current test method.
@@ -181,6 +180,8 @@ public class MojoExtension extends MavenDIExtension implements ParameterResolver
181180
* @return the test identifier
182181
*/
183182
public static String getTestId() {
183+
ExtensionContext context =
184+
getContext().orElseThrow(() -> new IllegalStateException("ExtensionContext must not be null"));
184185
return context.getRequiredTestClass().getSimpleName() + "-"
185186
+ context.getRequiredTestMethod().getName();
186187
}
@@ -193,7 +194,10 @@ public static String getTestId() {
193194
* @throws NullPointerException if neither basedir nor plugin basedir is set
194195
*/
195196
public static String getBasedir() {
196-
return requireNonNull(basedir != null ? basedir : MavenDIExtension.basedir);
197+
String basedir = getContext()
198+
.map(context -> context.getStore(MOJO_EXTENSION).get(BASEDIR_KEY, String.class))
199+
.orElse(null);
200+
return requireNonNull(basedir != null ? basedir : MavenDIExtension.getBasedir());
197201
}
198202

199203
/**
@@ -203,7 +207,7 @@ public static String getBasedir() {
203207
* @throws NullPointerException if plugin basedir is not set
204208
*/
205209
public static String getPluginBasedir() {
206-
return requireNonNull(pluginBasedir);
210+
return MavenDIExtension.getBasedir();
207211
}
208212

209213
/**
@@ -227,11 +231,9 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte
227231
throws ParameterResolutionException {
228232
try {
229233
Class<?> holder = parameterContext.getTarget().orElseThrow().getClass();
230-
PluginDescriptor descriptor = extensionContext
231-
.getStore(ExtensionContext.Namespace.GLOBAL)
232-
.get(PluginDescriptor.class, PluginDescriptor.class);
233-
Model model =
234-
extensionContext.getStore(ExtensionContext.Namespace.GLOBAL).get(Model.class, Model.class);
234+
PluginDescriptor descriptor =
235+
extensionContext.getStore(MOJO_EXTENSION).get(PluginDescriptor.class, PluginDescriptor.class);
236+
Model model = extensionContext.getStore(MOJO_EXTENSION).get(Model.class, Model.class);
235237
InjectMojo parameterInjectMojo =
236238
parameterContext.getAnnotatedElement().getAnnotation(InjectMojo.class);
237239
String goal;
@@ -331,23 +333,22 @@ private static String getGoalFromMojoImplementationClass(Class<?> cl) throws IOE
331333
@Override
332334
@SuppressWarnings("checkstyle:MethodLength")
333335
public void beforeEach(ExtensionContext context) throws Exception {
334-
if (pluginBasedir == null) {
335-
pluginBasedir = MavenDIExtension.getBasedir();
336-
}
337-
basedir = AnnotationSupport.findAnnotation(context.getElement().orElseThrow(), Basedir.class)
336+
setContext(context);
337+
338+
String pluginBasedir = MavenDIExtension.getBasedir();
339+
340+
String basedir = AnnotationSupport.findAnnotation(context.getElement().orElseThrow(), Basedir.class)
338341
.map(Basedir::value)
339342
.orElse(pluginBasedir);
340-
if (basedir != null) {
341-
if (basedir.isEmpty()) {
342-
basedir = pluginBasedir + "/target/tests/"
343-
+ context.getRequiredTestClass().getSimpleName() + "/"
344-
+ context.getRequiredTestMethod().getName();
345-
} else {
346-
basedir = basedir.replace("${basedir}", pluginBasedir);
347-
}
343+
if (basedir.isEmpty()) {
344+
basedir = pluginBasedir + "/target/tests/"
345+
+ context.getRequiredTestClass().getSimpleName() + "/"
346+
+ context.getRequiredTestMethod().getName();
347+
} else {
348+
basedir = basedir.replace("${basedir}", pluginBasedir);
348349
}
349350

350-
setContext(context);
351+
context.getStore(MOJO_EXTENSION).put(BASEDIR_KEY, basedir);
351352

352353
/*
353354
binder.install(ProviderMethodsModule.forObject(context.getRequiredTestInstance()));
@@ -438,7 +439,7 @@ public void beforeEach(ExtensionContext context) throws Exception {
438439
}
439440
tmodel = new DefaultModelPathTranslator(new DefaultPathTranslator())
440441
.alignToBaseDirectory(tmodel, Paths.get(getBasedir()), null);
441-
context.getStore(ExtensionContext.Namespace.GLOBAL).put(Model.class, tmodel);
442+
context.getStore(MOJO_EXTENSION).put(Model.class, tmodel);
442443

443444
// mojo execution
444445
// Map<Object, Object> map = getInjector().getContext().getContextData();
@@ -451,7 +452,7 @@ public void beforeEach(ExtensionContext context) throws Exception {
451452
// new InterpolationFilterReader(reader, map, "${", "}");
452453
pluginDescriptor = new PluginDescriptorStaxReader().read(reader);
453454
}
454-
context.getStore(ExtensionContext.Namespace.GLOBAL).put(PluginDescriptor.class, pluginDescriptor);
455+
context.getStore(MOJO_EXTENSION).put(PluginDescriptor.class, pluginDescriptor);
455456
// for (ComponentDescriptor<?> desc : pluginDescriptor.getComponents()) {
456457
// getContainer().addComponentDescriptor(desc);
457458
// }

‎impl/maven-testing/src/test/java/org/apache/maven/api/di/testing/SimpleDITest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ Session createSession() {
5555
@Test
5656
void testSetupContainerWithNullContext() {
5757
MavenDIExtension extension = new MavenDIExtension();
58-
MavenDIExtension.context = null;
59-
assertThrows(IllegalStateException.class, extension::setupContainer);
58+
assertThrows(IllegalStateException.class, () -> extension.setupContainer(null));
6059
}
6160

6261
@Test
@@ -65,10 +64,9 @@ void testSetupContainerWithNullTestClass() {
6564
final ExtensionContext context = mock(ExtensionContext.class);
6665
when(context.getRequiredTestClass()).thenReturn(null); // Mock null test class
6766
when(context.getRequiredTestInstance()).thenReturn(new TestClass()); // Valid instance
68-
MavenDIExtension.context = context;
6967
assertThrows(
7068
IllegalStateException.class,
71-
extension::setupContainer,
69+
() -> extension.setupContainer(context),
7270
"Should throw IllegalStateException for null test class");
7371
}
7472

0 commit comments

Comments
 (0)