Switch to Forge's game test system

It's now impossible to run the client tests (tests are still there, but
none of the other infrastructure is). We've not run these for months now
due to their severe flakiness :(.

build.gradle

@@ -88,26 +88,8 @@ minecraft {
             args '--mod', 'computercraft', '--all', '--output', file('src/generated/resources/'), '--existing', file('src/main/resources/')
         }
 
-        testClient {
+        gameTestServer {
-            workingDirectory project.file('test-files/client')
-            parent runs.client
-
-            mods {
-                cctest {
-                    source sourceSets.testMod
-                }
-            }
-
-            lazyToken('minecraft_classpath') {
-                (configurations.shade.copyRecursive().resolve() + configurations.testModExtra.copyRecursive().resolve())
-                    .collect { it.absolutePath }
-                    .join(File.pathSeparator)
-            }
-        }
-
-        testServer {
             workingDirectory project.file('test-files/server')
-            parent runs.server
 
             mods {
                 cctest {
@@ -374,66 +356,44 @@ task licenseFormatAPI(type: LicenseFormat)
     }
 }
 
-task setupServer(type: Copy) {
+tasks.register("testServer", JavaExec.class).configure {
-    group "test server"
+    it.group('In-game tests')
-    description "Sets up the environment for the test server."
+    it.description("Runs tests on a temporary Minecraft instance.")
+    it.dependsOn("prepareRunGameTestServer", "cleanTestServer", 'compileTestModJava')
 
-    from("src/testMod/server-files") {
+    // Copy from runTestServer. We do it in this slightly odd way as runTestServer
-        include "eula.txt"
+    // isn't created until the task is configured (which is no good for us).
-        include "server.properties"
+    JavaExec exec = tasks.getByName("runGameTestServer")
-    }
+    exec.copyTo(it)
-    into "test-files/server"
+    it.setClasspath(exec.getClasspath())
+    it.mainClass = exec.mainClass
+    it.setArgs(exec.getArgs())
+
+    // Jacoco and modlauncher don't play well together as the classes loaded in-game don't
+    // match up with those written to disk. We get Jacoco to dump all classes to disk, and
+    // use that when generating the report.
+    def coverageOut = new File(buildDir, "jacocoClassDump/testServer")
+    jacoco.applyTo(it)
+    it.jacoco.setIncludes(["dan200.computercraft.*"])
+    it.jacoco.setClassDumpDir(coverageOut)
+    it.outputs.dir(coverageOut)
+    // Older versions of modlauncher don't include a protection domain (and thus no code
+    // source). Jacoco skips such classes by default, so we need to explicitly include them.
+    it.jacoco.setIncludeNoLocationClasses(true)
 }
 
-["Client", "Server"].forEach { name ->
+tasks.register("jacocoTestServerReport", JacocoReport.class).configure {
-    tasks.register("test$name", JavaExec.class).configure {
+    it.group('In-game')
-        it.group('In-game tests')
+    it.description("Generate coverage reports for testServer")
-        it.description("Runs tests on a temporary Minecraft instance.")
+    it.dependsOn("testServer")
-        it.dependsOn(setupServer, "prepareRunTest$name", "cleanTest$name", 'compileTestModJava')
 
-        // Copy from runTestServer. We do it in this slightly odd way as runTestServer
+    it.executionData(new File(buildDir, "jacoco/testServer.exec"))
-        // isn't created until the task is configured (which is no good for us).
+    it.sourceDirectories.from(sourceSets.main.allJava.srcDirs)
-        JavaExec exec = tasks.getByName("runTest$name")
+    it.classDirectories.from(new File(buildDir, "jacocoClassDump/testServer"))
-        exec.copyTo(it)
-        it.setClasspath(exec.getClasspath())
-        it.mainClass = exec.mainClass
-        it.setArgs(exec.getArgs())
 
-        it.systemProperty('forge.logging.console.level', 'info')
+    it.reports {
-        it.systemProperty('cctest.run', 'true')
+        xml.enabled true
-
+        html.enabled true
-        // Jacoco and modlauncher don't play well together as the classes loaded in-game don't
-        // match up with those written to disk. We get Jacoco to dump all classes to disk, and
-        // use that when generating the report.
-        def coverageOut = new File(buildDir, "jacocoClassDump/test$name")
-        jacoco.applyTo(it)
-        it.jacoco.setIncludes(["dan200.computercraft.*"])
-        it.jacoco.setClassDumpDir(coverageOut)
-        it.outputs.dir(coverageOut)
-        // Older versions of modlauncher don't include a protection domain (and thus no code
-        // source). Jacoco skips such classes by default, so we need to explicitly include them.
-        it.jacoco.setIncludeNoLocationClasses(true)
-    }
-
-    tasks.register("jacocoTest${name}Report", JacocoReport.class).configure {
-        it.group('In-game')
-        it.description("Generate coverage reports for test$name")
-        it.dependsOn("test$name")
-
-        it.executionData(new File(buildDir, "jacoco/test${name}.exec"))
-        it.sourceDirectories.from(sourceSets.main.allJava.srcDirs)
-        it.classDirectories.from(new File(buildDir, "jacocoClassDump/test$name"))
-
-        it.reports {
-            xml.enabled true
-            html.enabled true
-        }
-    }
-
-    if (name != "Client" || project.findProperty('cc.tweaked.clientTests') == 'true') {
-        // Don't run client tests unless explicitly opted into them. They're a bit of a faff
-        // to run and pretty flakey.
-        check.dependsOn("jacocoTest${name}Report")
     }
 }
 

src/testMod/java/dan200/computercraft/ingame/ComputerTest.kt

@@ -1,12 +1,16 @@
 package dan200.computercraft.ingame
 
-import dan200.computercraft.ingame.api.*
+import dan200.computercraft.ComputerCraft
+import dan200.computercraft.ingame.api.modifyBlock
+import dan200.computercraft.ingame.api.sequence
 import net.minecraft.core.BlockPos
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 import net.minecraft.world.level.block.LeverBlock
 import net.minecraft.world.level.block.RedstoneLampBlock
+import net.minecraftforge.gametest.GameTestHolder
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class Computer_Test {
     /**
      * Ensures redstone signals do not travel through computers.

src/testMod/java/dan200/computercraft/ingame/CraftOsTest.kt

@@ -1,10 +1,13 @@
 package dan200.computercraft.ingame
 
+import dan200.computercraft.ComputerCraft
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenComputerOk
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
+import net.minecraftforge.gametest.GameTestHolder
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class CraftOs_Test {
     /**
      * Sends a rednet message to another a computer and back again.

src/testMod/java/dan200/computercraft/ingame/DiskDriveTest.kt

@@ -1,12 +1,15 @@
 package dan200.computercraft.ingame
 
+import dan200.computercraft.ComputerCraft
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenComputerOk
 import net.minecraft.core.BlockPos
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 import net.minecraft.world.item.Items
+import net.minecraftforge.gametest.GameTestHolder
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class Disk_Drive_Test {
     /**
      * Ensure audio disks exist and we can play them.

src/testMod/java/dan200/computercraft/ingame/ModemTest.kt

@@ -1,5 +1,6 @@
 package dan200.computercraft.ingame
 
+import dan200.computercraft.ComputerCraft
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenComputerOk
 import dan200.computercraft.shared.Registry
@@ -7,7 +8,9 @@ import dan200.computercraft.shared.peripheral.modem.wired.BlockCable
 import net.minecraft.core.BlockPos
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
+import net.minecraftforge.gametest.GameTestHolder
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class Modem_Test {
     @GameTest(timeoutTicks = TIMEOUT)
     fun Have_peripherals(helper: GameTestHelper) = helper.sequence { thenComputerOk() }

src/testMod/java/dan200/computercraft/ingame/MonitorTest.kt

@@ -2,7 +2,6 @@ package dan200.computercraft.ingame
 
 import dan200.computercraft.ComputerCraft
 import dan200.computercraft.ingame.api.*
-import dan200.computercraft.ingame.api.Timeouts.CLIENT_TIMEOUT
 import dan200.computercraft.shared.Capabilities
 import dan200.computercraft.shared.Registry
 import dan200.computercraft.shared.peripheral.monitor.MonitorRenderer
@@ -13,8 +12,10 @@ import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 import net.minecraft.nbt.CompoundTag
 import net.minecraft.world.level.block.Blocks
+import net.minecraftforge.gametest.GameTestHolder
 import java.util.*
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class Monitor_Test {
     @GameTest
     fun Ensures_valid_on_place(context: GameTestHelper) = context.sequence {
@@ -67,16 +68,16 @@ class Monitor_Test {
             .thenScreenshot()
     }
 
-    @GameTest(batch = "client:Monitor_Test.Looks_acceptable", timeoutTicks = CLIENT_TIMEOUT, template = LOOKS_ACCEPTABLE)
+    // @GameTest(batch = "Monitor_Test.Looks_acceptable", template = LOOKS_ACCEPTABLE)
     fun Looks_acceptable(helper: GameTestHelper) = looksAcceptable(helper, renderer = MonitorRenderer.TBO)
 
-    @GameTest(batch = "client:Monitor_Test.Looks_acceptable_dark", timeoutTicks = CLIENT_TIMEOUT, template = LOOKS_ACCEPTABLE_DARK)
+    // @GameTest(batch = "Monitor_Test.Looks_acceptable_dark", template = LOOKS_ACCEPTABLE_DARK)
     fun Looks_acceptable_dark(helper: GameTestHelper) = looksAcceptable(helper, renderer = MonitorRenderer.TBO)
 
-    @GameTest(batch = "client:Monitor_Test.Looks_acceptable_vbo", timeoutTicks = CLIENT_TIMEOUT, template = LOOKS_ACCEPTABLE)
+    // @GameTest(batch = "Monitor_Test.Looks_acceptable_vbo", template = LOOKS_ACCEPTABLE)
     fun Looks_acceptable_vbo(helper: GameTestHelper) = looksAcceptable(helper, renderer = MonitorRenderer.VBO)
 
-    @GameTest(batch = "client:Monitor_Test.Looks_acceptable_dark_vbo", timeoutTicks = CLIENT_TIMEOUT, template = LOOKS_ACCEPTABLE_DARK)
+    // @GameTest(batch = "Monitor_Test.Looks_acceptable_dark_vbo", template = LOOKS_ACCEPTABLE_DARK)
     fun Looks_acceptable_dark_vbo(helper: GameTestHelper) = looksAcceptable(helper, renderer = MonitorRenderer.VBO)
 
     private companion object {

src/testMod/java/dan200/computercraft/ingame/PrintoutTest.kt

@@ -1,14 +1,12 @@
 package dan200.computercraft.ingame
 
-import dan200.computercraft.ingame.api.Timeouts
 import dan200.computercraft.ingame.api.positionAtArmorStand
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenScreenshot
-import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 
 class PrintoutTest {
-    @GameTest(batch = "client:Printout_Test.In_frame_at_night", timeoutTicks = Timeouts.CLIENT_TIMEOUT)
+    // @GameTest(batch = "Printout_Test.In_frame_at_night", timeoutTicks = Timeouts.CLIENT_TIMEOUT)
     fun In_frame_at_night(helper: GameTestHelper) = helper.sequence {
         this
             .thenExecute { helper.positionAtArmorStand() }

src/testMod/java/dan200/computercraft/ingame/TurtleTest.kt

@@ -1,11 +1,14 @@
 package dan200.computercraft.ingame
 
+import dan200.computercraft.ComputerCraft
 import dan200.computercraft.ingame.api.Timeouts.COMPUTER_TIMEOUT
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenComputerOk
 import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
+import net.minecraftforge.gametest.GameTestHolder
 
+@GameTestHolder(ComputerCraft.MOD_ID)
 class Turtle_Test {
     @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Unequip_refreshes_peripheral(helper: GameTestHelper) = helper.sequence { thenComputerOk() }

src/testMod/java/dan200/computercraft/ingame/mod/CCTestCommand.java

@@ -11,7 +11,10 @@ import net.minecraft.ChatFormatting;
 import net.minecraft.client.Minecraft;
 import net.minecraft.commands.CommandSourceStack;
 import net.minecraft.core.BlockPos;
-import net.minecraft.gametest.framework.*;
+import net.minecraft.gametest.framework.GameTestRegistry;
+import net.minecraft.gametest.framework.StructureUtils;
+import net.minecraft.gametest.framework.TestCommand;
+import net.minecraft.gametest.framework.TestFunction;
 import net.minecraft.nbt.CompoundTag;
 import net.minecraft.network.chat.TextComponent;
 import net.minecraft.server.MinecraftServer;
@@ -23,7 +26,6 @@ import net.minecraft.world.level.block.entity.StructureBlockEntity;
 import net.minecraft.world.level.storage.LevelResource;
 import net.minecraftforge.fml.loading.FMLLoader;
 
-import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 
@@ -133,35 +135,6 @@ class CCTestCommand
         }
     }
 
-    private static class Callback implements GameTestListener
-    {
-        private final CommandSourceStack source;
-        private final MultipleTestTracker result;
-
-        Callback( CommandSourceStack source, MultipleTestTracker result )
-        {
-            this.source = source;
-            this.result = result;
-        }
-
-        @Override
-        public void testStructureLoaded( @Nonnull GameTestInfo tracker )
-        {
-        }
-
-        @Override
-        public void testFailed( @Nonnull GameTestInfo tracker )
-        {
-            TestHooks.writeResults( source, result );
-        }
-
-        @Override
-        public void testPassed( @Nonnull GameTestInfo tracker )
-        {
-            TestHooks.writeResults( source, result );
-        }
-    }
-
     private static int error( CommandSourceStack source, String message )
     {
         source.sendFailure( new TextComponent( message ).withStyle( ChatFormatting.RED ) );

src/testMod/java/dan200/computercraft/ingame/mod/TestHooks.java

@@ -6,45 +6,26 @@
 package dan200.computercraft.ingame.mod;
 
 import dan200.computercraft.ingame.api.Times;
-import net.minecraft.ChatFormatting;
-import net.minecraft.SharedConstants;
-import net.minecraft.client.Minecraft;
-import net.minecraft.commands.CommandSourceStack;
-import net.minecraft.core.BlockPos;
-import net.minecraft.gametest.framework.*;
-import net.minecraft.network.chat.TextComponent;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerLevel;
 import net.minecraft.world.level.GameRules;
 import net.minecraft.world.level.Level;
-import net.minecraft.world.level.block.Rotation;
-import net.minecraft.world.level.levelgen.Heightmap;
 import net.minecraftforge.event.RegisterCommandsEvent;
-import net.minecraftforge.event.TickEvent;
 import net.minecraftforge.event.server.ServerStartedEvent;
 import net.minecraftforge.eventbus.api.SubscribeEvent;
 import net.minecraftforge.fml.common.Mod;
-import net.minecraftforge.fml.loading.FMLLoader;
-import net.minecraftforge.server.ServerLifecycleHooks;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.util.Collection;
-
 @Mod.EventBusSubscriber( modid = TestMod.MOD_ID )
 public class TestHooks
 {
     private static final Logger LOG = LogManager.getLogger( TestHooks.class );
 
-    private static MultipleTestTracker runningTests = null;
-    private static boolean shutdown = false;
-    private static int countdown = 20;
-
     @SubscribeEvent
     public static void onRegisterCommands( RegisterCommandsEvent event )
     {
         LOG.info( "Starting server, registering command helpers." );
-        TestCommand.register( event.getDispatcher() );
         CCTestCommand.register( event.getDispatcher() );
     }
 
@@ -54,124 +35,15 @@ public class TestHooks
         MinecraftServer server = event.getServer();
         GameRules rules = server.getGameRules();
         rules.getRule( GameRules.RULE_DAYLIGHT ).set( false, server );
-        rules.getRule( GameRules.RULE_WEATHER_CYCLE ).set( false, server );
-        rules.getRule( GameRules.RULE_DOMOBSPAWNING ).set( false, server );
 
         ServerLevel world = event.getServer().getLevel( Level.OVERWORLD );
         if( world != null ) world.setDayTime( Times.NOON );
 
-        LOG.info( "Cleaning up after last run" );
+        // LOG.info( "Cleaning up after last run" );
-        CommandSourceStack source = server.createCommandSourceStack();
+        // CommandSourceStack source = server.createCommandSourceStack();
-        GameTestRunner.clearAllTests( source.getLevel(), getStart( source ), GameTestTicker.SINGLETON, 200 );
+        // GameTestRunner.clearAllTests( source.getLevel(), getStart( source ), GameTestTicker.SINGLETON, 200 );
 
         LOG.info( "Importing files" );
         CCTestCommand.importFiles( server );
     }
-
-    @SubscribeEvent
-    public static void onServerTick( TickEvent.ServerTickEvent event )
-    {
-        if( event.phase != TickEvent.Phase.START ) return;
-
-        // Let the world settle a bit before starting tests.
-        countdown--;
-        if( countdown == 0 && System.getProperty( "cctest.run", "false" ).equals( "true" ) ) startTests();
-
-        if( !SharedConstants.IS_RUNNING_IN_IDE ) GameTestTicker.SINGLETON.tick();
-
-        if( runningTests != null && runningTests.isDone() ) finishTests();
-    }
-
-    public static MultipleTestTracker runTests()
-    {
-        MinecraftServer server = ServerLifecycleHooks.getCurrentServer();
-        CommandSourceStack source = server.createCommandSourceStack();
-        Collection<TestFunction> tests = GameTestRegistry.getAllTestFunctions();
-
-        LOG.info( "Running {} tests...", tests.size() );
-
-        return new MultipleTestTracker( GameTestRunner.runTests(
-            tests, getStart( source ), Rotation.NONE, source.getLevel(), GameTestTicker.SINGLETON, 8
-        ) );
-    }
-
-    public static void writeResults( CommandSourceStack source, MultipleTestTracker result )
-    {
-        if( !result.isDone() ) return;
-
-        say( source, "Finished tests - " + result.getTotalCount() + " tests were run", ChatFormatting.WHITE );
-        if( result.hasFailedRequired() )
-        {
-            say( source, result.getFailedRequiredCount() + " required tests failed :(", ChatFormatting.RED );
-        }
-        else
-        {
-            say( source, "All required tests passed :)", ChatFormatting.GREEN );
-        }
-
-        if( result.hasFailedOptional() )
-        {
-            say( source, result.getFailedOptionalCount() + " optional tests failed", ChatFormatting.GRAY );
-        }
-    }
-
-    private static void startTests()
-    {
-        runningTests = runTests();
-    }
-
-    private static void finishTests()
-    {
-        if( shutdown ) return;
-        shutdown = true;
-        writeResults( ServerLifecycleHooks.getCurrentServer().createCommandSourceStack(), runningTests );
-
-        if( FMLLoader.getDist().isDedicatedServer() )
-        {
-            shutdownServer();
-        }
-        else
-        {
-            shutdownClient();
-        }
-    }
-
-    private static BlockPos getStart( CommandSourceStack source )
-    {
-        BlockPos pos = new BlockPos( source.getPosition() );
-        return new BlockPos( pos.getX(), source.getLevel().getHeightmapPos( Heightmap.Types.WORLD_SURFACE, pos ).getY(), pos.getZ() + 3 );
-    }
-
-    public static void shutdownCommon()
-    {
-        System.exit( runningTests.hasFailedRequired() ? 1 : 0 );
-    }
-
-    private static void shutdownServer()
-    {
-        // We can't exit normally as Minecraft registers a shutdown hook which results in a deadlock.
-        LOG.info( "Stopping server." );
-        MinecraftServer server = ServerLifecycleHooks.getCurrentServer();
-        new Thread( () -> {
-            server.halt( true );
-            shutdownCommon();
-        }, "Background shutdown" ).start();
-    }
-
-    private static void shutdownClient()
-    {
-        Minecraft minecraft = Minecraft.getInstance();
-        minecraft.execute( () -> {
-            LOG.info( "Stopping client." );
-            minecraft.level.disconnect();
-            minecraft.clearLevel();
-            minecraft.stop();
-            shutdownCommon();
-        } );
-    }
-
-    private static void say( CommandSourceStack source, String message, ChatFormatting colour )
-    {
-        source.sendFailure( new TextComponent( message ).withStyle( colour ) );
-    }
 }

src/testMod/java/dan200/computercraft/ingame/mod/TestLoader.java

@@ -1,56 +0,0 @@
-/*
- * This file is part of ComputerCraft - http://www.computercraft.info
- * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
- * Send enquiries to dratcliffe@gmail.com
- */
-package dan200.computercraft.ingame.mod;
-
-import net.minecraft.gametest.framework.GameTest;
-import net.minecraft.gametest.framework.GameTestRegistry;
-import net.minecraftforge.fml.ModList;
-import net.minecraftforge.fml.loading.FMLLoader;
-import net.minecraftforge.forgespi.language.ModFileScanData;
-import org.objectweb.asm.Type;
-
-import java.lang.reflect.Method;
-import java.util.Arrays;
-
-/**
- * Loads methods annotated with {@link GameTest} and adds them to the {@link GameTestRegistry}.
- */
-class TestLoader
-{
-    private static final Type gameTest = Type.getType( GameTest.class );
-
-    public static void setup()
-    {
-        ModList.get().getAllScanData().stream()
-            .flatMap( x -> x.getAnnotations().stream() )
-            .filter( x -> x.annotationType().equals( gameTest ) )
-            .forEach( TestLoader::loadTest );
-    }
-
-    private static void loadTest( ModFileScanData.AnnotationData annotation )
-    {
-        Class<?> klass;
-        Method method;
-        try
-        {
-            klass = TestLoader.class.getClassLoader().loadClass( annotation.clazz().getClassName() );
-
-            // We don't know the exact signature (could suspend or not), so find something with the correct descriptor instead.
-            String methodName = annotation.memberName();
-            method = Arrays.stream( klass.getMethods() ).filter( x -> (x.getName() + Type.getMethodDescriptor( x )).equals( methodName ) ).findFirst()
-                .orElseThrow( () -> new NoSuchMethodException( "No method " + annotation.clazz().getClassName() + "." + annotation.memberName() ) );
-        }
-        catch( ReflectiveOperationException e )
-        {
-            throw new RuntimeException( e );
-        }
-
-        GameTest test = method.getAnnotation( GameTest.class );
-        if( test.batch().startsWith( "client" ) && !FMLLoader.getDist().isClient() ) return;
-
-        GameTestRegistry.register( method );
-    }
-}

src/testMod/java/dan200/computercraft/ingame/mod/TestMod.java

@@ -27,7 +27,6 @@ public class TestMod
     {
         log.info( "CC: Test initialised" );
         ComputerCraftAPI.registerAPIFactory( TestAPI::new );
-        TestLoader.setup();
 
         StructureUtils.testStructuresDir = sourceDir.resolve( "structures" ).toString();
     }