Merge branch 'develop' into perf/fasterNetworkShutdown

Author: jdrueckert

engine-tests/build.gradle

@@ -78,6 +78,9 @@ dependencies {
     api("org.junit.jupiter:junit-jupiter-api") {
         because("we export jupiter Extensions for module tests")
     }
+    api("com.google.truth:truth:1.1.3") {
+        because("we provide some helper classes")
+    }
     implementation("org.mockito:mockito-inline:3.12.4") {
         because("classes like HeadlessEnvironment use mocks")
     }

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

@@ -9,12 +9,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.terasology.engine.config.Config;
-import org.terasology.engine.config.SystemConfig;
 import org.terasology.engine.context.Context;
 import org.terasology.engine.core.GameEngine;
 import org.terasology.engine.core.PathManager;
 import org.terasology.engine.core.PathManagerProvider;
-import org.terasology.engine.core.TerasologyConstants;
 import org.terasology.engine.core.TerasologyEngine;
 import org.terasology.engine.core.TerasologyEngineBuilder;
 import org.terasology.engine.core.modes.GameState;
@@ -40,20 +38,18 @@
 import org.terasology.engine.network.NetworkSystem;
 import org.terasology.engine.registry.CoreRegistry;
 import org.terasology.engine.rendering.opengl.ScreenGrabber;
-import org.terasology.engine.rendering.world.viewDistance.ViewDistance;
-import org.terasology.engine.testUtil.WithUnittestModule;
-import org.terasology.gestalt.module.Module;
-import org.terasology.gestalt.module.ModuleMetadataJsonAdapter;
-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.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
+import static org.junit.platform.commons.support.ReflectionSupport.newInstance;
+
 /**
  * Manages game engines for tests.
  * <p>
@@ -78,15 +74,18 @@ public class Engines {
     protected boolean doneLoading;
     protected Context hostContext;
     protected final List<TerasologyEngine> engines = Lists.newArrayList();
+    protected final List<Class<? extends EngineSubsystem>> subsystems = Lists.newArrayList();
 
     PathManager pathManager;
     PathManagerProvider.Cleaner pathManagerCleaner;
     TerasologyEngine host;
     private final NetworkMode networkMode;
 
-    public Engines(List<String> dependencies, String worldGeneratorUri, NetworkMode networkMode) {
+    public Engines(List<String> dependencies, String worldGeneratorUri, NetworkMode networkMode,
+                   List<Class<? extends EngineSubsystem>> subsystems) {
         this.networkMode = networkMode;
         this.dependencies.addAll(dependencies);
+        this.subsystems.addAll(subsystems);
 
         if (worldGeneratorUri != null) {
             this.worldGeneratorUri = worldGeneratorUri;
@@ -142,7 +141,6 @@ public void tearDown() {
      */
     public Context createClient(MainLoop mainLoop) throws IOException {
         TerasologyEngine client = createHeadlessEngine();
-        client.getFromEngineContext(Config.class).getRendering().setViewDistance(ViewDistance.LEGALLY_BLIND);
 
         client.changeState(new StateMainMenu());
         if (!connectToHost(client, mainLoop)) {
@@ -182,11 +180,12 @@ public Context getHostContext() {
     TerasologyEngine createHeadlessEngine() throws IOException {
         TerasologyEngineBuilder terasologyEngineBuilder = new TerasologyEngineBuilder();
         terasologyEngineBuilder
-                .add(new WithUnittestModule())
+                .add(new IntegrationEnvironmentSubsystem())
                 .add(new HeadlessGraphics())
                 .add(new HeadlessTimer())
                 .add(new HeadlessAudio())
                 .add(new HeadlessInput());
+        createExtraSubsystems().forEach(terasologyEngineBuilder::add);
 
         return createEngine(terasologyEngineBuilder);
     }
@@ -195,16 +194,31 @@ TerasologyEngine createHeadlessEngine() throws IOException {
     TerasologyEngine createHeadedEngine() throws IOException {
         EngineSubsystem audio = new LwjglAudio();
         TerasologyEngineBuilder terasologyEngineBuilder = new TerasologyEngineBuilder()
-                .add(new WithUnittestModule())
+                .add(new IntegrationEnvironmentSubsystem())
                 .add(audio)
                 .add(new LwjglGraphics())
                 .add(new LwjglTimer())
                 .add(new LwjglInput())
                 .add(new OpenVRInput());
+        createExtraSubsystems().forEach(terasologyEngineBuilder::add);
 
         return createEngine(terasologyEngineBuilder);
     }
 
+    List<EngineSubsystem> createExtraSubsystems()  {
+        List<EngineSubsystem> instances = new ArrayList<>();
+        for (Class<? extends EngineSubsystem> clazz : subsystems) {
+            try {
+                EngineSubsystem subsystem = newInstance(clazz);
+                instances.add(subsystem);
+                logger.debug("Created new {}", subsystem);
+            } catch (RuntimeException e) {
+                throw new RuntimeException("Failed creating new " + clazz.getName(), e);
+            }
+        }
+        return instances;
+    }
+
     TerasologyEngine createEngine(TerasologyEngineBuilder terasologyEngineBuilder) throws IOException {
         System.setProperty(ModuleManager.LOAD_CLASSPATH_MODULES_PROPERTY, "true");
 
@@ -220,43 +234,11 @@ TerasologyEngine createEngine(TerasologyEngineBuilder terasologyEngineBuilder) t
 
         TerasologyEngine terasologyEngine = terasologyEngineBuilder.build();
         terasologyEngine.initialize();
-        registerCurrentDirectoryIfModule(terasologyEngine);
 
         engines.add(terasologyEngine);
         return terasologyEngine;
     }
 
-    /**
-     * In standalone module environments (i.e. Jenkins CI builds) the CWD is the module under test. When it uses MTE it very likely needs to
-     * load itself as a module, but it won't be loadable from the typical path such as ./modules. This means that modules using MTE would
-     * always fail CI tests due to failing to load themselves.
-     * <p>
-     * For these cases we try to load the CWD (via the installPath) as a module and put it in the global module registry.
-     * <p>
-     * This process is based on how ModuleManagerImpl uses ModulePathScanner to scan for available modules.
-     */
-    protected void registerCurrentDirectoryIfModule(TerasologyEngine terasologyEngine) {
-        Path installPath = PathManager.getInstance().getInstallPath();
-        ModuleManager moduleManager = terasologyEngine.getFromEngineContext(ModuleManager.class);
-        ModuleRegistry registry = moduleManager.getRegistry();
-        ModuleMetadataJsonAdapter metadataReader = moduleManager.getModuleMetadataReader();
-        moduleManager.getModuleFactory().getModuleMetadataLoaderMap()
-                .put(TerasologyConstants.MODULE_INFO_FILENAME.toString(), metadataReader);
-
-
-        try {
-            Module module = moduleManager.getModuleFactory().createModule(installPath.toFile());
-            if (module != null) {
-                registry.add(module);
-                logger.info("Added install path as module: {}", installPath);
-            } else {
-                logger.info("Install path does not appear to be a module: {}", installPath);
-            }
-        } catch (IOException e) {
-            logger.warn("Could not read install path as module at " + installPath);
-        }
-    }
-
     protected void mockPathManager() {
         PathManager originalPathManager = PathManager.getInstance();
         pathManager = Mockito.spy(originalPathManager);
@@ -265,11 +247,10 @@ protected void mockPathManager() {
         PathManagerProvider.setPathManager(pathManager);
     }
 
-    TerasologyEngine createHost(NetworkMode networkMode) throws IOException {
+    TerasologyEngine createHost(NetworkMode hostNetworkMode) throws IOException {
         TerasologyEngine host = createHeadlessEngine();
-        host.getFromEngineContext(SystemConfig.class).writeSaveGamesEnabled.set(false);
         host.subscribeToStateChange(new HeadlessStateChangeListener(host));
-        host.changeState(new TestingStateHeadlessSetup(dependencies, worldGeneratorUri, networkMode));
+        host.changeState(new TestingStateHeadlessSetup(dependencies, worldGeneratorUri, hostNetworkMode));
 
         doneLoading = false;
         host.subscribeToStateChange(() -> {

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

@@ -0,0 +1,84 @@
+// Copyright 2022 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.engine.integrationenvironment;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.terasology.engine.config.Config;
+import org.terasology.engine.config.SystemConfig;
+import org.terasology.engine.context.Context;
+import org.terasology.engine.core.GameEngine;
+import org.terasology.engine.core.PathManager;
+import org.terasology.engine.core.TerasologyConstants;
+import org.terasology.engine.core.module.ModuleManager;
+import org.terasology.engine.core.subsystem.EngineSubsystem;
+import org.terasology.engine.integrationenvironment.jupiter.IntegrationEnvironment;
+import org.terasology.engine.rendering.world.viewDistance.ViewDistance;
+import org.terasology.engine.testUtil.WithUnittestModule;
+import org.terasology.gestalt.module.Module;
+import org.terasology.gestalt.module.ModuleMetadataJsonAdapter;
+import org.terasology.gestalt.module.ModuleRegistry;
+
+import java.io.IOException;
+import java.nio.file.Path;
+
+final class IntegrationEnvironmentSubsystem implements EngineSubsystem {
+    private static final Logger logger = LoggerFactory.getLogger(IntegrationEnvironmentSubsystem.class);
+
+    @Override
+    public String getName() {
+        return this.getClass().getSimpleName();
+    }
+
+    @Override
+    public void initialise(GameEngine engine, Context rootContext) {
+        ModuleManager moduleManager = rootContext.getValue(ModuleManager.class);
+        WithUnittestModule.registerUnittestModule(moduleManager);
+        registerCurrentDirectoryIfModule(moduleManager);
+        configure(rootContext);
+    }
+
+    /**
+     * Apply test environment default configuration.
+     * <p>
+     * You can override this by defining your own EngineSubsystem and passing it to
+     * {@link IntegrationEnvironment#subsystem()}; it will run after this does.
+     */
+    static void configure(Context context) {
+        Config config = context.getValue(Config.class);
+        config.getRendering().setViewDistance(ViewDistance.LEGALLY_BLIND);
+
+        SystemConfig sys = context.getValue(SystemConfig.class);
+        sys.writeSaveGamesEnabled.set(false);
+    }
+
+    /**
+     * In standalone module environments (i.e. Jenkins CI builds) the CWD is the module under test. When it uses MTE it very likely needs to
+     * load itself as a module, but it won't be loadable from the typical path such as ./modules. This means that modules using MTE would
+     * always fail CI tests due to failing to load themselves.
+     * <p>
+     * For these cases we try to load the CWD (via the installPath) as a module and put it in the global module registry.
+     * <p>
+     * This process is based on how ModuleManagerImpl uses ModulePathScanner to scan for available modules.
+     */
+    static void registerCurrentDirectoryIfModule(ModuleManager moduleManager) {
+        Path installPath = PathManager.getInstance().getInstallPath();
+        ModuleRegistry registry = moduleManager.getRegistry();
+        ModuleMetadataJsonAdapter metadataReader = moduleManager.getModuleMetadataReader();
+        moduleManager.getModuleFactory().getModuleMetadataLoaderMap()
+                .put(TerasologyConstants.MODULE_INFO_FILENAME.toString(), metadataReader);
+
+        try {
+            Module module = moduleManager.getModuleFactory().createModule(installPath.toFile());
+            if (module != null) {
+                registry.add(module);
+                logger.info("Added install path as module: {}", installPath);
+            } else {
+                logger.info("Install path does not appear to be a module: {}", installPath);
+            }
+        } catch (IOException e) {
+            logger.warn("Could not read install path as module at " + installPath);
+        }
+    }
+}

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

@@ -3,6 +3,8 @@
 
 package org.terasology.engine.integrationenvironment.jupiter;
 
+import org.terasology.engine.core.subsystem.EngineSubsystem;
+import org.terasology.engine.logic.players.LocalPlayer;
 import org.terasology.engine.network.NetworkMode;
 
 import java.lang.annotation.ElementType;
@@ -13,5 +15,34 @@
 @Target(ElementType.TYPE)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface IntegrationEnvironment {
+    /**
+     * The network mode the host engine starts with.
+     * <p>
+     * See {@link NetworkMode} for details on the options.
+     * <p>
+     * Some modes automatically include a {@link LocalPlayer}.
+     * <p>
+     * If you want to simulate multiple players with
+     * {@link org.terasology.engine.integrationenvironment.Engines#createClient Engines.createClient},
+     * you will need a mode with a {@linkplain NetworkMode#isServer() server}.
+     */
     NetworkMode networkMode() default NetworkMode.NONE;
+
+    /**
+     * Add an additional subsystem to the engine.
+     * <p>
+     * A new instance will be included in the engine's subsystems when it is created.
+     * <p>
+     * Implementing {@link EngineSubsystem#initialise} gives you the opportunity to
+     * make changes to the configuration <em>before</em> it would otherwise be available.
+     */
+    Class<? extends EngineSubsystem> subsystem() default NO_SUBSYSTEM.class;
+
+    /**
+     * Do not add an extra subsystem to the integration environment.
+     * <p>
+     * [Odd marker interface because annotation fields cannot default to null.]
+     */
+    @SuppressWarnings("checkstyle:TypeName")
+    abstract class NO_SUBSYSTEM implements EngineSubsystem { }
 }

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

@@ -13,6 +13,7 @@
 import org.junit.jupiter.api.extension.ParameterResolver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.terasology.engine.core.subsystem.EngineSubsystem;
 import org.terasology.engine.integrationenvironment.Engines;
 import org.terasology.engine.integrationenvironment.MainLoop;
 import org.terasology.engine.integrationenvironment.ModuleTestingHelper;
@@ -66,6 +67,8 @@
  * <p>
  * You can configure the environment with these additional annotations:
  * <dl>
+ *     <dt>{@link IntegrationEnvironment @IntegrationEnvironment}</dt>
+ *     <dd>Configure the network mode and add subsystems.</dd>
  *     <dt>{@link Dependencies @Dependencies}</dt>
  *     <dd>Specify which modules to include in the environment. Put the name of your module under test here.
  *         Any dependencies these modules declare in <code>module.txt</code> will be pulled in as well.</dd>
@@ -174,6 +177,12 @@ public NetworkMode getNetworkMode(ExtensionContext context) {
         return getAnnotationWithDefault(context, IntegrationEnvironment::networkMode);
     }
 
+    public List<Class<? extends EngineSubsystem>> getSubsystems(ExtensionContext context) {
+        var subsystem = getAnnotationWithDefault(context, IntegrationEnvironment::subsystem);
+        return subsystem.equals(IntegrationEnvironment.NO_SUBSYSTEM.class)
+                ? Collections.emptyList() : List.of(subsystem);
+    }
+
     private <T> T getAnnotationWithDefault(ExtensionContext context, Function<IntegrationEnvironment, T> method) {
         var ann =
                 findAnnotation(context.getRequiredTestClass(), IntegrationEnvironment.class)
@@ -200,7 +209,8 @@ protected Engines getEngines(ExtensionContext context) {
                 EnginesCleaner.class, k -> new EnginesCleaner(
                         getDependencyNames(context),
                         getWorldGeneratorUri(context),
-                        getNetworkMode(context)
+                        getNetworkMode(context),
+                        getSubsystems(context)
                 ),
                 EnginesCleaner.class);
         return autoCleaner.engines;
@@ -225,8 +235,9 @@ protected ExtensionContext.Namespace namespaceFor(ExtensionContext context) {
     static class EnginesCleaner implements ExtensionContext.Store.CloseableResource {
         protected Engines engines;
 
-        EnginesCleaner(List<String> dependencyNames, String worldGeneratorUri, NetworkMode networkMode) {
-            engines = new Engines(dependencyNames, worldGeneratorUri, networkMode);
+        EnginesCleaner(List<String> dependencyNames, String worldGeneratorUri, NetworkMode networkMode,
+                       List<Class<? extends EngineSubsystem>> subsystems) {
+            engines = new Engines(dependencyNames, worldGeneratorUri, networkMode, subsystems);
             engines.setup();
         }
 

engine-tests/src/main/java/org/terasology/engine/testUtil/Correspondences.java

@@ -0,0 +1,17 @@
+// Copyright 2022 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.engine.testUtil;
+
+import com.google.common.truth.Correspondence;
+
+public final class Correspondences {
+    private Correspondences() {
+    }
+
+    public static <A, E extends Class<?>> Correspondence<A, E> instanceOfExpected() {
+        // This is literally the example in the documentation of Correspondence.from.
+        // They could have included the implementation in the library for us to use!
+        return Correspondence.from((A a, E e) -> e.isInstance(a), "is an instance of");
+    }
+}

engine-tests/src/main/java/org/terasology/engine/testUtil/WithUnittestModule.java

@@ -1,4 +1,4 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 
 package org.terasology.engine.testUtil;
@@ -30,6 +30,10 @@ public String getName() {
     public void initialise(GameEngine engine, Context rootContext) {
         EngineSubsystem.super.initialise(engine, rootContext);
         ModuleManager manager = rootContext.get(ModuleManager.class);
+        registerUnittestModule(manager);
+    }
+
+    public static void registerUnittestModule(ModuleManager manager) {
         Module unittestModule = manager.registerPackageModule("org.terasology.unittest");
         manager.resolveAndLoadEnvironment(unittestModule.getId());
     }