From 5e554fcc45d7944fd888c9434a199427a2691494 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 18:37:24 -0700 Subject: [PATCH 01/15] refactor(MTEExtension): encapsulate use of helperContexts --- .../MTEExtension.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 20b5477..b0d00ba 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -55,15 +55,14 @@ public class MTEExtension implements BeforeAllCallback, AfterAllCallback, Parame @Override public void afterAll(ExtensionContext context) throws Exception { - Class testClass = context.getRequiredTestClass(); - if (testClass.isAnnotationPresent(Nested.class)) { + if (context.getRequiredTestClass().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(); + if (getHelper(context) != null) { + getHelper(context).tearDown(); } } @@ -71,8 +70,6 @@ public void afterAll(ExtensionContext context) throws Exception { public void beforeAll(ExtensionContext context) throws Exception { 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; } @@ -88,12 +85,12 @@ public void beforeAll(ExtensionContext context) throws Exception { helperContext.setWorldGeneratorUri(useWorldGenerator.value()); } helperContext.setup(); - helperContexts.put(context.getRequiredTestClass(), helperContext); + setHelper(context, 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 +98,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); @@ -116,7 +113,7 @@ private Object getDIInstance(ModuleTestingHelper helper, Class type) { @Override public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) throws Exception { - ModuleTestingHelper helper = helperContexts.get(extensionContext.getRequiredTestClass()); + ModuleTestingHelper helper = getHelper(extensionContext); List exceptionList = new LinkedList<>(); Class type = testInstance.getClass(); while (type != null) { @@ -140,6 +137,17 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } + private ModuleTestingHelper getHelper(ExtensionContext context) { + Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) + ? context.getRequiredTestClass().getEnclosingClass() + : context.getRequiredTestClass(); + return helperContexts.get(contextClass); + } + + private void setHelper(ExtensionContext context, ModuleTestingHelper helperContext) { + helperContexts.put(context.getRequiredTestClass(), helperContext); + } + /** * Apply our default logback configuration to the logger. *

From 81e22b6f02fb7b1a279adabc151682e63d6f86c8 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 18:47:12 -0700 Subject: [PATCH 02/15] refactor(MTEExtension): factor out ModuleTestingHelper building code --- .../MTEExtension.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index b0d00ba..94dbfbe 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -69,23 +69,10 @@ public void afterAll(ExtensionContext context) throws Exception { @Override public void beforeAll(ExtensionContext context) throws Exception { if (context.getRequiredTestClass().isAnnotationPresent(Nested.class)) { - // nested classes get set up in the parent - 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(); - setHelper(context, helperContext); + setHelper(context, setupHelper(context.getRequiredTestClass())); } @Override @@ -137,6 +124,20 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } + private ModuleTestingHelper setupHelper(Class testClass) throws Exception { + Dependencies dependencies = testClass.getAnnotation(Dependencies.class); + UseWorldGenerator useWorldGenerator = testClass.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(); + return helperContext; + } + private ModuleTestingHelper getHelper(ExtensionContext context) { Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) ? context.getRequiredTestClass().getEnclosingClass() From acb9a398b6b558fe8386d96541246c74b97fb88f Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 19:00:16 -0700 Subject: [PATCH 03/15] fix(MTEExtension): do not hold on to Helper instances after teardown --- .../MTEExtension.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 94dbfbe..e4ec520 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -59,11 +59,7 @@ public void afterAll(ExtensionContext context) throws Exception { // nested classes get torn down in the parent return; } - - // Could be null if an exception interrupts before setup is complete. - if (getHelper(context) != null) { - getHelper(context).tearDown(); - } + disposeHelper(context); } @Override @@ -72,7 +68,7 @@ public void beforeAll(ExtensionContext context) throws Exception { return; // nested classes get set up in the parent } setupLogging(); - setHelper(context, setupHelper(context.getRequiredTestClass())); + getHelper(context); } @Override @@ -124,7 +120,7 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } - private ModuleTestingHelper setupHelper(Class testClass) throws Exception { + private static ModuleTestingHelper setupHelper(Class testClass) { Dependencies dependencies = testClass.getAnnotation(Dependencies.class); UseWorldGenerator useWorldGenerator = testClass.getAnnotation(UseWorldGenerator.class); ModuleTestingHelper helperContext = new ModuleTestingHelper(); @@ -134,7 +130,13 @@ private ModuleTestingHelper setupHelper(Class testClass) throws Exception { if (useWorldGenerator != null) { helperContext.setWorldGeneratorUri(useWorldGenerator.value()); } - helperContext.setup(); + try { + helperContext.setup(); + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); + } return helperContext; } @@ -142,11 +144,15 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) ? context.getRequiredTestClass().getEnclosingClass() : context.getRequiredTestClass(); - return helperContexts.get(contextClass); + return helperContexts.computeIfAbsent(contextClass, MTEExtension::setupHelper); } - private void setHelper(ExtensionContext context, ModuleTestingHelper helperContext) { - helperContexts.put(context.getRequiredTestClass(), helperContext); + private void disposeHelper(ExtensionContext context) { + // Could be null if an exception interrupts before setup is complete. + ModuleTestingHelper helper = helperContexts.remove(context.getRequiredTestClass()); + if (helper != null) { + helper.tearDown(); + } } /** From 346a84aa0b07e401f2b17ae95143885db9c79f3c Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 19:54:03 -0700 Subject: [PATCH 04/15] test(IsolatedEngineTest): run in consistent order --- .../moduletestingenvironment/IsolatedEngineTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java index 8df37f3..eca3049 100644 --- a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java +++ b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java @@ -5,14 +5,17 @@ import com.google.common.collect.Sets; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; +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.extension.Dependencies; import org.terasology.moduletestingenvironment.fixtures.DummyComponent; import org.terasology.moduletestingenvironment.fixtures.DummyEvent; -import org.terasology.engine.registry.In; import java.util.Set; @@ -21,6 +24,7 @@ @ExtendWith(IsolatedMTEExtension.class) @Dependencies({"engine", "ModuleTestingEnvironment"}) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class IsolatedEngineTest { private final Set entityManagerSet = Sets.newHashSet(); private EntityRef entity; @@ -34,6 +38,7 @@ public void prepareEntityForTest() { } @Test + @Order(1) public void someTest() { // make sure we don't reuse the EntityManager assertFalse(entityManagerSet.contains(entityManager)); @@ -44,6 +49,7 @@ public void someTest() { } @Test + @Order(2) public void someOtherTest() { // make sure we don't reuse the EntityManager assertFalse(entityManagerSet.contains(entityManager)); From 78fbbadfab2a248953342193613eb467d5c22268 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:04:57 -0700 Subject: [PATCH 05/15] perf(MTEExtension): use ExtensionStore for storing Helpers This lets Jupiter help dispose of them once the test is over. --- .../moduletestingenvironment/MTEExtension.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index e4ec520..71ac2e2 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -25,10 +25,8 @@ import java.io.IOException; import java.io.InputStream; import java.util.Arrays; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; -import java.util.Map; /** * Junit 5 Extension for using {@link ModuleTestingHelper} in your test. @@ -51,8 +49,6 @@ public class MTEExtension implements BeforeAllCallback, AfterAllCallback, Parame static final String LOGBACK_RESOURCE = "default-logback.xml"; - protected final Map, ModuleTestingHelper> helperContexts = new HashMap<>(); - @Override public void afterAll(ExtensionContext context) throws Exception { if (context.getRequiredTestClass().isAnnotationPresent(Nested.class)) { @@ -144,12 +140,17 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) ? context.getRequiredTestClass().getEnclosingClass() : context.getRequiredTestClass(); - return helperContexts.computeIfAbsent(contextClass, MTEExtension::setupHelper); + ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); + return store.getOrComputeIfAbsent(ModuleTestingHelper.class, MTEExtension::setupHelper, ModuleTestingHelper.class); } private void disposeHelper(ExtensionContext context) { + Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) + ? context.getRequiredTestClass().getEnclosingClass() + : context.getRequiredTestClass(); + ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); // Could be null if an exception interrupts before setup is complete. - ModuleTestingHelper helper = helperContexts.remove(context.getRequiredTestClass()); + ModuleTestingHelper helper = store.remove(ModuleTestingHelper.class, ModuleTestingHelper.class); if (helper != null) { helper.tearDown(); } From fda6e93a11507c39be7b2d4b7a0c5d352a86a5b8 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:36:00 -0700 Subject: [PATCH 06/15] fix(MTEExtension): use extension CloseableResource to dispose of old contexts This helps it be called at the right time regardless of how the test lifecycle is configured. --- .../IsolatedMTEExtension.java | 21 ++------- .../MTEExtension.java | 46 ++++++++----------- .../IsolatedEngineTest.java | 4 +- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java index 21487db..00b2bc1 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java @@ -1,10 +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; @@ -17,25 +15,14 @@ *

* Use this within {@link org.junit.jupiter.api.extension.ExtendWith} */ -public class IsolatedMTEExtension extends MTEExtension implements BeforeAllCallback, AfterAllCallback, - AfterEachCallback, ParameterResolver, TestInstancePostProcessor { +public class IsolatedMTEExtension extends MTEExtension implements BeforeAllCallback, ParameterResolver, TestInstancePostProcessor { @Override - public void afterAll(ExtensionContext context) throws Exception { + public void beforeAll(ExtensionContext context) { // 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 { + public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) { // beforeEach would be run after postProcess so postProcess would NPE, so we initialize the MTE here beforehand super.beforeAll(extensionContext); super.postProcessTestInstance(testInstance, extensionContext); diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 71ac2e2..fc25d26 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; @@ -45,21 +44,12 @@ *

* 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"; @Override - public void afterAll(ExtensionContext context) throws Exception { - if (context.getRequiredTestClass().isAnnotationPresent(Nested.class)) { - // nested classes get torn down in the parent - return; - } - disposeHelper(context); - } - - @Override - public void beforeAll(ExtensionContext context) throws Exception { + public void beforeAll(ExtensionContext context) { if (context.getRequiredTestClass().isAnnotationPresent(Nested.class)) { return; // nested classes get set up in the parent } @@ -91,7 +81,7 @@ private Object getDIInstance(ModuleTestingHelper helper, Class type) { } @Override - public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) throws Exception { + public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) { ModuleTestingHelper helper = getHelper(extensionContext); List exceptionList = new LinkedList<>(); Class type = testInstance.getClass(); @@ -141,19 +131,10 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { ? context.getRequiredTestClass().getEnclosingClass() : context.getRequiredTestClass(); ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); - return store.getOrComputeIfAbsent(ModuleTestingHelper.class, MTEExtension::setupHelper, ModuleTestingHelper.class); - } - - private void disposeHelper(ExtensionContext context) { - Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) - ? context.getRequiredTestClass().getEnclosingClass() - : context.getRequiredTestClass(); - ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); - // Could be null if an exception interrupts before setup is complete. - ModuleTestingHelper helper = store.remove(ModuleTestingHelper.class, ModuleTestingHelper.class); - if (helper != null) { - helper.tearDown(); - } + HelperCleaner autoCleaner = store.getOrComputeIfAbsent( + HelperCleaner.class, k -> new HelperCleaner(setupHelper(contextClass)), + HelperCleaner.class); + return autoCleaner.helper; } /** @@ -190,4 +171,17 @@ void setupLogging() { StatusPrinter.printInCaseOfErrorsOrWarnings(context); } } + + static class HelperCleaner implements ExtensionContext.Store.CloseableResource { + final ModuleTestingHelper helper; + + protected HelperCleaner(ModuleTestingHelper helper) { + this.helper = helper; + } + + @Override + public void close() { + helper.tearDown(); + } + } } diff --git a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java index eca3049..24fae57 100644 --- a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java +++ b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java @@ -7,6 +7,7 @@ import org.junit.jupiter.api.BeforeEach; 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.TestMethodOrder; import org.junit.jupiter.api.extension.ExtendWith; @@ -22,8 +23,9 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +@Tag("MteTest") @ExtendWith(IsolatedMTEExtension.class) -@Dependencies({"engine", "ModuleTestingEnvironment"}) +@Dependencies("ModuleTestingEnvironment") @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class IsolatedEngineTest { private final Set entityManagerSet = Sets.newHashSet(); From c13b5a670a20e7152c5b6ff0843fc828966f96a7 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:52:42 -0700 Subject: [PATCH 07/15] refactor: factor out getTopTestClass --- .../moduletestingenvironment/MTEExtension.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index fc25d26..a76541d 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -127,9 +127,7 @@ private static ModuleTestingHelper setupHelper(Class testClass) { } private ModuleTestingHelper getHelper(ExtensionContext context) { - Class contextClass = context.getRequiredTestClass().isAnnotationPresent(Nested.class) - ? context.getRequiredTestClass().getEnclosingClass() - : context.getRequiredTestClass(); + Class contextClass = getTopTestClass(context); ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( HelperCleaner.class, k -> new HelperCleaner(setupHelper(contextClass)), @@ -137,6 +135,11 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { return autoCleaner.helper; } + protected static Class getTopTestClass(ExtensionContext context) { + Class testClass = context.getRequiredTestClass(); + return testClass.isAnnotationPresent(Nested.class) ? testClass.getEnclosingClass() : testClass; + } + /** * Apply our default logback configuration to the logger. *

From 139e82aa7efcf9ff017d3248ec8d1f732ab5d904 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:56:50 -0700 Subject: [PATCH 08/15] refactor: let setupHelper find its test class from context --- .../terasology/moduletestingenvironment/MTEExtension.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index a76541d..265ef36 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -106,7 +106,8 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } - private static ModuleTestingHelper setupHelper(Class testClass) { + private static ModuleTestingHelper setupHelper(ExtensionContext context) { + Class testClass = getTopTestClass(context); Dependencies dependencies = testClass.getAnnotation(Dependencies.class); UseWorldGenerator useWorldGenerator = testClass.getAnnotation(UseWorldGenerator.class); ModuleTestingHelper helperContext = new ModuleTestingHelper(); @@ -130,7 +131,7 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { Class contextClass = getTopTestClass(context); ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( - HelperCleaner.class, k -> new HelperCleaner(setupHelper(contextClass)), + HelperCleaner.class, k -> new HelperCleaner(setupHelper(context)), HelperCleaner.class); return autoCleaner.helper; } From 4517093482867478f605eae279641c6b9f4e156b Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 21:02:04 -0700 Subject: [PATCH 09/15] refactor: factor out getNamespace --- .../terasology/moduletestingenvironment/MTEExtension.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 265ef36..192a41f 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -128,14 +128,17 @@ private static ModuleTestingHelper setupHelper(ExtensionContext context) { } private ModuleTestingHelper getHelper(ExtensionContext context) { - Class contextClass = getTopTestClass(context); - ExtensionContext.Store store = context.getStore(ExtensionContext.Namespace.create(getClass(), contextClass)); + ExtensionContext.Store store = context.getStore(getNamespace(context)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( HelperCleaner.class, k -> new HelperCleaner(setupHelper(context)), HelperCleaner.class); return autoCleaner.helper; } + protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { + return ExtensionContext.Namespace.create(MTEExtension.class, getTopTestClass(context)); + } + protected static Class getTopTestClass(ExtensionContext context) { Class testClass = context.getRequiredTestClass(); return testClass.isAnnotationPresent(Nested.class) ? testClass.getEnclosingClass() : testClass; From a26f88eb179bc0269e28cc8afd59080e102e55df Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Wed, 1 Sep 2021 21:13:23 -0700 Subject: [PATCH 10/15] refactor(IsolatedMTEExtension): use more precise namespace for increased isolation --- .../IsolatedMTEExtension.java | 16 +++------------- .../moduletestingenvironment/MTEExtension.java | 4 ++-- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java index 00b2bc1..de46752 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java @@ -3,10 +3,7 @@ package org.terasology.moduletestingenvironment; -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 @@ -15,16 +12,9 @@ *

* Use this within {@link org.junit.jupiter.api.extension.ExtendWith} */ -public class IsolatedMTEExtension extends MTEExtension implements BeforeAllCallback, ParameterResolver, TestInstancePostProcessor { +public class IsolatedMTEExtension extends MTEExtension { @Override - public void beforeAll(ExtensionContext context) { - // don't call super - } - - @Override - public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) { - // beforeEach would be run after postProcess so postProcess would NPE, so we initialize the MTE here beforehand - super.beforeAll(extensionContext); - super.postProcessTestInstance(testInstance, extensionContext); + protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { + return ExtensionContext.Namespace.create(MTEExtension.class, context.getTestMethod()); } } diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 192a41f..1fcbb5f 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -54,7 +54,6 @@ public void beforeAll(ExtensionContext context) { return; // nested classes get set up in the parent } setupLogging(); - getHelper(context); } @Override @@ -180,7 +179,7 @@ void setupLogging() { } static class HelperCleaner implements ExtensionContext.Store.CloseableResource { - final ModuleTestingHelper helper; + ModuleTestingHelper helper; protected HelperCleaner(ModuleTestingHelper helper) { this.helper = helper; @@ -189,6 +188,7 @@ protected HelperCleaner(ModuleTestingHelper helper) { @Override public void close() { helper.tearDown(); + helper = null; } } } From 8273e4ff2999d5427ace2fd44ccc50c59e2f485e Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 4 Sep 2021 11:01:12 -0700 Subject: [PATCH 11/15] doc(MTEExtension): javadoc for methods managing the Helper --- .../IsolatedMTEExtension.java | 2 + .../MTEExtension.java | 42 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java index de46752..1bbe0c2 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java @@ -15,6 +15,8 @@ public class IsolatedMTEExtension extends MTEExtension { @Override protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { + // Resources are not shared between namespaces. We increase isolation by using a different + // namespace for every test method. return ExtensionContext.Namespace.create(MTEExtension.class, context.getTestMethod()); } } diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 1fcbb5f..86b5822 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -105,6 +105,15 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } + /** + * Create a ModuleTestingHelper configured for this test. + *

+ * The new ModuleTestingHelper instance is configured using the {@link Dependencies} and {@link UseWorldGenerator} + * annotations for the test class. + * + * @param context for the current test + * @return new instance + */ private static ModuleTestingHelper setupHelper(ExtensionContext context) { Class testClass = getTopTestClass(context); Dependencies dependencies = testClass.getAnnotation(Dependencies.class); @@ -126,6 +135,16 @@ private static ModuleTestingHelper setupHelper(ExtensionContext context) { return helperContext; } + /** + * Get the ModuleTestingHelper for this test. + *

+ * This will {@linkplain #setupHelper 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 + */ private ModuleTestingHelper getHelper(ExtensionContext context) { ExtensionContext.Store store = context.getStore(getNamespace(context)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( @@ -134,10 +153,25 @@ private ModuleTestingHelper getHelper(ExtensionContext context) { return autoCleaner.helper; } + /** The Namespace to store the ModuleTestingHelper state in. */ protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { - return ExtensionContext.Namespace.create(MTEExtension.class, getTopTestClass(context)); + return ExtensionContext.Namespace.create( + MTEExtension.class, // Start with this Extension, so it's clear where this came from. + getTopTestClass(context) // one namespace per test class + ); } + /** + * 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 + */ protected static Class getTopTestClass(ExtensionContext context) { Class testClass = context.getRequiredTestClass(); return testClass.isAnnotationPresent(Nested.class) ? testClass.getEnclosingClass() : testClass; @@ -178,6 +212,12 @@ void setupLogging() { } } + /** + * 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 { ModuleTestingHelper helper; From 8820ec224deeddf09083caff27a5daccf0149927 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 4 Sep 2021 17:34:41 -0700 Subject: [PATCH 12/15] refactor: make isolation level configurable without subclassing --- .../IsolatedMTEExtension.java | 7 +- .../MTEExtension.java | 30 +----- .../moduletestingenvironment/Scopes.java | 42 +++++++++ .../IsolatedEngineTest.java | 4 +- ...MTEExtensionTestWithPerClassLifecycle.java | 94 +++++++++++++++++++ 5 files changed, 144 insertions(+), 33 deletions(-) create mode 100644 src/main/java/org/terasology/moduletestingenvironment/Scopes.java create mode 100644 src/test/java/org/terasology/moduletestingenvironment/MTEExtensionTestWithPerClassLifecycle.java diff --git a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java index 1bbe0c2..a497810 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/IsolatedMTEExtension.java @@ -3,8 +3,6 @@ package org.terasology.moduletestingenvironment; -import org.junit.jupiter.api.extension.ExtensionContext; - /** * 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 @@ -13,10 +11,9 @@ * Use this within {@link org.junit.jupiter.api.extension.ExtendWith} */ public class IsolatedMTEExtension extends MTEExtension { - @Override - protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { + { // Resources are not shared between namespaces. We increase isolation by using a different // namespace for every test method. - return ExtensionContext.Namespace.create(MTEExtension.class, context.getTestMethod()); + 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 86b5822..85ba3fb 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -26,6 +26,7 @@ import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import java.util.function.Function; /** * Junit 5 Extension for using {@link ModuleTestingHelper} in your test. @@ -47,6 +48,7 @@ public class MTEExtension implements BeforeAllCallback, ParameterResolver, TestInstancePostProcessor { static final String LOGBACK_RESOURCE = "default-logback.xml"; + protected Function helperLifecycle = Scopes.PER_CLASS; @Override public void beforeAll(ExtensionContext context) { @@ -115,7 +117,7 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens * @return new instance */ private static ModuleTestingHelper setupHelper(ExtensionContext context) { - Class testClass = getTopTestClass(context); + Class testClass = Scopes.getTopTestClass(context); Dependencies dependencies = testClass.getAnnotation(Dependencies.class); UseWorldGenerator useWorldGenerator = testClass.getAnnotation(UseWorldGenerator.class); ModuleTestingHelper helperContext = new ModuleTestingHelper(); @@ -146,37 +148,13 @@ private static ModuleTestingHelper setupHelper(ExtensionContext context) { * @return configured for this test */ private ModuleTestingHelper getHelper(ExtensionContext context) { - ExtensionContext.Store store = context.getStore(getNamespace(context)); + ExtensionContext.Store store = context.getStore(helperLifecycle.apply(context)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( HelperCleaner.class, k -> new HelperCleaner(setupHelper(context)), HelperCleaner.class); return autoCleaner.helper; } - /** The Namespace to store the ModuleTestingHelper state in. */ - protected ExtensionContext.Namespace getNamespace(ExtensionContext context) { - return ExtensionContext.Namespace.create( - MTEExtension.class, // Start with this Extension, so it's clear where this came from. - getTopTestClass(context) // one namespace per test class - ); - } - - /** - * 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 - */ - protected static Class getTopTestClass(ExtensionContext context) { - Class testClass = context.getRequiredTestClass(); - return testClass.isAnnotationPresent(Nested.class) ? testClass.getEnclosingClass() : testClass; - } - /** * Apply our default logback configuration to the logger. *

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 24fae57..deebd9f 100644 --- a/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java +++ b/src/test/java/org/terasology/moduletestingenvironment/IsolatedEngineTest.java @@ -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).dummy); + assertFalse(entity.getComponent(DummyComponent.class).dummy, + "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..c4f1d26 --- /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.dummy); + + // 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.dummy); + 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())); + } +} From d027f718c6b13ad31160a257b2b730592e089681 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 4 Sep 2021 18:53:36 -0700 Subject: [PATCH 13/15] chore(ModuleTestingEnvironment): manage our own reference for dependencies Easier to keep track of who owns which references if our private field is final instead of replacing it with a mutable object passed in. --- .../moduletestingenvironment/ModuleTestingEnvironment.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java b/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java index 704d98a..312f2e2 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java +++ b/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java @@ -151,7 +151,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; @@ -217,7 +217,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); + } } /** From 7f4d9a17c22cdbc28a3febc6c730885ee0c6b90b Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 4 Sep 2021 18:54:46 -0700 Subject: [PATCH 14/15] chore(ModuleTestingEnvironment): no longer throw checked exceptions from setup --- .../ModuleTestingEnvironment.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java b/src/main/java/org/terasology/moduletestingenvironment/ModuleTestingEnvironment.java index 312f2e2..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; @@ -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); From c5c9f35fe01c5d32e3d5ef36a1ee812c89a2c87d Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 4 Sep 2021 19:09:02 -0700 Subject: [PATCH 15/15] refactor(MTEExtension): increase separation between Extension and ModuleTestingHelper Things relating to our annotations and implementing Jupiter's extension methods go on MTEExtension, separated from methods that have to know how to configure a ModuleTestingHelper. Move the method that creates new ModuleTestingHelper instances somewhere less conspicuous, to make people more likely to stick to getHelper. --- .../MTEExtension.java | 69 ++++++++++--------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java index 85ba3fb..cd0516b 100644 --- a/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java +++ b/src/main/java/org/terasology/moduletestingenvironment/MTEExtension.java @@ -24,8 +24,10 @@ import java.io.IOException; import java.io.InputStream; 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; /** @@ -49,6 +51,7 @@ public class MTEExtension implements BeforeAllCallback, ParameterResolver, TestI static final String LOGBACK_RESOURCE = "default-logback.xml"; protected Function helperLifecycle = Scopes.PER_CLASS; + protected Function> getTestClass = Scopes::getTopTestClass; @Override public void beforeAll(ExtensionContext context) { @@ -107,50 +110,33 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext extens } } - /** - * Create a ModuleTestingHelper configured for this test. - *

- * The new ModuleTestingHelper instance is configured using the {@link Dependencies} and {@link UseWorldGenerator} - * annotations for the test class. - * - * @param context for the current test - * @return new instance - */ - private static ModuleTestingHelper setupHelper(ExtensionContext context) { - Class testClass = Scopes.getTopTestClass(context); - Dependencies dependencies = testClass.getAnnotation(Dependencies.class); - UseWorldGenerator useWorldGenerator = testClass.getAnnotation(UseWorldGenerator.class); - ModuleTestingHelper helperContext = new ModuleTestingHelper(); - if (dependencies != null) { - helperContext.setDependencies(Sets.newHashSet(dependencies.value())); - } - if (useWorldGenerator != null) { - helperContext.setWorldGeneratorUri(useWorldGenerator.value()); - } - try { - helperContext.setup(); - } catch (RuntimeException e) { - throw e; - } catch (Exception e) { - throw new RuntimeException(e); - } - return helperContext; + 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. *

- * This will {@linkplain #setupHelper create a new instance} when necessary. It will be stored in the + * 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 */ - private ModuleTestingHelper getHelper(ExtensionContext context) { + protected ModuleTestingHelper getHelper(ExtensionContext context) { ExtensionContext.Store store = context.getStore(helperLifecycle.apply(context)); HelperCleaner autoCleaner = store.getOrComputeIfAbsent( - HelperCleaner.class, k -> new HelperCleaner(setupHelper(context)), + HelperCleaner.class, k -> new HelperCleaner(getDependencyNames(context), getWorldGeneratorUri(context)), HelperCleaner.class); return autoCleaner.helper; } @@ -197,12 +183,29 @@ void setupLogging() { * the {@link ModuleTestingHelper} when the context is closed. */ static class HelperCleaner implements ExtensionContext.Store.CloseableResource { - ModuleTestingHelper helper; + protected ModuleTestingHelper helper; - protected HelperCleaner(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();