diff --git a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java index 21487db..a497810 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java @@ -1,15 +1,8 @@ -// Copyright 2020 The Terasology Foundation +// Copyright 2021 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.moduletestingenvironment; -import org.junit.jupiter.api.extension.AfterAllCallback; -import org.junit.jupiter.api.extension.AfterEachCallback; -import org.junit.jupiter.api.extension.BeforeAllCallback; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.api.extension.ParameterResolver; -import org.junit.jupiter.api.extension.TestInstancePostProcessor; - /** * Subclass of {@link MTEExtension} which isolates all test cases by creating a new engine for each test. This is much * slower since it runs the startup and shutdown process for all tests. You should use {@link MTEExtension} unless @@ -17,27 +10,10 @@ *

* Use this within {@link org.junit.jupiter.api.extension.ExtendWith} */ -public class IsolatedMTEExtension extends MTEExtension implements BeforeAllCallback, AfterAllCallback, - AfterEachCallback, ParameterResolver, TestInstancePostProcessor { - @Override - public void afterAll(ExtensionContext context) throws Exception { - // don't call super - } - - @Override - public void beforeAll(ExtensionContext context) throws Exception { - // don't call super - } - - @Override - public void afterEach(ExtensionContext context) throws Exception { - super.afterAll(context); - } - - @Override - public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) throws Exception { - // beforeEach would be run after postProcess so postProcess would NPE, so we initialize the MTE here beforehand - super.beforeAll(extensionContext); - super.postProcessTestInstance(testInstance, extensionContext); +public class IsolatedMTEExtension extends MTEExtension { + { + // Resources are not shared between namespaces. We increase isolation by using a different + // namespace for every test method. + helperLifecycle = Scopes.PER_METHOD; } } diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 20b5477..cd0516b 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -9,7 +9,6 @@ import ch.qos.logback.core.util.StatusPrinter; import com.google.common.collect.Sets; import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ParameterContext; @@ -25,10 +24,11 @@ import java.io.IOException; import java.io.InputStream; import java.util.Arrays; -import java.util.HashMap; +import java.util.Collections; import java.util.LinkedList; import java.util.List; -import java.util.Map; +import java.util.Set; +import java.util.function.Function; /** * Junit 5 Extension for using {@link ModuleTestingHelper} in your test. @@ -47,53 +47,23 @@ *

* Use this within {@link org.junit.jupiter.api.extension.ExtendWith} */ -public class MTEExtension implements BeforeAllCallback, AfterAllCallback, ParameterResolver, TestInstancePostProcessor { +public class MTEExtension implements BeforeAllCallback, ParameterResolver, TestInstancePostProcessor { static final String LOGBACK_RESOURCE = "default-logback.xml"; - - protected final Map, ModuleTestingHelper> helperContexts = new HashMap<>(); + protected Function helperLifecycle = Scopes.PER_CLASS; + protected Function> getTestClass = Scopes::getTopTestClass; @Override - public void afterAll(ExtensionContext context) throws Exception { - Class testClass = context.getRequiredTestClass(); - if (testClass.isAnnotationPresent(Nested.class)) { - // nested classes get torn down in the parent - return; - } - - // Could be null if an exception interrupts before setup is complete. - if (helperContexts.get(testClass) != null) { - helperContexts.get(testClass).tearDown(); - } - } - - @Override - public void beforeAll(ExtensionContext context) throws Exception { + public void beforeAll(ExtensionContext context) { if (context.getRequiredTestClass().isAnnotationPresent(Nested.class)) { - // nested classes get set up in the parent - ModuleTestingHelper parentHelper = helperContexts.get(context.getRequiredTestClass().getEnclosingClass()); - helperContexts.put(context.getRequiredTestClass(), parentHelper); - return; + return; // nested classes get set up in the parent } - setupLogging(); - - Dependencies dependencies = context.getRequiredTestClass().getAnnotation(Dependencies.class); - UseWorldGenerator useWorldGenerator = context.getRequiredTestClass().getAnnotation(UseWorldGenerator.class); - ModuleTestingHelper helperContext = new ModuleTestingHelper(); - if (dependencies != null) { - helperContext.setDependencies(Sets.newHashSet(dependencies.value())); - } - if (useWorldGenerator != null) { - helperContext.setWorldGeneratorUri(useWorldGenerator.value()); - } - helperContext.setup(); - helperContexts.put(context.getRequiredTestClass(), helperContext); } @Override public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException { - ModuleTestingHelper helper = helperContexts.get(extensionContext.getRequiredTestClass()); + ModuleTestingHelper helper = getHelper(extensionContext); return helper.getHostContext().get(parameterContext.getParameter().getType()) != null || parameterContext.getParameter().getType().equals(ModuleTestingEnvironment.class) || parameterContext.getParameter().getType().equals(ModuleTestingHelper.class); @@ -101,7 +71,7 @@ public boolean supportsParameter(ParameterContext parameterContext, ExtensionCon @Override public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException { - ModuleTestingHelper helper = helperContexts.get(extensionContext.getRequiredTestClass()); + ModuleTestingHelper helper = getHelper(extensionContext); Class type = parameterContext.getParameter().getType(); return getDIInstance(helper, type); @@ -115,8 +85,8 @@ private Object getDIInstance(ModuleTestingHelper helper, Class type) { } @Override - public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) throws Exception { - ModuleTestingHelper helper = helperContexts.get(extensionContext.getRequiredTestClass()); + public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) { + ModuleTestingHelper helper = getHelper(extensionContext); List exceptionList = new LinkedList<>(); Class type = testInstance.getClass(); while (type != null) { @@ -140,6 +110,37 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } + public String getWorldGeneratorUri(ExtensionContext context) { + UseWorldGenerator useWorldGenerator = getTestClass.apply(context).getAnnotation(UseWorldGenerator.class); + return useWorldGenerator != null ? useWorldGenerator.value() : null; + } + + public Set getDependencyNames(ExtensionContext context) { + Dependencies dependencies = getTestClass.apply(context).getAnnotation(Dependencies.class); + return dependencies != null ? Sets.newHashSet(dependencies.value()) : Collections.emptySet(); + } + + /** + * Get the ModuleTestingHelper for this test. + *

+ * The new ModuleTestingHelper instance is configured using the {@link Dependencies} and {@link UseWorldGenerator} + * annotations for the test class. + *

+ * This will create a new instance when necessary. It will be stored in the + * {@link ExtensionContext} for reuse between tests that wish to avoid the expense of creating a new + * instance every time, and will be disposed of when the context closes. + * + * @param context for the current test + * @return configured for this test + */ + protected ModuleTestingHelper getHelper(ExtensionContext context) { + ExtensionContext.Store store = context.getStore(helperLifecycle.apply(context)); + HelperCleaner autoCleaner = store.getOrComputeIfAbsent( + HelperCleaner.class, k -> new HelperCleaner(getDependencyNames(context), getWorldGeneratorUri(context)), + HelperCleaner.class); + return autoCleaner.helper; + } + /** * Apply our default logback configuration to the logger. *

@@ -174,4 +175,41 @@ void setupLogging() { StatusPrinter.printInCaseOfErrorsOrWarnings(context); } } + + /** + * Manages a ModuleTestingHelper for storage in an ExtensionContext. + *

+ * Implements {@link ExtensionContext.Store.CloseableResource CloseableResource} to dispose of + * the {@link ModuleTestingHelper} when the context is closed. + */ + static class HelperCleaner implements ExtensionContext.Store.CloseableResource { + protected ModuleTestingHelper helper; + + HelperCleaner(ModuleTestingHelper helper) { + this.helper = helper; + } + + HelperCleaner(Set dependencyNames, String worldGeneratorUri) { + this(setupHelper(new ModuleTestingHelper(), dependencyNames, worldGeneratorUri)); + } + + protected static ModuleTestingHelper setupHelper(ModuleTestingHelper helper, Set dependencyNames, + String worldGeneratorUri) { + // This is a shim to fit the existing ModuleTestingEnvironment interface, but + // I expect we can make things cleaner after we drop the old interface that is + // also pretending to be a TestCase class itself. + helper.setDependencies(dependencyNames); + if (worldGeneratorUri != null) { + helper.setWorldGeneratorUri(worldGeneratorUri); + } + helper.setup(); + return helper; + } + + @Override + public void close() { + helper.tearDown(); + helper = null; + } + } } diff --git a/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java b/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java index 704d98a..ee56eeb 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java +++ b/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java @@ -54,6 +54,7 @@ import org.terasology.gestalt.module.ModuleRegistry; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; @@ -151,7 +152,7 @@ public class ModuleTestingEnvironment { public static final long DEFAULT_SAFETY_TIMEOUT = 60000; public static final long DEFAULT_GAME_TIME_TIMEOUT = 30000; private static final Logger logger = LoggerFactory.getLogger(ModuleTestingEnvironment.class); - private Set dependencies = Sets.newHashSet("engine"); + private final Set dependencies = Sets.newHashSet("engine"); private String worldGeneratorUri = "moduletestingenvironment:dummy"; private boolean doneLoading; private TerasologyEngine host; @@ -166,13 +167,15 @@ public class ModuleTestingEnvironment { * Set up and start the engine as configured via this environment. *

* Every instance should be shut down properly by calling {@link #tearDown()}. - * - * @throws Exception */ @BeforeEach - public void setup() throws Exception { + public void setup() { mockPathManager(); - host = createHost(); + try { + host = createHost(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } ScreenGrabber grabber = mock(ScreenGrabber.class); hostContext.put(ScreenGrabber.class, grabber); CoreRegistry.put(GameEngine.class, host); @@ -217,7 +220,10 @@ public Set getDependencies() { */ void setDependencies(Set dependencies) { Preconditions.checkState(host == null, "You cannot set Dependencies after setup"); - this.dependencies = dependencies; + synchronized (this.dependencies) { + this.dependencies.clear(); + this.dependencies.addAll(dependencies); + } } /** diff --git a/src/main/java/org/terasology/moduletestingenvironment/Scopes.java b/src/main/java/org/terasology/moduletestingenvironment/Scopes.java new file mode 100644 index 0000000..4dfa6e0 --- /dev/null +++ b/src/main/java/org/terasology/moduletestingenvironment/Scopes.java @@ -0,0 +1,42 @@ +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.moduletestingenvironment; + +import com.google.common.collect.ObjectArrays; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.extension.ExtensionContext; + +import java.util.function.Function; + +public final class Scopes { + + /** One per top-level test class. */ + public static final Function PER_CLASS = context -> mteNamespace(getTopTestClass(context)); + + /** One per test method. */ + public static final Function PER_METHOD = context -> mteNamespace(context.getTestMethod()); + + private Scopes() { }; + + static ExtensionContext.Namespace mteNamespace(Object... parts) { + // Start with this Extension, so it's clear where this came from. + return ExtensionContext.Namespace.create(ObjectArrays.concat(MTEExtension.class, parts)); + } + + /** + * The outermost class defining this test. + *

+ * For nested tests, this + * returns the outermost class in which this test is nested. + *

+ * Most tests are not nested, in which case this returns the class defining the test. + * + * @param context for the current test + * @return a test class + */ + public static Class getTopTestClass(ExtensionContext context) { + Class testClass = context.getRequiredTestClass(); + return testClass.isAnnotationPresent(Nested.class) ? testClass.getEnclosingClass() : testClass; + } +} diff --git a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java index 5a20953..fafc4ac 100644 --- a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java +++ b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java @@ -25,7 +25,7 @@ @Tag("MteTest") @ExtendWith(IsolatedMTEExtension.class) -@Dependencies({"engine", "ModuleTestingEnvironment"}) +@Dependencies("ModuleTestingEnvironment") @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class IsolatedEngineTest { private final Set entityManagerSet = Sets.newHashSet(); @@ -55,8 +55,8 @@ public void someTest() { public void someOtherTest() { // make sure we don't reuse the EntityManager assertFalse(entityManagerSet.contains(entityManager)); - entityManagerSet.add(entityManager); - assertFalse(entity.getComponent(DummyComponent.class).eventReceived); + assertFalse(entity.getComponent(DummyComponent.class).eventReceived, + "This entity should not have its field set yet!"); } } diff --git a/src/test/java/org/terasology/moduletestingenvironment/MTEExtensionTestWithPerClassLifecycle.java b/src/test/java/org/terasology/moduletestingenvironment/MTEExtensionTestWithPerClassLifecycle.java new file mode 100644 index 0000000..a858d8f --- /dev/null +++ b/src/test/java/org/terasology/moduletestingenvironment/MTEExtensionTestWithPerClassLifecycle.java @@ -0,0 +1,94 @@ +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.moduletestingenvironment; + +import com.google.common.collect.Lists; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.extension.ExtendWith; +import org.terasology.engine.entitySystem.entity.EntityManager; +import org.terasology.engine.entitySystem.entity.EntityRef; +import org.terasology.engine.registry.In; +import org.terasology.moduletestingenvironment.fixtures.DummyComponent; +import org.terasology.moduletestingenvironment.fixtures.DummyEvent; + +import java.util.List; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Ensure a test class with a per-class Jupiter lifecycle can share an engine between tests. + */ +@Tag("MteTest") +@ExtendWith(MTEExtension.class) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) // Lifecycle of the test instance +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class MTEExtensionTestWithPerClassLifecycle { + @In + public EntityManager entityManager; + + // java 8 doesn't have ConcurrentSet + private final ConcurrentMap seenNames = new ConcurrentHashMap<>(); + + @BeforeAll + public void createEntity(TestInfo testInfo) { + // Create some entity to be shared by all the tests. + EntityRef entity = entityManager.create(new DummyComponent()); + + // Do some stuff to configure it. + entity.send(new DummyEvent()); + + entity.updateComponent(DummyComponent.class, component -> { + // Mark with something unique (and not reliant on the entity id system) + component.name = testInfo.getDisplayName() + "#" + UUID.randomUUID(); + return component; + }); + } + + @Test + @Order(1) + public void firstTestFindsThings() { + List entities = Lists.newArrayList(entityManager.getEntitiesWith(DummyComponent.class)); + // There should be one entity, created by the @BeforeAll method + assertEquals(1, entities.size()); + + DummyComponent component = entities.get(0).getComponent(DummyComponent.class); + assertTrue(component.eventReceived); + + // Remember that a test has seen this one. + assertNotNull(component.name); + assertFalse(seenNames.containsKey(component.name)); + seenNames.put(component.name, 1); + } + + @Test + @Order(2) + public void thingsStillExistForSecondTest() { + List entities = Lists.newArrayList(entityManager.getEntitiesWith(DummyComponent.class)); + // There should be one entity, created by the @BeforeAll method + assertEquals(1, entities.size()); + + // Make sure that this is the same one that the first test saw. + DummyComponent component = entities.get(0).getComponent(DummyComponent.class); + assertTrue(component.eventReceived); + assertNotNull(component.name); + assertTrue(seenNames.containsKey(component.name), () -> + String.format("This is not the same entity as seen in the first test!%n" + + "Current entity: %s%n" + + "Previously seen: %s", + component.name, seenNames.keySet())); + } +}