test(MTE): replace IsolatedMTEExtension with JUnit's TestInstance.Lifecycle

engine-tests/src/main/java/org/terasology/engine/integrationenvironment/Engines.java

@@ -84,7 +84,7 @@ public class Engines {
     TerasologyEngine host;
     private final NetworkMode networkMode;
 
-    public Engines(Set<String> dependencies, String worldGeneratorUri, NetworkMode networkMode) {
+    public Engines(List<String> dependencies, String worldGeneratorUri, NetworkMode networkMode) {
         this.networkMode = networkMode;
         this.dependencies.addAll(dependencies);
 

engine-tests/src/main/java/org/terasology/engine/integrationenvironment/jupiter/MTEExtension.java

@@ -3,32 +3,30 @@
 
 package org.terasology.engine.integrationenvironment.jupiter;
 
-import com.google.common.collect.Sets;
 import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.ParameterContext;
 import org.junit.jupiter.api.extension.ParameterResolutionException;
 import org.junit.jupiter.api.extension.ParameterResolver;
-import org.junit.jupiter.api.extension.TestInstancePostProcessor;
-import org.opentest4j.MultipleFailuresError;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.terasology.engine.integrationenvironment.Engines;
 import org.terasology.engine.integrationenvironment.MainLoop;
 import org.terasology.engine.integrationenvironment.ModuleTestingHelper;
-import org.terasology.engine.integrationenvironment.Scopes;
 import org.terasology.engine.network.NetworkMode;
 import org.terasology.engine.registry.In;
 import org.terasology.unittest.worlds.DummyWorldGenerator;
 
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
-import java.util.Set;
 import java.util.function.Function;
 
 import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
+import static org.terasology.engine.registry.InjectionHelper.inject;
 
 /**
  * Sets up a Terasology environment for use with your {@index JUnit} 5 test.
@@ -42,7 +40,7 @@
  * import org.junit.jupiter.api.Test;
  * import org.terasology.engine.registry.In;
  *
- * @{@link org.junit.jupiter.api.extension.ExtendWith}(MTEExtension.class)
+ * @{@link org.junit.jupiter.api.extension.ExtendWith ExtendWith}(MTEExtension.class)
  * @{@link Dependencies}("MyModule")
  * @{@link UseWorldGenerator}("Pathfinding:pathfinder")
  * public class ExampleTest {
@@ -77,23 +75,65 @@
  * </dl>
  *
  * <p>
- * Every class annotated with this will create a single instance of {@link Engines} and use it during execution of
- * all tests in the class. This also means that all engine instances are shared between all tests in the class. If you
- * want isolated engine instances try {@link IsolatedMTEExtension}.
+ * By default, JUnit uses a {@link org.junit.jupiter.api.TestInstance.Lifecycle#PER_METHOD PER_METHOD} lifecycle
+ * for test instances. The <i>instance</i> of your test class—i.e. {@code this} when your test method executes—
+ * is re-created for every {@code @Test} method. The {@link org.terasology.engine.core.GameEngine GameEngine}
+ * created by this extension follows the same rules, created for each test instance.
+ * <p>
+ * If you <em>don't</em> want the engine shut down and recreated for every test method, mark your test class
+ * for {@link org.junit.jupiter.api.TestInstance.Lifecycle#PER_CLASS PER_CLASS} lifecycle.
  * <p>
  * Note that classes marked {@link Nested} will share the engine context with their parent.
+ *
+ * @see <a href="https://junit.org/junit5/docs/current/user-guide/#writing-tests-test-instance-lifecycle"
+ *     >JUnit User Guide: Test Instance Lifecycle</a>
  */
-public class MTEExtension implements ParameterResolver, TestInstancePostProcessor {
+public class MTEExtension implements BeforeAllCallback, BeforeEachCallback, ParameterResolver {
 
     private static final Logger logger = LoggerFactory.getLogger(MTEExtension.class);
 
-    protected Function<ExtensionContext, ExtensionContext.Namespace> helperLifecycle = Scopes.PER_CLASS;
-    protected Function<ExtensionContext, Class<?>> getTestClass = Scopes::getTopTestClass;
+    @Override
+    public void beforeEach(ExtensionContext extContext) {
+        injectTestInstances(extContext);
+    }
+
+    @Override
+    public void beforeAll(ExtensionContext extContext) {
+        extContext.getTestInstance().ifPresentOrElse(
+            o -> injectTestInstances(extContext),
+            () -> {
+                var lifecycle = extContext.getTestInstanceLifecycle().orElse(TestInstance.Lifecycle.PER_METHOD);
+                if (!lifecycle.equals(TestInstance.Lifecycle.PER_METHOD)) {
+                    logger.warn("Unexpected: This {} test has no instance for {} in context {}",
+                            lifecycle, extContext.getUniqueId(), extContext);
+                }
+            });
+    }
+
+    private void injectTestInstances(ExtensionContext extContext) {
+        Engines engines = getEngines(extContext);
+        // Usually just one instance, but @Nested tests have one per level of nesting.
+        for (Object instance : extContext.getRequiredTestInstances().getAllInstances()) {
+            injectTestInstance(instance, engines);
+        }
+    }
+
+    protected void injectTestInstance(Object testInstance, Engines engines) {
+        var context = engines.getHostContext();
+        context.put(Engines.class, engines);
+        context.put(MainLoop.class, new MainLoop(engines));
+        context.put(ModuleTestingHelper.class, new ModuleTestingHelper(engines));
+        inject(testInstance, context);
+    }
 
     @Override
     public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
-        Class<?> type = parameterContext.getParameter().getType();
+        if (extensionContext.getTestInstance().isEmpty()) {
+            logger.debug("Cannot provide parameters to {} before we have a test instance.", parameterContext.getDeclaringExecutable());
+            return false;
+        }
         Engines engines = getEngines(extensionContext);
+        Class<?> type = parameterContext.getParameter().getType();
         return engines.getHostContext().get(type) != null
                 || type.isAssignableFrom(Engines.class)
                 || type.isAssignableFrom(MainLoop.class)
@@ -120,40 +160,14 @@ private Object getDIInstance(Engines engines, Class<?> type) {
         }
     }
 
-    @Override
-    public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) {
-        Engines engines = getEngines(extensionContext);
-        List<IllegalAccessException> exceptionList = new LinkedList<>();
-        Class<?> type = testInstance.getClass();
-        while (type != null) {
-            Arrays.stream(type.getDeclaredFields())
-                    .filter((field) -> field.getAnnotation(In.class) != null)
-                    .peek((field) -> field.setAccessible(true))
-                    .forEach((field) -> {
-                        Object candidateObject = getDIInstance(engines, field.getType());
-                        try {
-                            field.set(testInstance, candidateObject);
-                        } catch (IllegalAccessException e) {
-                            exceptionList.add(e);
-                        }
-                    });
-
-            type = type.getSuperclass();
-        }
-        // It is tests, then it is legal ;)
-        if (!exceptionList.isEmpty()) {
-            throw new MultipleFailuresError("I cannot provide DI instances:", exceptionList);
-        }
-    }
-
     public String getWorldGeneratorUri(ExtensionContext context) {
-        UseWorldGenerator useWorldGenerator = getTestClass.apply(context).getAnnotation(UseWorldGenerator.class);
-        return useWorldGenerator != null ? useWorldGenerator.value() : null;
+        return findAnnotation(context.getRequiredTestClass(), UseWorldGenerator.class)
+                .map(UseWorldGenerator::value).orElse(null);
     }
 
-    public Set<String> getDependencyNames(ExtensionContext context) {
-        Dependencies dependencies = getTestClass.apply(context).getAnnotation(Dependencies.class);
-        return dependencies != null ? Sets.newHashSet(dependencies.value()) : Collections.emptySet();
+    public List<String> getDependencyNames(ExtensionContext context) {
+        return findAnnotation(context.getRequiredTestClass(), Dependencies.class)
+                .map(a -> Arrays.asList(a.value())).orElse(Collections.emptyList());
     }
 
     public NetworkMode getNetworkMode(ExtensionContext context) {
@@ -181,7 +195,7 @@ private <T> T getAnnotationWithDefault(ExtensionContext context, Function<Integr
      * @return configured for this test
      */
     protected Engines getEngines(ExtensionContext context) {
-        ExtensionContext.Store store = context.getStore(helperLifecycle.apply(context));
+        ExtensionContext.Store store = context.getStore(namespaceFor(context));
         EnginesCleaner autoCleaner = store.getOrComputeIfAbsent(
                 EnginesCleaner.class, k -> new EnginesCleaner(
                         getDependencyNames(context),
@@ -192,6 +206,16 @@ protected Engines getEngines(ExtensionContext context) {
         return autoCleaner.engines;
     }
 
+    protected ExtensionContext.Namespace namespaceFor(ExtensionContext context) {
+        logger.debug("Seeking engines for {} : {}", context.getUniqueId(),
+                context.getTestInstance().orElse("[NO INSTANCE]"));
+        // Start with this Extension, so it's clear where this came from.
+        return ExtensionContext.Namespace.create(
+                MTEExtension.class,
+                context.getRequiredTestInstance()
+        );
+    }
+
     /**
      * Manages Engines for storage in an ExtensionContext.
      * <p>
@@ -201,7 +225,7 @@ protected Engines getEngines(ExtensionContext context) {
     static class EnginesCleaner implements ExtensionContext.Store.CloseableResource {
         protected Engines engines;
 
-        EnginesCleaner(Set<String> dependencyNames, String worldGeneratorUri, NetworkMode networkMode) {
+        EnginesCleaner(List<String> dependencyNames, String worldGeneratorUri, NetworkMode networkMode) {
             engines = new Engines(dependencyNames, worldGeneratorUri, networkMode);
             engines.setup();
         }

engine-tests/src/main/java/org/terasology/engine/integrationenvironment/jupiter/package-info.java

@@ -0,0 +1,5 @@
+// Copyright 2022 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+/** Integration Environment support for the JUnit Jupiter platform. */
+package org.terasology.engine.integrationenvironment.jupiter;