Fix programmatic extension registration for nested classes

Prior to this commit, extensions registered programmatically using the
`RegisterExtension` annotation were added to the wrong extension
registry multiple times for nested classes and their descendants.
Instead of storing them in the registry on the same level as the test
instance, they were always stored in the registry of the enclosing
class.

`TestInstancesProvider` now takes two arguments: An unmodifiable
`ExtensionRegistry` that is used to instantiate and initialize the test
instance and an extension registrar that is used to register additional
extensions that are registered programmatically. Except for nested
classes these are always the same. For nested classes, we need to
ensure that extensions registered on their level and below are not used
to initialize enclosing classes. Thus, the parent's registry is used
for initialization but additional extensions from fields of the class
are registered with the current registry (usually the registry of the
test method, unless Lifecycle.PER_CLASS is used).

Fixes #1925.

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java

@@ -54,7 +54,9 @@
 import org.junit.jupiter.engine.execution.ExecutableInvoker.ReflectiveInterceptorCall.VoidMethodInterceptorCall;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
 import org.junit.jupiter.engine.execution.TestInstancesProvider;
+import org.junit.jupiter.engine.extension.ExtensionRegistrar;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.util.BlacklistedExceptions;
 import org.junit.platform.commons.util.ExceptionUtils;
@@ -135,7 +137,7 @@ public Set<ExclusiveResource> getExclusiveResources() {
 
 	@Override
 	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) {
-		ExtensionRegistry registry = populateNewExtensionRegistryFromExtendWithAnnotation(
+		MutableExtensionRegistry registry = populateNewExtensionRegistryFromExtendWithAnnotation(
 			context.getExtensionRegistry(), this.testClass);
 
 		// Register extensions from static fields here, at the class level but
@@ -159,7 +161,7 @@ public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext conte
 
 		// @formatter:off
 		return context.extend()
-				.withTestInstancesProvider(testInstancesProvider(context, registry, extensionContext))
+				.withTestInstancesProvider(testInstancesProvider(context, extensionContext))
 				.withExtensionRegistry(registry)
 				.withExtensionContext(extensionContext)
 				.withThrowableCollector(throwableCollector)
@@ -177,7 +179,7 @@ public JupiterEngineExecutionContext before(JupiterEngineExecutionContext contex
 			// and store the instance in the ExtensionContext.
 			ClassExtensionContext extensionContext = (ClassExtensionContext) context.getExtensionContext();
 			throwableCollector.execute(() -> extensionContext.setTestInstances(
-				context.getTestInstancesProvider().getTestInstances(Optional.empty())));
+				context.getTestInstancesProvider().getTestInstances(context.getExtensionRegistry())));
 		}
 
 		if (throwableCollector.isEmpty()) {
@@ -242,29 +244,26 @@ private TestInstanceFactory resolveTestInstanceFactory(ExtensionRegistry registr
 	}
 
 	private TestInstancesProvider testInstancesProvider(JupiterEngineExecutionContext parentExecutionContext,
-			ExtensionRegistry registry, ClassExtensionContext extensionContext) {
+			ClassExtensionContext extensionContext) {
 
-		TestInstancesProvider testInstancesProvider = childRegistry -> instantiateAndPostProcessTestInstance(
-			parentExecutionContext, extensionContext, childRegistry.orElse(registry));
-
-		return childRegistry -> extensionContext.getTestInstances().orElseGet(
-			() -> testInstancesProvider.getTestInstances(childRegistry));
+		return (registry, registrar) -> extensionContext.getTestInstances().orElseGet(
+			() -> instantiateAndPostProcessTestInstance(parentExecutionContext, extensionContext, registry, registrar));
 	}
 
 	private TestInstances instantiateAndPostProcessTestInstance(JupiterEngineExecutionContext parentExecutionContext,
-			ExtensionContext extensionContext, ExtensionRegistry registry) {
+			ExtensionContext extensionContext, ExtensionRegistry registry, ExtensionRegistrar registrar) {
 
-		TestInstances instances = instantiateTestClass(parentExecutionContext, registry, extensionContext);
+		TestInstances instances = instantiateTestClass(parentExecutionContext, registry, registrar, extensionContext);
 		invokeTestInstancePostProcessors(instances.getInnermostInstance(), registry, extensionContext);
 		// In addition, we register extensions from instance fields here since the
 		// best time to do that is immediately following test class instantiation
 		// and post processing.
-		registerExtensionsFromFields(registry, this.testClass, instances.getInnermostInstance());
+		registerExtensionsFromFields(registrar, this.testClass, instances.getInnermostInstance());
 		return instances;
 	}
 
 	protected abstract TestInstances instantiateTestClass(JupiterEngineExecutionContext parentExecutionContext,
-			ExtensionRegistry registry, ExtensionContext extensionContext);
+			ExtensionRegistry registry, ExtensionRegistrar registrar, ExtensionContext extensionContext);
 
 	protected TestInstances instantiateTestClass(Optional<TestInstances> outerInstances, ExtensionRegistry registry,
 			ExtensionContext extensionContext) {
@@ -420,12 +419,12 @@ private void invokeAfterAllCallbacks(JupiterEngineExecutionContext context) {
 				.forEach(extension -> throwableCollector.execute(() -> extension.afterAll(extensionContext)));
 	}
 
-	private void registerBeforeEachMethodAdapters(ExtensionRegistry registry) {
+	private void registerBeforeEachMethodAdapters(ExtensionRegistrar registrar) {
 		List<Method> beforeEachMethods = findBeforeEachMethods(this.testClass);
-		registerMethodsAsExtensions(beforeEachMethods, registry, this::synthesizeBeforeEachMethodAdapter);
+		registerMethodsAsExtensions(beforeEachMethods, registrar, this::synthesizeBeforeEachMethodAdapter);
 	}
 
-	private void registerAfterEachMethodAdapters(ExtensionRegistry registry) {
+	private void registerAfterEachMethodAdapters(ExtensionRegistrar registrar) {
 		// Make a local copy since findAfterEachMethods() returns an immutable list.
 		List<Method> afterEachMethods = new ArrayList<>(findAfterEachMethods(this.testClass));
 
@@ -435,13 +434,13 @@ private void registerAfterEachMethodAdapters(ExtensionRegistry registry) {
 		// we register them as synthesized extensions.
 		Collections.reverse(afterEachMethods);
 
-		registerMethodsAsExtensions(afterEachMethods, registry, this::synthesizeAfterEachMethodAdapter);
+		registerMethodsAsExtensions(afterEachMethods, registrar, this::synthesizeAfterEachMethodAdapter);
 	}
 
-	private void registerMethodsAsExtensions(List<Method> methods, ExtensionRegistry registry,
+	private void registerMethodsAsExtensions(List<Method> methods, ExtensionRegistrar registrar,
 			Function<Method, Extension> extensionSynthesizer) {
 
-		methods.forEach(method -> registry.registerExtension(extensionSynthesizer.apply(method), method));
+		methods.forEach(method -> registrar.registerExtension(extensionSynthesizer.apply(method), method));
 	}
 
 	private BeforeEachMethodAdapter synthesizeBeforeEachMethodAdapter(Method method) {

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java

@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.extension.TestInstances;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
+import org.junit.jupiter.engine.extension.ExtensionRegistrar;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestTag;
@@ -65,7 +66,7 @@ public ExecutionMode getExecutionMode() {
 
 	@Override
 	protected TestInstances instantiateTestClass(JupiterEngineExecutionContext parentExecutionContext,
-			ExtensionRegistry registry, ExtensionContext extensionContext) {
+			ExtensionRegistry registry, ExtensionRegistrar registrar, ExtensionContext extensionContext) {
 		return instantiateTestClass(Optional.empty(), registry, extensionContext);
 	}
 

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExtensionUtils.java

@@ -29,15 +29,17 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.Extension;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.ExtensionRegistrar;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.commons.util.ReflectionUtils;
 
 /**
  * Collection of utilities for working with extensions and the extension registry.
  *
  * @since 5.1
- * @see ExtensionRegistry
+ * @see ExtensionRegistrar
+ * @see MutableExtensionRegistry
  * @see ExtendWith
  * @see RegisterExtension
  */
@@ -48,7 +50,7 @@ private ExtensionUtils() {
 	}
 
 	/**
-	 * Populate a new {@link ExtensionRegistry} from extension types declared via
+	 * Populate a new {@link MutableExtensionRegistry} from extension types declared via
 	 * {@link ExtendWith @ExtendWith} on the supplied {@link AnnotatedElement}.
 	 *
 	 * @param parentRegistry the parent extension registry to set in the newly
@@ -59,8 +61,8 @@ private ExtensionUtils() {
 	 * @return the new extension registry; never {@code null}
 	 * @since 5.0
 	 */
-	static ExtensionRegistry populateNewExtensionRegistryFromExtendWithAnnotation(ExtensionRegistry parentRegistry,
-			AnnotatedElement annotatedElement) {
+	static MutableExtensionRegistry populateNewExtensionRegistryFromExtendWithAnnotation(
+			MutableExtensionRegistry parentRegistry, AnnotatedElement annotatedElement) {
 
 		Preconditions.notNull(parentRegistry, "Parent ExtensionRegistry must not be null");
 		Preconditions.notNull(annotatedElement, "AnnotatedElement must not be null");
@@ -72,7 +74,7 @@ static ExtensionRegistry populateNewExtensionRegistryFromExtendWithAnnotation(Ex
 				.collect(toList());
 		// @formatter:on
 
-		return ExtensionRegistry.createRegistryFrom(parentRegistry, extensionTypes);
+		return MutableExtensionRegistry.createRegistryFrom(parentRegistry, extensionTypes);
 	}
 
 	/**
@@ -82,13 +84,13 @@ static ExtensionRegistry populateNewExtensionRegistryFromExtendWithAnnotation(Ex
 	 * <p>The extensions will be sorted according to {@link Order @Order} semantics
 	 * prior to registration.
 	 *
-	 * @param registry the registry in which to register the extensions; never {@code null}
+	 * @param registrar the registrar with which to register the extensions; never {@code null}
 	 * @param clazz the class or interface in which to find the fields; never {@code null}
 	 * @param instance the instance of the supplied class; may be {@code null}
 	 * when searching for {@code static} fields in the class
 	 */
-	static void registerExtensionsFromFields(ExtensionRegistry registry, Class<?> clazz, Object instance) {
-		Preconditions.notNull(registry, "ExtensionRegistry must not be null");
+	static void registerExtensionsFromFields(ExtensionRegistrar registrar, Class<?> clazz, Object instance) {
+		Preconditions.notNull(registrar, "ExtensionRegistrar must not be null");
 		Preconditions.notNull(clazz, "Class must not be null");
 
 		Predicate<Field> predicate = (instance == null ? ReflectionUtils::isStatic : ReflectionUtils::isNotStatic);
@@ -108,7 +110,7 @@ static void registerExtensionsFromFields(ExtensionRegistry registry, Class<?> cl
 				Preconditions.condition(value instanceof Extension, () -> String.format(
 					"Failed to register extension via @RegisterExtension field [%s]: field value's type [%s] must implement an [%s] API.",
 					field, (value != null ? value.getClass().getName() : null), Extension.class.getName()));
-				registry.registerExtension((Extension) value, field);
+				registrar.registerExtension((Extension) value, field);
 			});
 		});
 	}

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterEngineDescriptor.java

@@ -12,13 +12,12 @@
 
 import static org.apiguardian.api.API.Status.INTERNAL;
 import static org.junit.jupiter.engine.descriptor.JupiterTestDescriptor.toExecutionMode;
-import static org.junit.jupiter.engine.extension.ExtensionRegistry.createRegistryWithDefaultExtensions;
 
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
-import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.engine.EngineExecutionListener;
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.support.descriptor.EngineDescriptor;
@@ -49,7 +48,8 @@ public ExecutionMode getExecutionMode() {
 
 	@Override
 	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) {
-		ExtensionRegistry extensionRegistry = createRegistryWithDefaultExtensions(context.getConfiguration());
+		MutableExtensionRegistry extensionRegistry = MutableExtensionRegistry.createRegistryWithDefaultExtensions(
+			context.getConfiguration());
 		EngineExecutionListener executionListener = context.getExecutionListener();
 		ExtensionContext extensionContext = new JupiterEngineExtensionContext(executionListener, this,
 			context.getConfiguration());

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java

@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.extension.TestInstances;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
+import org.junit.jupiter.engine.extension.ExtensionRegistrar;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestTag;
@@ -60,12 +61,12 @@ public final Set<TestTag> getTags() {
 
 	@Override
 	protected TestInstances instantiateTestClass(JupiterEngineExecutionContext parentExecutionContext,
-			ExtensionRegistry registry, ExtensionContext extensionContext) {
+			ExtensionRegistry registry, ExtensionRegistrar registrar, ExtensionContext extensionContext) {
 
-		// Extensions registered for nested classes and below are not to be used for instantiating outer classes
-		Optional<ExtensionRegistry> childExtensionRegistryForOuterInstance = Optional.empty();
+		// Extensions registered for nested classes and below are not to be used for instantiating and initializing outer classes
+		ExtensionRegistry extensionRegistryForOuterInstanceCreation = parentExecutionContext.getExtensionRegistry();
 		TestInstances outerInstances = parentExecutionContext.getTestInstancesProvider().getTestInstances(
-			childExtensionRegistryForOuterInstance);
+			extensionRegistryForOuterInstanceCreation, registrar);
 		return instantiateTestClass(Optional.of(outerInstances), registry, extensionContext);
 	}
 

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java

@@ -38,6 +38,7 @@
 import org.junit.jupiter.engine.execution.ExecutableInvoker.ReflectiveInterceptorCall;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.commons.logging.Logger;
 import org.junit.platform.commons.logging.LoggerFactory;
 import org.junit.platform.commons.util.BlacklistedExceptions;
@@ -96,13 +97,13 @@ public Type getType() {
 	// --- Node ----------------------------------------------------------------
 
 	@Override
-	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) throws Exception {
-		ExtensionRegistry registry = populateNewExtensionRegistry(context);
+	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) {
+		MutableExtensionRegistry registry = populateNewExtensionRegistry(context);
 		ThrowableCollector throwableCollector = createThrowableCollector();
 		MethodExtensionContext extensionContext = new MethodExtensionContext(context.getExtensionContext(),
 			context.getExecutionListener(), this, context.getConfiguration(), throwableCollector);
 		throwableCollector.execute(() -> {
-			TestInstances testInstances = context.getTestInstancesProvider().getTestInstances(Optional.of(registry));
+			TestInstances testInstances = context.getTestInstancesProvider().getTestInstances(registry);
 			extensionContext.setTestInstances(testInstances);
 		});
 
@@ -115,7 +116,7 @@ public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext conte
 		// @formatter:on
 	}
 
-	protected ExtensionRegistry populateNewExtensionRegistry(JupiterEngineExecutionContext context) {
+	protected MutableExtensionRegistry populateNewExtensionRegistry(JupiterEngineExecutionContext context) {
 		return populateNewExtensionRegistryFromExtendWithAnnotation(context.getExtensionRegistry(), getTestMethod());
 	}
 

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateInvocationTestDescriptor.java

@@ -22,7 +22,7 @@
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.ExecutableInvoker.ReflectiveInterceptorCall;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
-import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.support.hierarchical.ExclusiveResource;
@@ -63,8 +63,8 @@ public String getLegacyReportingName() {
 	}
 
 	@Override
-	protected ExtensionRegistry populateNewExtensionRegistry(JupiterEngineExecutionContext context) {
-		ExtensionRegistry registry = super.populateNewExtensionRegistry(context);
+	protected MutableExtensionRegistry populateNewExtensionRegistry(JupiterEngineExecutionContext context) {
+		MutableExtensionRegistry registry = super.populateNewExtensionRegistry(context);
 		invocationContext.getAdditionalExtensions().forEach(
 			extension -> registry.registerExtension(extension, invocationContext));
 		return registry;

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java

@@ -27,6 +27,7 @@
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
@@ -71,7 +72,7 @@ public boolean mayRegisterTests() {
 
 	@Override
 	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) throws Exception {
-		ExtensionRegistry registry = populateNewExtensionRegistryFromExtendWithAnnotation(
+		MutableExtensionRegistry registry = populateNewExtensionRegistryFromExtendWithAnnotation(
 			context.getExtensionRegistry(), getTestMethod());
 
 		// The test instance should be properly maintained by the enclosing class's ExtensionContext.

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/JupiterEngineExecutionContext.java

@@ -15,7 +15,7 @@
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
-import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.logging.Logger;
 import org.junit.platform.commons.logging.LoggerFactory;
@@ -71,7 +71,7 @@ public TestInstancesProvider getTestInstancesProvider() {
 		return this.state.testInstancesProvider;
 	}
 
-	public ExtensionRegistry getExtensionRegistry() {
+	public MutableExtensionRegistry getExtensionRegistry() {
 		return this.state.extensionRegistry;
 	}
 
@@ -125,7 +125,7 @@ private static final class State implements Cloneable {
 		final EngineExecutionListener executionListener;
 		final JupiterConfiguration configuration;
 		TestInstancesProvider testInstancesProvider;
-		ExtensionRegistry extensionRegistry;
+		MutableExtensionRegistry extensionRegistry;
 		ExtensionContext extensionContext;
 		ThrowableCollector throwableCollector;
 
@@ -160,7 +160,7 @@ public Builder withTestInstancesProvider(TestInstancesProvider testInstancesProv
 			return this;
 		}
 
-		public Builder withExtensionRegistry(ExtensionRegistry extensionRegistry) {
+		public Builder withExtensionRegistry(MutableExtensionRegistry extensionRegistry) {
 			newState().extensionRegistry = extensionRegistry;
 			return this;
 		}

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/TestInstancesProvider.java

@@ -12,11 +12,11 @@
 
 import static org.apiguardian.api.API.Status.INTERNAL;
 
-import java.util.Optional;
-
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.extension.TestInstances;
+import org.junit.jupiter.engine.extension.ExtensionRegistrar;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
+import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
 
 /**
  * @since 5.0
@@ -25,6 +25,10 @@
 @API(status = INTERNAL, since = "5.0")
 public interface TestInstancesProvider {
 
-	TestInstances getTestInstances(Optional<ExtensionRegistry> childExtensionRegistry);
+	default TestInstances getTestInstances(MutableExtensionRegistry extensionRegistry) {
+		return getTestInstances(extensionRegistry, extensionRegistry);
+	}
+
+	TestInstances getTestInstances(ExtensionRegistry extensionRegistry, ExtensionRegistrar extensionRegistrar);
 
 }