Several fixes and improvements for client tests

- Fix broken /cctest marker
 - Correctly wait for the screenshot to be taken before continuing.
 - Filter out client tests in a different place, meaning we can remove
   the /cctest runall command
 - Bump kotlin version

build.gradle

@@ -17,7 +17,7 @@ plugins {
     id "com.github.hierynomus.license" version "0.16.1"
     id "com.matthewprenger.cursegradle" version "1.4.0"
     id "com.github.breadmoirai.github-release" version "2.2.12"
-    id "org.jetbrains.kotlin.jvm" version "1.3.72"
+    id "org.jetbrains.kotlin.jvm" version "1.5.21"
     id "com.modrinth.minotaur" version "1.2.1"
 }
 
@@ -143,9 +143,9 @@ dependencies {
     testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.0'
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0'
     testImplementation 'org.hamcrest:hamcrest:2.2'
-    testImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.3.72'
+    testImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.5.21'
-    testImplementation 'org.jetbrains.kotlin:kotlin-reflect:1.3.72'
+    testImplementation 'org.jetbrains.kotlin:kotlin-reflect:1.5.21'
-    testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.8'
+    testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.5.1'
 
     testModImplementation sourceSets.main.output
 

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

@@ -2,11 +2,12 @@ package dan200.computercraft.ingame
 
 import dan200.computercraft.ingame.api.GameTest
 import dan200.computercraft.ingame.api.GameTestHelper
+import dan200.computercraft.ingame.api.Timeouts.COMPUTER_TIMEOUT
 import dan200.computercraft.ingame.api.sequence
 import dan200.computercraft.ingame.api.thenComputerOk
 
 class Turtle_Test {
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Unequip_refreshes_peripheral(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -14,7 +15,7 @@ class Turtle_Test {
      *
      * @see [#537](https://github.com/SquidDev-CC/CC-Tweaked/issues/537)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Shears_sheep(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -22,7 +23,7 @@ class Turtle_Test {
      *
      * @see [#518](https://github.com/SquidDev-CC/CC-Tweaked/issues/518)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Place_lava(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -30,7 +31,7 @@ class Turtle_Test {
      *
      * @see [#385](https://github.com/SquidDev-CC/CC-Tweaked/issues/385)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Place_waterlogged(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -38,7 +39,7 @@ class Turtle_Test {
      *
      * @see [#297](https://github.com/SquidDev-CC/CC-Tweaked/issues/297)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Gather_lava(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -46,7 +47,7 @@ class Turtle_Test {
      *
      * @see [#258](https://github.com/SquidDev-CC/CC-Tweaked/issues/258)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Hoe_dirt(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -54,14 +55,14 @@ class Turtle_Test {
      *
      * @see [#691](https://github.com/SquidDev-CC/CC-Tweaked/issues/691)
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Place_monitor(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
      * Checks turtles can place into compostors. These are non-typical inventories, so
      * worth testing.
      */
-    @GameTest(timeoutTicks = TIMEOUT)
+    @GameTest(timeoutTicks = COMPUTER_TIMEOUT)
     fun Use_compostors(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
 
     /**
@@ -71,8 +72,4 @@ class Turtle_Test {
      */
     @GameTest
     fun Cleaned_with_cauldrons(helper: GameTestHelper) = helper.sequence { thenComputerOk() }
-
-    companion object {
-        const val TIMEOUT = 200
-    }
 }

src/testMod/java/dan200/computercraft/ingame/api/GameTest.java

@@ -39,6 +39,13 @@ public @interface GameTest
      */
     String batch() default "default";
 
+    /**
+     * The template to use for this test. Otherwise defaults to the test's name.
+     *
+     * @return This tests' template.
+     */
+    String template() default "";
+
     /**
      * If this test must pass. When false, test failures do not cause a build failure.
      *

src/testMod/java/dan200/computercraft/ingame/api/TestExtensions.kt

@@ -13,10 +13,22 @@ import net.minecraft.world.gen.Heightmap
 import java.nio.file.Files
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.ExecutionException
-import java.util.concurrent.atomic.AtomicInteger
+import java.util.concurrent.atomic.AtomicBoolean
 import java.util.function.Supplier
 import javax.imageio.ImageIO
 
+object Times {
+    const val NOON: Long = 6000
+}
+
+/**
+ * Custom timeouts for various test types.
+ */
+object Timeouts {
+    const val COMPUTER_TIMEOUT: Int = 200
+
+    const val CLIENT_TIMEOUT: Int = 400
+}
 
 /**
  * Wait until a computer has finished running and check it is OK.
@@ -55,25 +67,31 @@ fun GameTestSequence.thenScreenshot(name: String? = null): GameTestSequence {
     val suffix = if (name == null) "" else "-$name"
     val fullName = "${parent.testName}$suffix"
 
-    val counter = AtomicInteger()
+    var counter = 0
+    val hasScreenshot = AtomicBoolean()
+
     return this
         // Wait until all chunks have been rendered and we're idle for an extended period.
-        .thenExecute { counter.set(0) }
+        .thenExecute { counter = 0 }
         .thenWaitUntil {
-            if (Minecraft.getInstance().levelRenderer.hasRenderedAllChunks()) {
+            val renderer = Minecraft.getInstance().levelRenderer
-                val idleFor = counter.getAndIncrement()
+            if (renderer.chunkRenderDispatcher != null && renderer.hasRenderedAllChunks()) {
+                val idleFor = ++counter
                 if (idleFor <= 20) throw GameTestAssertException("Only idle for $idleFor ticks")
             } else {
-                counter.set(0)
+                counter = 0
                 throw GameTestAssertException("Waiting for client to finish rendering")
             }
         }
         // Now disable the GUI, take a screenshot and reenable it. We sleep either side to give the client time to do
         // its thing.
-        .thenExecute { Minecraft.getInstance().options.hideGui = true }
+        .thenExecute {
+            Minecraft.getInstance().options.hideGui = true
+            hasScreenshot.set(false)
+        }
         .thenIdle(5) // Some delay before/after to ensure the render thread has caught up.
-        .thenOnClient { screenshot("$fullName.png") }
+        .thenOnClient { screenshot("$fullName.png") { hasScreenshot.set(true) } }
-        .thenIdle(2)
+        .thenWaitUntil { if (!hasScreenshot.get()) throw GameTestAssertException("Screenshot does not exist") }
         .thenExecute {
             Minecraft.getInstance().options.hideGui = false
 
@@ -84,21 +102,22 @@ fun GameTestSequence.thenScreenshot(name: String? = null): GameTestSequence {
             if (!Files.exists(originalPath)) throw GameTestAssertException("$fullName does not exist. Use `/cctest promote' to create it.");
 
             val screenshot = ImageIO.read(screenshotPath.toFile())
+                ?: throw GameTestAssertException("Error reading screenshot from $screenshotPath")
             val original = ImageIO.read(originalPath.toFile())
 
             if (screenshot.width != original.width || screenshot.height != original.height) {
                 throw GameTestAssertException("$fullName screenshot is ${screenshot.width}x${screenshot.height} but original is ${original.width}x${original.height}")
             }
 
-            if (ImageUtils.areSame(screenshot, original)) return@thenExecute
-
             ImageUtils.writeDifference(screenshotsPath.resolve("$fullName.diff.png"), screenshot, original)
-            throw GameTestAssertException("Images are different.")
+            if (!ImageUtils.areSame(screenshot, original)) throw GameTestAssertException("Images are different.")
         }
 }
 
 val GameTestHelper.testName: String get() = tracker.testName
 
+val GameTestHelper.structureName: String get() = tracker.structureName
+
 /**
  * Modify a block state within the test.
  */
@@ -137,11 +156,11 @@ fun GameTestHelper.normaliseScene() {
  * Position the player at an armor stand.
  */
 fun GameTestHelper.positionAtArmorStand() {
-    val entities = level.getEntities(null, bounds) { it.name.string == testName }
+    val entities = level.getEntities(null, bounds) { it.name.string == structureName }
-    if (entities.size <= 0 || entities[0] !is ArmorStandEntity) throw IllegalStateException("Cannot find armor stand")
+    if (entities.size <= 0 || entities[0] !is ArmorStandEntity) throw GameTestAssertException("Cannot find armor stand")
 
     val stand = entities[0] as ArmorStandEntity
-    val player = level.randomPlayer ?: throw NullPointerException("Player does not exist")
+    val player = level.randomPlayer ?: throw GameTestAssertException("Player does not exist")
 
     player.connection.teleport(stand.x, stand.y, stand.z, stand.yRot, stand.xRot)
 }
@@ -150,10 +169,13 @@ fun GameTestHelper.positionAtArmorStand() {
 class ClientTestHelper {
     val minecraft: Minecraft = Minecraft.getInstance()
 
-    fun screenshot(name: String) {
+    fun screenshot(name: String, callback: () -> Unit = {}) {
         ScreenShotHelper.grab(
             minecraft.gameDirectory, name,
             minecraft.window.width, minecraft.window.height, minecraft.mainRenderTarget
-        ) { TestMod.log.info(it.string) }
+        ) {
+            TestMod.log.info(it.string)
+            callback()
+        }
     }
 }

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

@@ -60,13 +60,6 @@ class CCTestCommand
                 }
                 return total;
             } ) )
-            .then( literal( "runall" ).executes( context -> {
-                TestRegistry.forgetFailedTests();
-                TestResultList result = TestHooks.runTests();
-                result.addListener( new Callback( context.getSource(), result ) );
-                result.addFailureListener( x -> TestRegistry.rememberFailedTest( x.getTestFunction() ) );
-                return 0;
-            } ) )
 
             .then( literal( "promote" ).executes( context -> {
                 if( !FMLLoader.getDist().isClient() ) return error( context.getSource(), "Cannot run on server" );
@@ -96,6 +89,7 @@ class CCTestCommand
                 armorStand.readAdditionalSaveData( nbt );
                 armorStand.copyPosition( player );
                 armorStand.setCustomName( new StringTextComponent( info.getTestName() ) );
+                player.getLevel().addFreshEntity( armorStand );
                 return 0;
             } ) )
         );

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

@@ -5,6 +5,7 @@
  */
 package dan200.computercraft.ingame.mod;
 
+import dan200.computercraft.ingame.api.Times;
 import net.minecraft.client.Minecraft;
 import net.minecraft.command.CommandSource;
 import net.minecraft.server.MinecraftServer;
@@ -27,7 +28,6 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
 import java.util.Collection;
-import java.util.stream.Collectors;
 
 @Mod.EventBusSubscriber( modid = TestMod.MOD_ID )
 public class TestHooks
@@ -56,7 +56,7 @@ public class TestHooks
         rules.getRule( GameRules.RULE_DOMOBSPAWNING ).set( false, server );
 
         ServerWorld world = event.getServer().getLevel( World.OVERWORLD );
-        if( world != null ) world.setDayTime( 6000 );
+        if( world != null ) world.setDayTime( Times.NOON );
 
         LOG.info( "Cleaning up after last run" );
         CommandSource source = server.createCommandSourceStack();
@@ -84,16 +84,12 @@ public class TestHooks
     {
         MinecraftServer server = ServerLifecycleHooks.getCurrentServer();
         CommandSource source = server.createCommandSourceStack();
-        Collection<TestFunctionInfo> tests = TestRegistry.getAllTestFunctions()
+        Collection<TestFunctionInfo> tests = TestRegistry.getAllTestFunctions();
-            .stream()
-            .filter( x -> FMLLoader.getDist().isClient() | !x.batchName.startsWith( "client" ) )
-            .collect( Collectors.toList() );
 
         LOG.info( "Running {} tests...", tests.size() );
 
-        Collection<TestBatch> batches = TestUtils.groupTestsIntoBatches( tests );
+        return new TestResultList( TestUtils.runTests(
-        return new TestResultList( TestUtils.runTestBatches(
+            tests, getStart( source ), Rotation.NONE, source.getLevel(), TestCollection.singleton, 8
-            batches, getStart( source ), Rotation.NONE, source.getLevel(), TestCollection.singleton, 8
         ) );
     }
 

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

@@ -13,6 +13,7 @@ import net.minecraft.test.TestTrackerHolder;
 import net.minecraft.util.Rotation;
 import net.minecraftforge.fml.ModList;
 import net.minecraftforge.fml.common.ObfuscationReflectionHelper;
+import net.minecraftforge.fml.loading.FMLLoader;
 import net.minecraftforge.fml.unsafe.UnsafeHacks;
 import net.minecraftforge.forgespi.language.ModFileScanData;
 import org.objectweb.asm.Type;
@@ -65,11 +66,12 @@ class TestLoader
         String name = className + "." + method.getName().toLowerCase( Locale.ROOT );
 
         GameTest test = method.getAnnotation( GameTest.class );
+        if( test.batch().startsWith( "client" ) && !FMLLoader.getDist().isClient() ) return;
 
         TestMod.log.info( "Adding test " + name );
         testClassNames.add( className );
         testFunctions.add( createTestFunction(
-            test.batch(), name, name,
+            test.batch(), name, test.template().isEmpty() ? name : className + "." + test.template(),
             test.required(),
             holder -> runTest( holder, method ),
             test.timeoutTicks(),

src/testMod/resources/META-INF/accesstransformer.cfg

@@ -24,3 +24,5 @@ public net.minecraft.test.TestTrackerHolder field_229487_a_ # testInfo
 public net.minecraft.test.TestUtils func_229559_b_(Lnet/minecraft/test/TestTracker;Lnet/minecraft/block/Block;)V # spawnBeacon
 
 public net.minecraft.test.TestExecutor func_229479_a_(Lnet/minecraft/test/TestTracker;)V # testCompleted
+
+public net.minecraft.client.renderer.WorldRenderer field_174995_M # chunkRenderDispatcher