Add a system for client-side tests (#1219)

- Add a new ClientJavaExec Gradle task, which is used for client-side
   tests. This:

   - Copies the exec spec from another JavaExec task.
   - Sets some additional system properties to configure on gametest framework.
   - Runs Java inside an X framebuffer (when available), meaning we
     don't need to spin up a new window.

   We also configure this task so that only one instance can run at
   once, meaning we don't spawn multiple MC windows at once!

 - Port our 1.16 client test framework to 1.19. This is mostly the same
   as before, but screenshots no longer do a golden test: they /just/
   write to a folder. Screenshots are compared manually afterwards.

   This is still pretty brittle, and there's a lot of sleeps scattered
   around in the code. It's not clear how well this will play on CI.

 - Roll our own game test loader, rather than relying on the mod loader
   to do it for us. This ensures that loading is consistent between
   platforms (we already had to do some hacks for Forge) and makes it
   easier to provide custom logic for loading client-only tests.

 - Run several client tests (namely those involving monitor rendering)
   against Sodium and Iris too. There's some nastiness here to set up
   new Loom run configurations and automatically configure Iris to use
   Complementary Shaders, but it's not too bad. These tests /don't/ run
   on CI, so it doesn't need to be as reliable.

buildSrc/src/main/kotlin/cc/tweaked/gradle/CCTweakedExtension.kt

@@ -4,22 +4,21 @@ import net.ltgt.gradle.errorprone.CheckSeverity
 import net.ltgt.gradle.errorprone.errorprone
 import org.gradle.api.NamedDomainObjectProvider
 import org.gradle.api.Project
+import org.gradle.api.Task
 import org.gradle.api.attributes.TestSuiteType
 import org.gradle.api.file.FileSystemOperations
 import org.gradle.api.plugins.JavaPluginExtension
 import org.gradle.api.provider.Provider
 import org.gradle.api.provider.SetProperty
 import org.gradle.api.reporting.ReportingExtension
-import org.gradle.api.tasks.JavaExec
 import org.gradle.api.tasks.SourceSet
-import org.gradle.api.tasks.SourceSetContainer
 import org.gradle.api.tasks.bundling.Jar
 import org.gradle.api.tasks.compile.JavaCompile
 import org.gradle.api.tasks.javadoc.Javadoc
 import org.gradle.configurationcache.extensions.capitalized
-import org.gradle.kotlin.dsl.get
 import org.gradle.language.base.plugins.LifecycleBasePlugin
 import org.gradle.language.jvm.tasks.ProcessResources
+import org.gradle.process.JavaForkOptions
 import org.gradle.testing.jacoco.plugins.JacocoCoverageReport
 import org.gradle.testing.jacoco.plugins.JacocoPluginExtension
 import org.gradle.testing.jacoco.plugins.JacocoTaskExtension
@@ -27,8 +26,11 @@ import org.gradle.testing.jacoco.tasks.JacocoReport
 import org.jetbrains.kotlin.gradle.dsl.KotlinProjectExtension
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
 import java.io.BufferedWriter
+import java.io.File
 import java.io.IOException
 import java.io.OutputStreamWriter
+import java.net.URI
+import java.net.URL
 import java.util.regex.Pattern
 
 abstract class CCTweakedExtension(
@@ -79,8 +81,7 @@ abstract class CCTweakedExtension(
     /**
      * References to other sources
      */
-    val sourceDirectories: SetProperty<SourceSetReference> =
-        project.objects.setProperty(SourceSetReference::class.java)
+    val sourceDirectories: SetProperty<SourceSetReference> = project.objects.setProperty(SourceSetReference::class.java)
 
     /** All source sets referenced by this project. */
     val sourceSets = sourceDirectories.map { x -> x.map { it.sourceSet } }
@@ -181,7 +182,7 @@ abstract class CCTweakedExtension(
         }
     }
 
-    fun jacoco(task: NamedDomainObjectProvider<JavaExec>) {
+    fun <T> jacoco(task: NamedDomainObjectProvider<T>) where T : Task, T : JavaForkOptions {
         val classDump = project.buildDir.resolve("jacocoClassDump/${task.name}")
         val reportTaskName = "jacoco${task.name.capitalized()}Report"
 
@@ -210,8 +211,7 @@ abstract class CCTweakedExtension(
             classDirectories.from(classDump)
 
             // Don't want to use sourceSets(...) here as we have a custom class directory.
-            val sourceSets = project.extensions.getByType(SourceSetContainer::class.java)
-            sourceDirectories.from(sourceSets["main"].allSource.sourceDirectories)
+            for (ref in sourceSets.get()) sourceDirectories.from(ref.allSource.sourceDirectories)
         }
 
         project.extensions.configure(ReportingExtension::class.java) {
@@ -221,6 +221,41 @@ abstract class CCTweakedExtension(
         }
     }
 
+    /**
+     * Download a file by creating a dummy Ivy repository.
+     *
+     * This should only be used for one-off downloads. Using a more conventional Ivy or Maven repository is preferred
+     * where possible.
+     */
+    fun downloadFile(label: String, url: String): File {
+        val url = URL(url)
+        val path = File(url.path)
+
+        project.repositories.ivy {
+            name = label
+            setUrl(URI(url.protocol, url.userInfo, url.host, url.port, path.parent, null, null))
+            patternLayout {
+                artifact("[artifact].[ext]")
+            }
+            metadataSources {
+                artifact()
+            }
+            content {
+                includeModule("cc.tweaked.internal", path.nameWithoutExtension)
+            }
+        }
+
+        return project.configurations.detachedConfiguration(
+            project.dependencies.create(
+                mapOf(
+                    "group" to "cc.tweaked.internal",
+                    "name" to path.nameWithoutExtension,
+                    "ext" to path.extension,
+                ),
+            ),
+        ).resolve().single()
+    }
+
     companion object {
         private val EMAIL = Pattern.compile("^([^<]+) <.+>$")
         private val IGNORED_USERS = setOf(
@@ -238,7 +273,7 @@ abstract class CCTweakedExtension(
             }
         }
 
-        internal val isIdeSync: Boolean
+        private val isIdeSync: Boolean
             get() = java.lang.Boolean.parseBoolean(System.getProperty("idea.sync.active", "false"))
     }
 }

buildSrc/src/main/kotlin/cc/tweaked/gradle/CheckChangelog.kt

@@ -6,7 +6,6 @@ import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.tasks.*
 import org.gradle.language.base.plugins.LifecycleBasePlugin
-import java.nio.charset.StandardCharsets
 
 /**
  * Checks the `changelog.md` and `whatsnew.md` files are well-formed.

buildSrc/src/main/kotlin/cc/tweaked/gradle/CheckLicense.kt

@@ -5,7 +5,6 @@ import com.diffplug.spotless.FormatterStep
 import com.diffplug.spotless.generic.LicenseHeaderStep
 import java.io.File
 import java.io.Serializable
-import java.nio.charset.StandardCharsets
 
 /**
  * Similar to [LicenseHeaderStep], but supports multiple licenses.

buildSrc/src/main/kotlin/cc/tweaked/gradle/Extensions.kt

@@ -1,11 +1,10 @@
 package cc.tweaked.gradle
 
-import org.gradle.api.artifacts.ResolvedDependency
 import org.gradle.api.artifacts.dsl.DependencyHandler
-import org.gradle.api.publish.maven.MavenPublication
-import org.gradle.api.specs.Spec
 import org.gradle.api.tasks.JavaExec
+import org.gradle.process.BaseExecSpec
 import org.gradle.process.JavaExecSpec
+import org.gradle.process.ProcessForkOptions
 
 /**
  * Add an annotation processor to all source sets.
@@ -26,10 +25,28 @@ fun DependencyHandler.annotationProcessorEverywhere(dep: Any) {
  */
 fun JavaExec.copyToFull(spec: JavaExec) {
     copyTo(spec)
-    spec.classpath = classpath
-    spec.mainClass.set(mainClass)
-    spec.javaLauncher.set(javaLauncher)
+
+    // Additional Java options
+    spec.jvmArgs = jvmArgs // Fabric overrides getJvmArgs so copyTo doesn't do the right thing.
     spec.args = args
+    spec.argumentProviders.addAll(argumentProviders)
+    spec.mainClass.set(mainClass)
+    spec.classpath = classpath
+    spec.javaLauncher.set(javaLauncher)
+    if (executable != null) spec.setExecutable(executable!!)
+
+    // Additional ExecSpec options
+    copyToExec(spec)
+}
+
+/**
+ * Copy additional [BaseExecSpec] options which aren't handled by [ProcessForkOptions.copyTo].
+ */
+fun BaseExecSpec.copyToExec(spec: BaseExecSpec) {
+    spec.isIgnoreExitValue = isIgnoreExitValue
+    if (standardInput != null) spec.standardInput = standardInput
+    if (standardOutput != null) spec.standardOutput = standardOutput
+    if (errorOutput != null) spec.errorOutput = errorOutput
 }
 
 /**
@@ -42,3 +59,59 @@ fun JavaExec.copyToFull(spec: JavaExec) {
  * ```
  */
 class UseNamedArgs private constructor()
+
+/**
+ * An [AutoCloseable] implementation which can be used to combine other [AutoCloseable] instances.
+ *
+ * Values which implement [AutoCloseable] can be dynamically registered with [CloseScope.add]. When the scope is closed,
+ * each value is closed in the opposite order.
+ *
+ * This is largely intended for cases where it's not appropriate to nest [AutoCloseable.use], for instance when nested
+ * would be too deep.
+ */
+class CloseScope : AutoCloseable {
+    private val toClose = ArrayDeque<AutoCloseable>()
+
+    /**
+     * Add a value to be closed when this scope is closed.
+     */
+    public fun add(value: AutoCloseable) {
+        toClose.addLast(value)
+    }
+
+    override fun close() {
+        close(null)
+    }
+
+    @PublishedApi
+    internal fun close(baseException: Throwable?) {
+        var exception = baseException
+
+        while (true) {
+            var toClose = toClose.removeLastOrNull() ?: break
+            try {
+                toClose.close()
+            } catch (e: Throwable) {
+                if (exception == null) {
+                    exception = e
+                } else {
+                    exception.addSuppressed(e)
+                }
+            }
+        }
+
+        if (exception != null) throw exception
+    }
+
+    inline fun <R> use(block: (CloseScope) -> R): R {
+        var exception: Throwable? = null
+        try {
+            return block(this)
+        } catch (e: Throwable) {
+            exception = e
+            throw e
+        } finally {
+            close(exception)
+        }
+    }
+}

buildSrc/src/main/kotlin/cc/tweaked/gradle/IdeaRunConfigurations.kt

@@ -6,7 +6,6 @@ import org.w3c.dom.Attr
 import org.w3c.dom.Document
 import org.w3c.dom.Node
 import org.xml.sax.InputSource
-import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.nio.file.Path
 import javax.xml.parsers.DocumentBuilderFactory
@@ -36,7 +35,7 @@ internal class IdeaRunConfigurations(project: Project) {
         val ideaMisc = ideaDir.resolve("misc.xml")
 
         try {
-            val doc = Files.newBufferedReader(ideaMisc.toPath(), StandardCharsets.UTF_8).use {
+            val doc = Files.newBufferedReader(ideaMisc.toPath()).use {
                 documentBuilder.parse(InputSource(it))
             }
             val node =

buildSrc/src/main/kotlin/cc/tweaked/gradle/MinecraftExec.kt

@@ -0,0 +1,202 @@
+package cc.tweaked.gradle
+
+import org.gradle.api.GradleException
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.invocation.Gradle
+import org.gradle.api.provider.Provider
+import org.gradle.api.services.BuildService
+import org.gradle.api.services.BuildServiceParameters
+import org.gradle.api.tasks.*
+import org.gradle.kotlin.dsl.getByName
+import org.gradle.language.base.plugins.LifecycleBasePlugin
+import java.io.File
+import java.nio.file.Files
+import java.util.concurrent.TimeUnit
+import java.util.function.Supplier
+import javax.inject.Inject
+import kotlin.random.Random
+
+/**
+ * A [JavaExec] task for client-tests. This sets some common setup, and uses [MinecraftRunnerService] to ensure only one
+ * test runs at once.
+ */
+abstract class ClientJavaExec : JavaExec() {
+    private val clientRunner: Provider<MinecraftRunnerService> = MinecraftRunnerService.get(project.gradle)
+
+    init {
+        group = LifecycleBasePlugin.VERIFICATION_GROUP
+        usesService(clientRunner)
+    }
+
+    /**
+     * When [false], tests will not be run automatically, allowing the user to debug rendering.
+     */
+    @get:Input
+    val clientDebug get() = project.hasProperty("clientDebug")
+
+    /**
+     * When [false], tests will not run under a framebuffer.
+     */
+    @get:Input
+    val useFramebuffer get() = !clientDebug && !project.hasProperty("clientNoFramebuffer")
+
+    /**
+     * The folder screenshots are written to.
+     */
+    @get:OutputDirectory
+    val screenshots = project.layout.buildDirectory.dir("testScreenshots")
+
+    /**
+     * The path test results are written to.
+     */
+    @get:OutputFile
+    val testResults = project.layout.buildDirectory.file("test-results/$name.xml")
+
+    /**
+     * Copy configuration from a task with the given name.
+     */
+    fun copyFrom(path: String) = copyFrom(project.tasks.getByName(path, JavaExec::class))
+
+    /**
+     * Copy configuration from an existing [JavaExec] task.
+     */
+    fun copyFrom(task: JavaExec) {
+        for (dep in task.dependsOn) dependsOn(dep)
+        task.copyToFull(this)
+
+        if (!clientDebug) systemProperty("cctest.client", "")
+        systemProperty("cctest.gametest-report", testResults.get().asFile.absoluteFile)
+        workingDir(project.buildDir.resolve("gametest").resolve(name))
+    }
+
+    /**
+     * Only run tests with the given tags.
+     */
+    fun tags(vararg tags: String) {
+        systemProperty("cctest.tags", tags.joinToString(","))
+    }
+
+    /**
+     * Write a file with the given contents before starting Minecraft. This may be useful for writing config files.
+     */
+    fun withFileContents(path: Any, contents: Supplier<String>) {
+        val file = project.file(path).toPath()
+        doFirst {
+            Files.createDirectories(file.parent)
+            Files.writeString(file, contents.get())
+        }
+    }
+
+    /**
+     * Copy a file to the provided path before starting Minecraft. This copy only occurs if the file does not already
+     * exist.
+     */
+    fun withFileFrom(path: Any, source: Supplier<File>) {
+        val file = project.file(path).toPath()
+        doFirst {
+            Files.createDirectories(file.parent)
+            if (!Files.exists(file)) Files.copy(source.get().toPath(), file)
+        }
+    }
+
+    @TaskAction
+    override fun exec() {
+        Files.createDirectories(workingDir.toPath())
+        fsOperations.delete { delete(workingDir.resolve("screenshots")) }
+
+        if (useFramebuffer) {
+            clientRunner.get().wrapClient(this) { super.exec() }
+        } else {
+            super.exec()
+        }
+
+        fsOperations.copy {
+            from(workingDir.resolve("screenshots"))
+            into(screenshots)
+        }
+    }
+
+    @get:Inject
+    protected abstract val fsOperations: FileSystemOperations
+}
+
+/**
+ * A service for [JavaExec] tasks which start Minecraft.
+ *
+ * Tasks may run `usesService(MinecraftRunnerService.get(gradle))` to ensure that only one Minecraft-related task runs
+ * at once.
+ */
+abstract class MinecraftRunnerService : BuildService<BuildServiceParameters.None> {
+    private val hasXvfb = lazy {
+        System.getProperty("os.name", "").equals("linux", ignoreCase = true) && ProcessHelpers.onPath("xvfb-run")
+    }
+
+    internal fun wrapClient(exec: JavaExec, run: () -> Unit) = when {
+        hasXvfb.value -> runXvfb(exec, run)
+        else -> run()
+    }
+
+    /**
+     * Run a program under Xvfb, preventing it spawning a window.
+     */
+    private fun runXvfb(exec: JavaExec, run: () -> Unit) {
+        fun ProcessBuilder.startVerbose(): Process {
+            exec.logger.info("Running ${this.command()}")
+            return start()
+        }
+
+        CloseScope().use { scope ->
+            val dir = Files.createTempDirectory("cctweaked").toAbsolutePath()
+            scope.add { fsOperations.delete { delete(dir) } }
+
+            val authFile = Files.createTempFile(dir, "Xauthority", "").toAbsolutePath()
+
+            val cookie = StringBuilder().also {
+                for (i in 0..31) it.append("0123456789abcdef"[Random.nextInt(16)])
+            }.toString()
+
+            val xvfb =
+                ProcessBuilder("Xvfb", "-displayfd", "1", "-screen", "0", "640x480x24", "-nolisten", "tcp").also {
+                    it.inheritIO()
+                    it.environment()["XAUTHORITY"] = authFile.toString()
+                    it.redirectOutput(ProcessBuilder.Redirect.PIPE)
+                }.startVerbose()
+            scope.add { xvfb.destroyForcibly().waitFor() }
+
+            val server = xvfb.inputReader().use { it.readLine().trim() }
+            exec.logger.info("Running at :$server (XAUTHORITY=$authFile.toA")
+
+            ProcessBuilder("xauth", "add", ":$server", ".", cookie).also {
+                it.inheritIO()
+                it.environment()["XAUTHORITY"] = authFile.toString()
+            }.startVerbose().waitForOrThrow("Failed to setup XAuthority file")
+
+            scope.add {
+                ProcessBuilder("xauth", "remove", ":$server").also {
+                    it.inheritIO()
+                    it.environment()["XAUTHORITY"] = authFile.toString()
+                }.startVerbose().waitFor()
+            }
+
+            // Wait a few seconds for Xvfb to start. Ugly, but identical to xvfb-run.
+            if (xvfb.waitFor(3, TimeUnit.SECONDS)) {
+                throw GradleException("Xvfb unexpectedly exited (with status code ${xvfb.exitValue()})")
+            }
+
+            exec.environment("XAUTHORITY", authFile.toString())
+            exec.environment("DISPLAY", ":$server")
+
+            run()
+        }
+    }
+
+    @get:Inject
+    protected abstract val fsOperations: FileSystemOperations
+
+    companion object {
+        fun get(gradle: Gradle): Provider<MinecraftRunnerService> =
+            gradle.sharedServices.registerIfAbsent("cc.tweaked.gradle.ClientJavaExec", MinecraftRunnerService::class.java) {
+                maxParallelUsages.set(1)
+            }
+    }
+}

buildSrc/src/main/kotlin/cc/tweaked/gradle/ProcessHelpers.kt

@@ -1,7 +1,9 @@
 package cc.tweaked.gradle
 
 import org.codehaus.groovy.runtime.ProcessGroovyMethods
+import org.gradle.api.GradleException
 import java.io.BufferedReader
+import java.io.File
 import java.io.IOException
 import java.io.InputStreamReader
 
@@ -31,4 +33,14 @@ internal object ProcessHelpers {
         if (process.waitFor() != 0) throw IOException("Command exited with a non-0 status")
         return out
     }
+
+    fun onPath(name: String): Boolean {
+        val path = System.getenv("PATH") ?: return false
+        return path.splitToSequence(File.pathSeparator).any { File(it, name).exists() }
+    }
+}
+
+internal fun Process.waitForOrThrow(message: String) {
+    val ret = waitFor()
+    if (ret != 0) throw GradleException("$message (exited with $ret)")
 }