From 91f21e9322c3d7f50e31de1199bd7efaad305960 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 06:49:12 +0000 Subject: [PATCH 1/9] test(e2e): cover the supported Minecraft versions with a runtime smoke suite The full Plugwright suite only runs on 1.21.8, so nothing proved that the plugin loads and answers on the versions it claims to support. The oldest Paper builds need a few things the tooling does not give them per environment, and this wires those up: - a launcher script per version, so the server runs on the JDK that version needs and the legacy CraftBukkit builds do not see the --nogui flag they reject - the newest Paper build per version, because the oldest one the API lists for 1.8.8 and 1.12.2 is a paperclip jar that fetches the vanilla server jar from a URL Mojang retired - use-native-transport=false for 1.8.8, whose Netty 4.0 cannot use direct buffers on a Java 11 JVM - EssentialsX 2.20.1 for the legacy versions, because 2.21 and newer need an Adventure API that Paper only ships from 1.18 on 1.12.2 joins the supported versions list on Java 11. --- .github/workflows/e2e.yml | 55 ++++ build.gradle.kts | 521 +++++++++++++++++++++++++++++-- docs/e2e-coverage.md | 25 +- docs/e2e-testing.md | 90 +++++- src/test/e2e/tests/smoke.spec.ts | 67 ++++ 5 files changed, 732 insertions(+), 26 deletions(-) create mode 100644 src/test/e2e/tests/smoke.spec.ts diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 8e821114a..938dbe54e 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -101,5 +101,60 @@ jobs: build/reports/plugwright/** src/test/e2e/generated/*/run/logs/** + # The runtime smoke suite: the same plugin, the same staged config and the same dependencies on + # every Minecraft version the plugin claims to support. It proves that the plugin loads and + # answers on the old servers, which the full suite above cannot run on. + smoke: + name: Runtime smoke (every supported version) + runs-on: ubuntu-latest + # Six servers, one after the other, each with its own Paper build to download and its own JDK + # to provision. + timeout-minutes: 45 + + env: + TEST_TIMEOUT: "60000" + + steps: + - name: Checkout sources + uses: actions/checkout@v7 + + - name: Set up Java + uses: actions/setup-java@v5 + with: + distribution: temurin + # The build itself is pinned to Java 21 by the toolchain in build.gradle.kts. Each smoke + # environment starts its server on the JDK its Minecraft version needs, which Gradle + # provisions through the toolchain resolver. + java-version: "21" + + - name: Setup Gradle + uses: gradle/actions/setup-gradle@v6 + with: + cache-provider: basic + + - name: Make Gradle wrapper executable + run: chmod +x ./gradlew + + - name: Set up Node + uses: actions/setup-node@v5 + with: + node-version: "22" + cache: npm + cache-dependency-path: src/test/e2e/package-lock.json + + - name: Run the smoke suite + run: ./gradlew plugwrightSmokeTest --no-daemon --stacktrace + + - name: Upload server logs and test reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: e2e-smoke + if-no-files-found: warn + retention-days: 14 + path: | + build/reports/plugwright/** + src/test/e2e/generated/*/run/logs/** + # No merge, no deploy: this workflow builds, tests and reports. The jar it produces is never # published from here, and the job holds no write permission beyond its own checks. \ No newline at end of file diff --git a/build.gradle.kts b/build.gradle.kts index 9539df2a0..d05adfcfa 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,11 +1,22 @@ import com.diffplug.gradle.spotless.FormatExtension import com.diffplug.gradle.spotless.SpotlessExtension import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import me.drownek.plugwright.PlugwrightTestTask +import me.drownek.plugwright.api.ConfigNodeBuilder +import me.drownek.plugwright.api.ConfigValue +import me.drownek.plugwright.api.EnvironmentSpec +import me.drownek.plugwright.api.PlugwrightMode +import me.drownek.plugwright.local.LocalEnvironmentSpec import me.drownek.plugwright.local.LocalMode import org.gradle.language.jvm.tasks.ProcessResources import org.jetbrains.kotlin.gradle.dsl.JvmTarget import org.jetbrains.kotlin.gradle.tasks.KotlinCompile import xyz.jpenilla.runpaper.task.RunServer +import java.net.URI +import java.net.http.HttpClient +import java.net.http.HttpRequest +import java.net.http.HttpResponse +import java.util.concurrent.TimeUnit plugins { `java-library` @@ -492,6 +503,7 @@ data class MinecraftRunTarget( val supportedMinecraftVersions = listOf( MinecraftRunTarget("1.8.8", 11), + MinecraftRunTarget("1.12.2", 11), MinecraftRunTarget("1.16.5", 16), MinecraftRunTarget("1.18.2", 17), MinecraftRunTarget("1.19.4", 17), @@ -571,13 +583,114 @@ tasks { } } +/* + * The versions the runtime smoke suite runs on. + * + * The full suite only runs on a modern server; the oldest supported versions get the smoke suite + * instead, which proves the plugin loads and answers there without dragging the whole suite along. + * The newest supported release is included so a regression in the current Paper build is caught + * here too. + * + * The JDK each version needs is the one `supportedMinecraftVersions` already maps it to, so the + * two lists cannot drift apart. + */ +data class SmokeEnvironment( + val name: String, + val minecraftVersion: String, + val javaVersion: Int, + /** + * Whether CraftBukkit on this version refuses the `--nogui` flag the runner passes. + * + * The old builds parse their command line with joptsimple and reject the flag as an unknown + * option, which stops the server before it loads a single plugin. + */ + val refusesNoGui: Boolean, + /** + * Whether the server's Netty cannot use direct buffers on a modern JVM. + * + * Netty 4.0, which 1.8.8 ships, decides whether it can by reading the address of a heap + * buffer, which no longer reads as zero on Java 9 and later. Without that support its native + * transport cannot work, and every connection the server accepts dies with "Unable to access + * address of buffer". + */ + val needsNioTransport: Boolean, + /** + * The EssentialsX build this environment runs. + * + * EssentialsX 2.21 and newer need an Adventure API that Paper only started shipping with + * 1.18, so on the older servers the plugin fails to enable with a NoClassDefFoundError for + * MiniMessage's TagResolver. 2.20.1 is the newest build that does not need it. + */ + val essentialsX: String, + /** + * The Minecraft version the bot connects with. + * + * minecraft-data — the protocol table mineflayer reads — has no entry for 26.3 yet, and a bot + * asked for a version it does not know refuses to start at all. 26.1 and 26.2 are the same + * protocol, so the bot is pointed at 26.1 for those. + */ + val botVersion: String, + /** + * Whether a bot can be driven on this version at all. + * + * 26.3 is a protocol newer than anything minecraft-data knows, so the server refuses the bot + * as an outdated client. The server itself is fine, so that environment gets a server-level + * check instead of the bot-driven one. + */ + val botless: Boolean, +) + +/* + * The EssentialsX builds the environments run. + * + * EssentialsX 2.21 and newer need an Adventure API that Paper only started shipping with 1.18, + * so on the older servers the plugin fails to enable with a NoClassDefFoundError for MiniMessage's + * TagResolver. 2.20.1 is the newest build that does not need it, and it is what the legacy + * environments run. + */ +val CURRENT_ESSENTIALS_X = "https://cdn.modrinth.com/data/hXiIvTyT/versions/nY6VN1XH/EssentialsX-2.22.0.jar" +val LEGACY_ESSENTIALS_X = "https://cdn.modrinth.com/data/hXiIvTyT/versions/puUfqBpY/EssentialsX-2.20.1.jar" + +val smokeEnvironments = listOf( + "1.8.8", + "1.12.2", + "1.16.5", + "1.21.8", + "26.1.2", + supportedMinecraftVersions.last().minecraftVersion, +) + .distinct() + .map { version -> + val target = supportedMinecraftVersions.first { it.minecraftVersion == version } + val legacy = version == "1.8.8" || version == "1.12.2" || version == "1.16.5" + + SmokeEnvironment( + name = "smoke-$version", + minecraftVersion = version, + javaVersion = target.javaVersion, + refusesNoGui = version == "1.8.8" || version == "1.12.2", + needsNioTransport = version == "1.8.8", + essentialsX = if (legacy) LEGACY_ESSENTIALS_X else CURRENT_ESSENTIALS_X, + botVersion = if (version.startsWith("26.")) "26.1" else version, + botless = version == "26.3", + ) + } + /* * End-to-end testing. * * Plugwright downloads a Paper server, stages Guilds' shadowJar plus the three plugins Guilds - * refuses to start without, starts the server and drives it with real mineflayer bots. One - * environment only: 1.21.8 on Java 21, the newest version in `supportedMinecraftVersions` that - * both Paper and minecraft-data can serve. Adding the historical matrix is separate work. + * refuses to start without, starts the server and drives it with real mineflayer bots. + * + * Two sets of environments live here: + * + * - "local" runs the whole suite on one version, 1.21.8 by default and whatever + * -Pe2eMinecraftVersion names otherwise. This is what the pull-request job runs. + * + * - The smoke environments run the same plugin, the same staged config and the same dependencies + * on the versions the full suite cannot run on: the oldest ones the plugin supports and the + * newest one. They run a single spec file and have their own task, so the pull-request job is + * left exactly as it was. */ plugwright { testsDir.set(file("src/test/e2e")) @@ -588,36 +701,396 @@ plugwright { minecraftVersion.set( providers.gradleProperty("e2eMinecraftVersion").getOrElse("1.21.8") ) - acceptEula.set(true) - jvmArgs.set(listOf("-Xmx3G")) + configureGuildsEnvironment(listOf("-Xmx3G"), CURRENT_ESSENTIALS_X) + } + + smokeEnvironments.forEach { smoke -> + create(smoke.name, LocalMode) { + minecraftVersion.set(smoke.minecraftVersion) + + /* + * The pull-request job runs the full suite on "local" and nothing else. The smoke + * environments have their own task, so leaving them out of the matrix keeps that + * job's runtime and its failure surface exactly what it was. + */ + includeInMatrix.set(false) + + configureGuildsEnvironment(listOf("-Xmx1500M"), smoke.essentialsX) + } + } + } +} + +/** + * The plugin set and the staged configuration every Guilds E2E environment runs with. + * + * Guilds has no plugin.yml defaults for its commands, so a non-op player has none of + * `guilds.command.*`. Staging the config here is what makes every test deterministic: + * `plugwrightClean` wipes the run directory before each provision, so the plugin only ever sees + * this config and a freshly generated tiers.yml/roles.yml. + */ +fun LocalEnvironmentSpec.configureGuildsEnvironment(jvmArgs: List, essentialsX: String) { + acceptEula.set(true) + this.jvmArgs.set(jvmArgs) + + /* + * The same pins `configureGuildsRunServer` already downloads for the manual runServer* tasks. + * Vault is not on Hangar or any Maven repo, so it comes from the upstream release jar; + * LuckPerms' loader URL moved, so Modrinth's copy is used. + */ + downloadPlugins { + url("https://github.com/MilkBowl/Vault/releases/download/1.7.3/Vault.jar") + url(essentialsX) + url("https://cdn.modrinth.com/data/Vebnzrzj/versions/b0mk8uS6/LuckPerms-Bukkit-5.5.71.jar") + } + + writeFiles { + file("plugins/Guilds/config.yml", file("src/test/e2e/server/plugins/Guilds/config.yml")) + file("plugins/Guilds/tiers.yml", file("src/test/e2e/server/plugins/Guilds/tiers.yml")) + file("plugins/Guilds/buffs.yml", file("src/test/e2e/server/plugins/Guilds/buffs.yml")) + } + + plugins { + local("guilds") + } +} + +/** + * The script a smoke environment's server is started with. + * + * It exists for two reasons, both of them things plugwright's local mode cannot do per + * environment: + * + * - It runs the server on the JDK that the Minecraft version needs. Plugwright starts the server + * with the JVM of the project's Java toolchain, which is one setting for the whole build, and a + * Paper 1.8.8 server does not run on Java 21. + * + * - It makes the legacy servers start at all. Their CraftBukkit refuses the `--nogui` flag the + * runner always passes. + */ +fun smokeLauncherScript(javaPath: String, refusesNoGui: Boolean): String { + val dropNoGui = if (refusesNoGui) { + """ + # CraftBukkit on this version rejects --nogui as an unknown option, and the runner always + # passes it, so it is dropped here. + count=${'$'}# + index=0 + while [ "${'$'}index" -lt "${'$'}count" ]; do + argument="${'$'}1" + shift + if [ "${'$'}argument" != "--nogui" ]; then + set -- "${'$'}@" "${'$'}argument" + fi + index=${'$'}((index + 1)) + done + """.trimIndent() + } else { + "" + } + + return """ + #!/bin/sh + # Starts the Paper server for one of the Guilds smoke environments. + $dropNoGui + exec "$javaPath" "${'$'}@" + """.trimIndent() + System.lineSeparator() +} + +/** + * The server.properties the 1.8.8 environment runs with. + * + * That server ships Netty 4.0, which decides whether it can use direct buffers by reading the + * address of a heap buffer, which no longer reads as zero on Java 9 and later. Without that support + * the native transport cannot work, and every connection the server accepts dies with "Unable to + * access address of buffer" — the bot never gets a reply and the smoke test fails on a socket + * error rather than on anything to do with Guilds. + * + * Turning the native transport off puts the server on the same NIO transport every Windows server + * uses, which works on a Java 11 JVM. + */ +fun writeLegacyServerProperties(properties: File) { + val line = "use-native-transport=false" + val existing = if (properties.exists()) properties.readText() else "" + + val updated = if (existing.lines().any { it.trim().startsWith("use-native-transport=") }) { + existing.lines().joinToString("\n") { + if (it.trim().startsWith("use-native-transport=")) line else it + } + } else { + listOf(existing.trimEnd(), line).filter { it.isNotEmpty() }.joinToString("\n") + } + + properties.parentFile.mkdirs() + properties.writeText(updated + System.lineSeparator()) +} + +/** + * Starts a Paper server and checks that Guilds came up on it. + * + * This is what a version no bot can be driven on gets instead of the smoke spec. It cannot prove + * that the command layer works — that needs a player — but it does prove that the plugin loads, + * that its hard dependencies load with it, and that Guilds finds the economy and the permission + * provider it refuses to start without. + */ +fun checkServerLoads(smoke: SmokeEnvironment, launcher: File, runDir: File) { + val server = ProcessBuilder( + "/bin/sh", + launcher.absolutePath, + "-Xmx1500M", + "-Dcom.mojang.eula.agree=true", + "-jar", + "server.jar", + ) + .directory(runDir) + .redirectErrorStream(true) + .start() + + val log = StringBuilder() + val reader = Thread { + server.inputStream.bufferedReader().forEachLine { line -> + synchronized(log) { log.appendLine(line) } + } + } + reader.isDaemon = true + reader.start() + + try { + val deadline = System.currentTimeMillis() + 300_000 + while (System.currentTimeMillis() < deadline) { + val ready = synchronized(log) { log.contains("Done (") } + if (ready || !server.isAlive) break + Thread.sleep(500) + } + + val text = synchronized(log) { log.toString() } + check(text.contains("Done (")) { + "The Paper server for Minecraft ${smoke.minecraftVersion} did not finish starting.\n$text" + } + for (plugin in listOf("Vault", "Essentials", "LuckPerms", "Guilds")) { + check(text.contains("Enabling $plugin")) { + "$plugin was not enabled on Minecraft ${smoke.minecraftVersion}.\n$text" + } + } + check(!text.contains("Disabling Guilds")) { + "Guilds disabled itself on Minecraft ${smoke.minecraftVersion}.\n$text" + } + check(text.contains("Economy Found:")) { + "Guilds found no economy on Minecraft ${smoke.minecraftVersion}.\n$text" + } + check(text.contains("Permissions Found:")) { + "Guilds found no permission provider on Minecraft ${smoke.minecraftVersion}.\n$text" + } + } finally { + server.outputStream.write("stop\n".toByteArray()) + server.outputStream.flush() + if (!server.waitFor(60, TimeUnit.SECONDS)) { + server.destroyForcibly() + } + } +} + +/* + * The smoke environments need a Paper build of their own as well. + * + * Plugwright's provision task downloads the oldest build the API lists for a version. For 1.8.8 + * and 1.12.2 that build is a paperclip jar whose patch data points at a Mojang URL that no longer + * serves the vanilla server jar, so the server exits before it starts. The smoke environments + * download the newest build instead, and the provision task leaves a server jar that is already in + * the run directory alone. + */ +fun downloadNewestPaperBuild(version: String, serverJar: File, versionMarker: File) { + val client = HttpClient.newHttpClient() + + fun get(url: String): String { + val request = HttpRequest.newBuilder(URI.create(url)).GET().build() + val response = client.send(request, HttpResponse.BodyHandlers.ofString()) + check(response.statusCode() == 200) { "GET $url returned ${response.statusCode()}" } + return response.body() + } + + @Suppress("UNCHECKED_CAST") + val builds = groovy.json.JsonSlurper() + .parseText(get("https://fill.papermc.io/v3/projects/paper/versions/$version/builds")) as List> + val newest = builds.maxByOrNull { (it["id"] as Number).toInt() } + ?: error("Paper publishes no build for Minecraft $version") + + @Suppress("UNCHECKED_CAST") + val build = groovy.json.JsonSlurper() + .parseText( + get("https://fill.papermc.io/v3/projects/paper/versions/$version/builds/${newest["id"]}") + ) as Map + @Suppress("UNCHECKED_CAST") + val downloads = build["downloads"] as Map> + val download = downloads["server:default"] ?: downloads["application"] + ?: error("Paper build ${newest["id"]} of Minecraft $version has no server download") + + serverJar.parentFile.mkdirs() + URI.create(download["url"] as String).toURL().openStream().use { input -> + serverJar.outputStream().use { output -> input.copyTo(output) } + } + versionMarker.writeText(version) +} + +/* + * Wires the smoke environments up. + * + * afterEvaluate, because the plugwright plugin registers the environments and their tasks from + * its own afterEvaluate block, which runs first. + */ +afterEvaluate { + val smokeServerChecks = mutableListOf>() + val smokeTestTasks = smokeEnvironments.map { smoke -> + val entry = plugwright.environments.get(smoke.name)!! + @Suppress("UNCHECKED_CAST") + val spec = entry.spec as LocalEnvironmentSpec + val runDir = spec.runDir.get().asFile + val suffix = smoke.name.replaceFirstChar { it.uppercase() } + + /* + * The launcher script. The environment's runner configuration points at it, so the server + * runs on this version's JDK rather than on the project's toolchain. + */ + val launcherScript = layout.buildDirectory.file("smoke/${smoke.name}/launcher.sh") + val launcher = tasks.register("smokeLauncher$suffix") { + val javaLauncher = javaToolchains.launcherFor { + languageVersion.set(JavaLanguageVersion.of(smoke.javaVersion)) + } + + inputs.property("minecraftVersion", smoke.minecraftVersion) + inputs.property("javaVersion", smoke.javaVersion) + outputs.file(launcherScript) + + doLast { + val script = launcherScript.get().asFile + script.parentFile.mkdirs() + script.writeText( + smokeLauncherScript( + javaLauncher.get().executablePath.asFile.absolutePath, + smoke.refusesNoGui, + ) + ) + script.setExecutable(true) + } + } + + /* The Paper build, downloaded before the provision task looks for a server jar. */ + val serverJar = File(runDir, "server.jar") + val versionMarker = File(runDir, ".minecraft-version") + val paper = tasks.register("smokePaper$suffix") { + inputs.property("minecraftVersion", smoke.minecraftVersion) + outputs.file(serverJar) + + doLast { + downloadNewestPaperBuild(smoke.minecraftVersion, serverJar, versionMarker) + logger.lifecycle("Paper ${smoke.minecraftVersion} downloaded to $serverJar") + } + } + + val clean = tasks.named("plugwrightClean$suffix") + + /* + * The server properties the versions that need them run with. The provision task rewrites + * the keys it knows about and leaves the rest alone, so writing this first is enough. + */ + val properties = File(runDir, "server.properties") + val serverProperties = tasks.register("smokeServerProperties$suffix") { + outputs.file(properties) + + doLast { + if (smoke.needsNioTransport) { + writeLegacyServerProperties(properties) + } + } + } + + tasks.named("plugwrightProvision$suffix") { + dependsOn(paper) + dependsOn(serverProperties) + } + paper.configure { + mustRunAfter(clean) + } + serverProperties.configure { + mustRunAfter(clean) + } + + val test = tasks.named("plugwrightTest$suffix") + test.configure { + dependsOn(launcher) /* - * The same pins `configureGuildsRunServer` already downloads for the manual - * runServer* tasks. Vault is not on Hangar or any Maven repo, so it comes from the - * upstream release jar; LuckPerms' loader URL moved, so Modrinth's copy is used. + * Only the smoke spec runs here. The rest of the suite is written against a modern + * server and its expectations do not hold on 1.8.8. */ - downloadPlugins { - url("https://github.com/MilkBowl/Vault/releases/download/1.7.3/Vault.jar") - url("https://cdn.modrinth.com/data/hXiIvTyT/versions/nY6VN1XH/EssentialsX-2.22.0.jar") - url("https://cdn.modrinth.com/data/Vebnzrzj/versions/b0mk8uS6/LuckPerms-Bukkit-5.5.71.jar") - } + testFiles.set("smoke") /* - * Guilds has no plugin.yml defaults for its commands, so a non-op player has none of - * `guilds.command.*`. Staging the config here is what makes every test deterministic: - * `plugwrightClean` wipes the run directory before each provision, so the plugin only - * ever sees this config and a freshly generated tiers.yml/roles.yml. + * The server runs on the launcher script instead of on the JVM of the project's Java + * toolchain, and the bot connects with the protocol version mineflayer knows. Everything + * else in the environment configuration is plugwright's own, so only those two change. */ - writeFiles { - file("plugins/Guilds/config.yml", file("src/test/e2e/server/plugins/Guilds/config.yml")) - file("plugins/Guilds/tiers.yml", file("src/test/e2e/server/plugins/Guilds/tiers.yml")) - file("plugins/Guilds/buffs.yml", file("src/test/e2e/server/plugins/Guilds/buffs.yml")) - } + environmentConfig.set( + providers.provider { + val node = ConfigNodeBuilder() + @Suppress("UNCHECKED_CAST") + (entry.mode as PlugwrightMode).serialize(entry.spec as EnvironmentSpec, node) + + val original = node.build() + original.copy( + entries = original.entries + mapOf( + "javaPath" to ConfigValue.Str(launcherScript.get().asFile.absolutePath), + "minecraftVersion" to ConfigValue.Str(smoke.botVersion), + ) + ) + } + ) + } - plugins { - local("guilds") + if (smoke.botless) { + /* + * No bot can be driven on this version, so the bot-driven task has nothing to do. The + * environment stays, because it is what provisions the server the check below runs. + */ + test.configure { + enabled = false } + + tasks.register("smokeServerCheck$suffix") { + group = "verification" + description = + "Checks that Guilds loads and finds its providers on Minecraft ${smoke.minecraftVersion}." + + dependsOn(launcher) + dependsOn(tasks.named("plugwrightProvision$suffix")) + + doLast { + checkServerLoads(smoke, launcherScript.get().asFile, runDir) + } + }.also { smokeServerChecks.add(it) } } + + test + } + + tasks.register("plugwrightSmokeTest") { + group = "verification" + description = "Runs the runtime smoke suite on every supported Minecraft version." + + dependsOn(smokeTestTasks) + dependsOn(smokeServerChecks) + } + + /* + * One server at a time. Every environment binds the same port, and two servers on one port + * means one of them fails to bind and the run fails for the wrong reason. + */ + smokeTestTasks.zipWithNext().forEach { (earlier, later) -> + later.configure { mustRunAfter(earlier) } + } + + val matrix = tasks.named("plugwrightTest") + smokeTestTasks.forEach { smokeTask -> + smokeTask.configure { mustRunAfter(matrix) } } } diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 10d87eacd..fa1736c9a 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -6,6 +6,10 @@ how to run any of it. 141 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. +The runtime smoke suite (`tests/smoke.spec.ts`, 5 tests) is a separate, much smaller suite that +runs on the older Minecraft versions the full suite cannot. It is listed under "Runtime smoke +coverage" below. + ## Preflight — `plugins/guilds-preflight.spec.ts` Runs first, every time. A failure here means the environment is broken and nothing after it is @@ -188,6 +192,25 @@ the server answering in another language for the rest of the run. ACF's `usePerI false)` is what makes that safe, and one player switching language leaves everyone else in English asserts it rather than assuming it. +## Runtime smoke coverage — `tests/smoke.spec.ts` (5) + +The full suite above is written against a modern server. The smoke suite is the part that has to +hold on every version the plugin claims to support, so it is much smaller: the plugin and its +hard dependencies came up, Vault found an economy and a permission provider, the economy moves +money, a command answers, and a guild can be created. + +| Requirement | Test | +| --- | --- | +| The plugin loads | Guilds and its hard dependencies are enabled | +| The plugin finds what it needs | the economy and permission providers Vault needs are registered | +| The economy works | the economy moves money | +| The command layer works | a guild command answers | +| The plugin is usable | a guild can be created | + +The same spec runs on every environment, the modern ones included, so a smoke failure on 1.21.8 +is as loud as one on 1.8.8. See `e2e-testing.md` for how the per-version environments are set up +and what each one can and cannot prove. + ## Fixture self-check — `tests/bank.spec.ts` `STARTING_BALANCE is what the fixtures actually fund` is not a Guilds test. It checks that the @@ -206,5 +229,5 @@ Out of scope for this phase, as agreed: | Server restart and migration | The suite assumes a freshly provisioned server and never restarts one; migration testing needs a stop/start harness the runner does not currently expose | | Internal API and event inspection | Deliberately excluded: the suite asserts on player-visible outcomes only. Reaching into the API would make tests pass on internals that a player would never see | | PlaceholderAPI | Needs a second plugin and a way to read resolved placeholders as a bot; there is no command surface for it | -| Historical Minecraft versions | One environment (1.21.8 / Java 21). The build pins `e2eMinecraftVersion` as a property, so a matrix is a matter of adding environments once one is proven | +| Historical Minecraft versions | The full suite runs on one version (1.21.8 / Java 21). The older ones are covered by the runtime smoke suite instead, which proves the plugin loads and answers there rather than exercising every feature | | Spy mode | `GuildHandler#isSpy` has no command or GUI surface a bot can exercise | \ No newline at end of file diff --git a/docs/e2e-testing.md b/docs/e2e-testing.md index 0c17775ec..cf6aa559a 100644 --- a/docs/e2e-testing.md +++ b/docs/e2e-testing.md @@ -27,6 +27,85 @@ The first run takes a few minutes because it downloads a server. After that, the ./gradlew plugwrightTest # re-run the whole suite ``` +### The smoke suite on the other versions + +`plugwrightTest` runs the whole suite on 1.21.8. The versions the plugin supports but the full +suite cannot run on are covered by a smaller smoke suite instead: the plugin and its hard +dependencies come up, Vault finds an economy and a permission provider, the economy moves money, a +command answers, and a guild can be created. + +```bash +./gradlew plugwrightSmokeTest # every supported version, one after the other +``` + +One environment at a time, because every environment binds the same port. The run takes about +three minutes, most of it server start-up. + +| Environment | Minecraft | JDK | What it runs | +| --- | --- | --- | --- | +| `smoke-1.8.8` | 1.8.8 | 11 | the smoke spec | +| `smoke-1.12.2` | 1.12.2 | 11 | the smoke spec | +| `smoke-1.16.5` | 1.16.5 | 16 | the smoke spec | +| `smoke-1.21.8` | 1.21.8 | 21 | the smoke spec | +| `smoke-26.1.2` | 26.1.2 | 25 | the smoke spec | +| `smoke-26.3` | 26.3 | 25 | a server-level check, see below | + +A single environment can be run on its own: + +```bash +./gradlew plugwrightTestSmoke-1.8.8 +``` + +#### What the older environments need, and why + +Three things about the old servers are not obvious, and each of them is handled in +`build.gradle.kts` rather than in the test: + +**A JDK of their own.** Plugwright's local mode starts the server with the JVM of the project's +Java toolchain, and that toolchain is one setting for the whole build. A Paper 1.8.8 server does +not run on Java 21, and a Paper 26.3 server does not run on it either. Every smoke environment +therefore gets a launcher script that runs the server on the JDK its version needs. + +**No `--nogui` flag.** The runner always passes it, and the old CraftBukkit builds reject it as an +unknown option, which stops the server before it loads a single plugin. The launcher script drops +it for the versions that refuse it. + +**No native transport.** The old CraftBukkit builds ship a Netty that predates Java 9 and decides +whether it can use direct buffers by reading the address of a heap buffer, which no longer reads +as zero on a modern JVM. Without that support the native transport cannot work, and every +connection the server accepts dies with "Unable to access address of buffer" — the bot never gets +a reply, and the test fails on a socket error rather than on anything to do with Guilds. The +legacy environments set `use-native-transport=false` in `server.properties`, which puts the server +on the same NIO transport every Windows server uses. + +**An older EssentialsX.** EssentialsX 2.21 and newer need an Adventure API that Paper only +started shipping with 1.18, so on 1.16.5 the plugin fails to enable with a +`NoClassDefFoundError` for MiniMessage's `TagResolver`. The legacy environments run 2.20.1, the +newest build that does not need it. + +**The newest Paper build, not the oldest.** Plugwright's provision task downloads the oldest +build the API lists for a version, and for 1.8.8 and 1.12.2 that build is a paperclip jar whose +patch data points at a Mojang URL that no longer serves the vanilla server jar, so the server +exits before it starts. The smoke environments download the newest build instead. + +#### 26.3 has no bot yet + +`minecraft-data`, the protocol table mineflayer reads, has no entry for 26.3. The server is fine +— it comes up, Guilds enables and it finds its providers — but the bot is refused as an outdated +client, so no test that needs a player can run there. That environment gets a server-level check +instead: + +```bash +./gradlew smokeServerCheckSmoke-26.3 +``` + +It starts the server and asserts that Guilds and its hard dependencies enabled, that Guilds did +not disable itself, and that it found the economy and the permission provider. It cannot prove +that the command layer works, because that needs a player. + +26.1.2 is the same protocol as 26.1, which is the newest one minecraft-data knows, so the bot +connects to that environment as 26.1. + ### Running one file, or one test Plugwright filters by substring, through the generated config rather than a Gradle flag. The config @@ -63,6 +142,12 @@ values out of that config and will fail confusingly against the defaults. Useful when a test fails and you want to walk into the situation yourself with a real client. +The smoke environments have their own, and the server they start is the one the test drives: + +```bash +./gradlew plugwrightRunServerSmoke-1.8.8 +``` + ### Wiping state ```bash @@ -78,7 +163,7 @@ writes anything outside that directory. ``` src/test/e2e/ plugins/ runner plugin: fixtures, assertions, read-back helpers, expected strings - tests/ the specs, one file per feature area + tests/ the specs, one file per feature area, plus smoke.spec.ts server/plugins/ the Guilds config the tests are written against generated/ the server, worlds and reports (git-ignored) ``` @@ -92,6 +177,9 @@ file is the only thing that needs to change. A full run is roughly ten minutes: about 1,500 commands paced at 200ms each, plus the handful of tests that wait on a real three-second cooldown expiring. +The smoke suite is about three minutes for every version together, almost all of it server +start-up: each version downloads its own Paper build and starts its own server. + The runner's per-test timeout defaults to 30s, which the cooldown tests sit uncomfortably close to. CI sets `TEST_TIMEOUT=60000` for headroom. To match locally: diff --git a/src/test/e2e/tests/smoke.spec.ts b/src/test/e2e/tests/smoke.spec.ts new file mode 100644 index 000000000..5ae0b4a4a --- /dev/null +++ b/src/test/e2e/tests/smoke.spec.ts @@ -0,0 +1,67 @@ +import { expect, test } from '@plugwright/runner'; +import { MSG } from '../plugins/support/expected.js'; +import { stripColors } from '../plugins/support/text.js'; + +/** + * The runtime smoke test. + * + * The rest of the suite is written against a modern server and cannot run on the oldest versions + * the plugin supports. This file can: it only asserts that the plugin came up, that the things it + * refuses to start without came up with it, and that it answers. + * + * That is deliberately less than the full suite and deliberately more than "the server started". A + * plugin that loads but whose economy provider never registered, or whose command layer is dead, + * is broken on that version, and only a check that drives it finds that out. + * + * It runs on every environment, the modern ones included, so a smoke failure on 1.21.8 is as loud + * as one on 1.8.8. + */ + +/** The plugins Guilds refuses to start without, as they appear in the server log. */ +const REQUIRED_PLUGINS = ['Vault', 'Essentials', 'LuckPerms'] as const; + +test('Guilds and its hard dependencies are enabled', async ({ server }) => { + const log = server.session.consoleLog.slice().join('\n'); + + for (const plugin of REQUIRED_PLUGINS) { + expect(log).toContain(`Enabling ${plugin}`); + expect(log).not.toContain(`Could not load '${plugin}'`); + } + + // Guilds disables itself rather than starting in a broken state when no economy has registered. + expect(log).toContain('Enabling Guilds'); + expect(log).not.toContain('Disabling Guilds'); +}); + +test('the economy and permission providers Vault needs are registered', async ({ server }) => { + const vaults = stripColors(await server.execute('vault-info')); + + expect(vaults).toContain('Economy:'); + expect(vaults).toContain('Permission:'); + expect(vaults).not.toContain('Economy: null'); +}); + +test('the economy moves money', async ({ player, guilds }) => { + await guilds.fund(player, 2500); + + expect(await guilds.balanceOf(player)).toBe(2500); +}); + +test('a guild command answers', async ({ player, guilds }) => { + await guilds.run(player, '/guild check'); + await expect(player).toHaveReceivedMessage(MSG.noPendingInvites, { timeout: 10_000 }); +}); + +test('a guild can be created', async ({ player, guilds }) => { + const name = guilds.guildName('smoke'); + + await guilds.fund(player, 5000); + await guilds.run(player, `/guild create ${name} ${name.slice(0, 8)}`); + await expect(player).toHaveReceivedMessage(MSG.createWarning); + + await guilds.confirm(player); + await expect(player).toHaveReceivedMessage(MSG.created(name)); + + // The fixture removes the guild again once the test is over. + guilds.track(name); +}); From 019ad0bebc31d9408f2dfbe84749cf706fb3a4a2 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 06:51:57 +0000 Subject: [PATCH 2/9] test(e2e): only write the server properties where they are needed --- build.gradle.kts | 19 +++++++++++-------- docs/e2e-testing.md | 3 +++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index d05adfcfa..3fd861964 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -993,26 +993,29 @@ afterEvaluate { * the keys it knows about and leaves the rest alone, so writing this first is enough. */ val properties = File(runDir, "server.properties") - val serverProperties = tasks.register("smokeServerProperties$suffix") { - outputs.file(properties) + if (smoke.needsNioTransport) { + val serverProperties = tasks.register("smokeServerProperties$suffix") { + outputs.file(properties) - doLast { - if (smoke.needsNioTransport) { + doLast { writeLegacyServerProperties(properties) } } + + tasks.named("plugwrightProvision$suffix") { + dependsOn(serverProperties) + } + serverProperties.configure { + mustRunAfter(clean) + } } tasks.named("plugwrightProvision$suffix") { dependsOn(paper) - dependsOn(serverProperties) } paper.configure { mustRunAfter(clean) } - serverProperties.configure { - mustRunAfter(clean) - } val test = tasks.named("plugwrightTest$suffix") test.configure { diff --git a/docs/e2e-testing.md b/docs/e2e-testing.md index cf6aa559a..76dc7fef0 100644 --- a/docs/e2e-testing.md +++ b/docs/e2e-testing.md @@ -106,6 +106,9 @@ that the command layer works, because that needs a player. 26.1.2 is the same protocol as 26.1, which is the newest one minecraft-data knows, so the bot connects to that environment as 26.1. +The legacy environments start the server through a shell script, and so does the 26.3 server +check, so they do not run on Windows. + ### Running one file, or one test Plugwright filters by substring, through the generated config rather than a Gradle flag. The config From 5676c6854c1a29eaa389bf5669c0e3ada91d5a02 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 06:56:08 +0000 Subject: [PATCH 3/9] test(e2e): run the server-level check in the same chain as the smoke environments --- build.gradle.kts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 3fd861964..8b1d731d2 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1085,14 +1085,15 @@ afterEvaluate { /* * One server at a time. Every environment binds the same port, and two servers on one port - * means one of them fails to bind and the run fails for the wrong reason. + * means one of them fails to bind and the run fails for the wrong reason. The server-level + * check starts a server too, so it is part of the chain. */ - smokeTestTasks.zipWithNext().forEach { (earlier, later) -> + (smokeTestTasks + smokeServerChecks).zipWithNext().forEach { (earlier, later) -> later.configure { mustRunAfter(earlier) } } val matrix = tasks.named("plugwrightTest") - smokeTestTasks.forEach { smokeTask -> + (smokeTestTasks + smokeServerChecks).forEach { smokeTask -> smokeTask.configure { mustRunAfter(matrix) } } } From 420d4c07dfe5607bd047c7b7e0f0a1efbfb6935c Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 07:10:39 +0000 Subject: [PATCH 4/9] test(e2e): give the smoke tests names of their own --- src/test/e2e/tests/smoke.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/e2e/tests/smoke.spec.ts b/src/test/e2e/tests/smoke.spec.ts index 5ae0b4a4a..cb3cf6153 100644 --- a/src/test/e2e/tests/smoke.spec.ts +++ b/src/test/e2e/tests/smoke.spec.ts @@ -20,7 +20,7 @@ import { stripColors } from '../plugins/support/text.js'; /** The plugins Guilds refuses to start without, as they appear in the server log. */ const REQUIRED_PLUGINS = ['Vault', 'Essentials', 'LuckPerms'] as const; -test('Guilds and its hard dependencies are enabled', async ({ server }) => { +test('Guilds and its dependencies are up', async ({ server }) => { const log = server.session.consoleLog.slice().join('\n'); for (const plugin of REQUIRED_PLUGINS) { @@ -33,7 +33,7 @@ test('Guilds and its hard dependencies are enabled', async ({ server }) => { expect(log).not.toContain('Disabling Guilds'); }); -test('the economy and permission providers Vault needs are registered', async ({ server }) => { +test('Vault has both an economy and a permission provider', async ({ server }) => { const vaults = stripColors(await server.execute('vault-info')); expect(vaults).toContain('Economy:'); From 954920e1efc7c3d622b9561921dccc9a0b0708fd Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 07:11:03 +0000 Subject: [PATCH 5/9] docs(e2e): say which version needs the NIO transport --- docs/e2e-testing.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/e2e-testing.md b/docs/e2e-testing.md index 76dc7fef0..c86b74b54 100644 --- a/docs/e2e-testing.md +++ b/docs/e2e-testing.md @@ -58,7 +58,7 @@ A single environment can be run on its own: #### What the older environments need, and why -Three things about the old servers are not obvious, and each of them is handled in +Five things about the old servers are not obvious, and each of them is handled in `build.gradle.kts` rather than in the test: **A JDK of their own.** Plugwright's local mode starts the server with the JVM of the project's @@ -66,17 +66,18 @@ Java toolchain, and that toolchain is one setting for the whole build. A Paper 1 not run on Java 21, and a Paper 26.3 server does not run on it either. Every smoke environment therefore gets a launcher script that runs the server on the JDK its version needs. -**No `--nogui` flag.** The runner always passes it, and the old CraftBukkit builds reject it as an -unknown option, which stops the server before it loads a single plugin. The launcher script drops -it for the versions that refuse it. - -**No native transport.** The old CraftBukkit builds ship a Netty that predates Java 9 and decides -whether it can use direct buffers by reading the address of a heap buffer, which no longer reads -as zero on a modern JVM. Without that support the native transport cannot work, and every -connection the server accepts dies with "Unable to access address of buffer" — the bot never gets -a reply, and the test fails on a socket error rather than on anything to do with Guilds. The -legacy environments set `use-native-transport=false` in `server.properties`, which puts the server -on the same NIO transport every Windows server uses. +**No `--nogui` flag.** The runner always passes it, and the CraftBukkit builds on 1.8.8 and 1.12.2 +reject it as an unknown option, which stops the server before it loads a single plugin. The +launcher script drops it for those two versions. + +**No native transport, on 1.8.8.** That server ships Netty 4.0, which decides whether it can use +direct buffers by reading the address of a heap buffer, and that address no longer reads as zero on +Java 9 and later. Without that support the native transport cannot work, and every connection the +server accepts dies with "Unable to access address of buffer" — the bot never gets a reply, and the +test fails on a socket error rather than on anything to do with Guilds. The 1.8.8 environment sets +`use-native-transport=false` in `server.properties`, which puts the server on the same NIO +transport every Windows server uses. 1.12.2 and 1.16.5 ship Netty 4.1, which copes with a modern +JVM on its own. **An older EssentialsX.** EssentialsX 2.21 and newer need an Adventure API that Paper only started shipping with 1.18, so on 1.16.5 the plugin fails to enable with a From 4234e24f91fc4b70f7da325ce946fc46dba95732 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 12:41:11 +0000 Subject: [PATCH 6/9] test(e2e): cover the recovery path in the runtime smoke suite The plugin reads four folders under plugins/Guilds when it starts, and the recovery work writes copies of unreadable files into a fifth one. Nothing proved that a file in the backup directory stays out of the plugin's records on a real server, where the reading happens at startup and the versions differ. Two guild files are staged into each smoke environment: one in the data folder, which the plugin has to load, and one in the backup directory, which it has to leave alone. Both are complete, valid guild records, so a plugin that read the wrong folder would load them. The smoke spec asserts both. The 26.3 environment has no bot to ask, so its server-level check asks the console instead. --- build.gradle.kts | 81 +++++++++++++++++-- docs/e2e-coverage.md | 11 ++- docs/e2e-testing.md | 26 +++++- .../1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json | 42 ++++++++++ .../8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json | 42 ++++++++++ src/test/e2e/tests/smoke.spec.ts | 46 +++++++++++ 6 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 src/test/e2e/server/plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json create mode 100644 src/test/e2e/server/plugins/Guilds/data/8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json diff --git a/build.gradle.kts b/build.gradle.kts index 8b1d731d2..081c0ac43 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -676,6 +676,28 @@ val smokeEnvironments = listOf( ) } +/* + * The files the smoke environments stage into the server's plugin data folder. + * + * The plugin reads four folders under plugins/Guilds when it starts: data, arenas, challenges and + * cooldowns. Two files are staged next to them: + * + * - A guild file in the backup directory the recovery work writes to, plugins/Guilds/backups/ + * corrupt//guilds/. It is a complete, valid guild record, so a plugin that treated + * the backup directory as a data folder would load it as a guild. The smoke suite asserts that + * it does not. + * + * - A guild file in the data folder itself. It is the same shape, and the plugin is expected to + * load it, which is what proves the recovery work did not break the ordinary path. + * + * The local environment gets neither. A guild in its data folder would change what the full suite + * sees, and the full suite is not what these files are for. + */ +val smokeStagedFiles = listOf( + "src/test/e2e/server/plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json", + "src/test/e2e/server/plugins/Guilds/data/8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json", +).map { source -> source.removePrefix("src/test/e2e/server/") to file(source) } + /* * End-to-end testing. * @@ -715,12 +737,13 @@ plugwright { */ includeInMatrix.set(false) - configureGuildsEnvironment(listOf("-Xmx1500M"), smoke.essentialsX) + configureGuildsEnvironment(listOf("-Xmx1500M"), smoke.essentialsX, smokeStagedFiles) } } } } + /** * The plugin set and the staged configuration every Guilds E2E environment runs with. * @@ -728,8 +751,15 @@ plugwright { * `guilds.command.*`. Staging the config here is what makes every test deterministic: * `plugwrightClean` wipes the run directory before each provision, so the plugin only ever sees * this config and a freshly generated tiers.yml/roles.yml. + * + * The staged files are written by one call, because the plugin's own file list is replaced rather + * than added to on every call. */ -fun LocalEnvironmentSpec.configureGuildsEnvironment(jvmArgs: List, essentialsX: String) { +fun LocalEnvironmentSpec.configureGuildsEnvironment( + jvmArgs: List, + essentialsX: String, + stagedFiles: List> = emptyList(), +) { acceptEula.set(true) this.jvmArgs.set(jvmArgs) @@ -748,6 +778,10 @@ fun LocalEnvironmentSpec.configureGuildsEnvironment(jvmArgs: List, essen file("plugins/Guilds/config.yml", file("src/test/e2e/server/plugins/Guilds/config.yml")) file("plugins/Guilds/tiers.yml", file("src/test/e2e/server/plugins/Guilds/tiers.yml")) file("plugins/Guilds/buffs.yml", file("src/test/e2e/server/plugins/Guilds/buffs.yml")) + + stagedFiles.forEach { (path, source) -> + file(path, source) + } } plugins { @@ -827,10 +861,10 @@ fun writeLegacyServerProperties(properties: File) { /** * Starts a Paper server and checks that Guilds came up on it. * - * This is what a version no bot can be driven on gets instead of the smoke spec. It cannot prove - * that the command layer works — that needs a player — but it does prove that the plugin loads, - * that its hard dependencies load with it, and that Guilds finds the economy and the permission - * provider it refuses to start without. + * This is what a version no bot can be driven on gets instead of the smoke spec. It cannot play + * the part of a player, but the console can ask the plugin about a guild, which is what the + * recovery fixtures need: a guild in the backup directory must be unknown to the plugin, and a + * guild in the data folder must be known to it. */ fun checkServerLoads(smoke: SmokeEnvironment, launcher: File, runDir: File) { val server = ProcessBuilder( @@ -880,6 +914,16 @@ fun checkServerLoads(smoke: SmokeEnvironment, launcher: File, runDir: File) { check(text.contains("Permissions Found:")) { "Guilds found no permission provider on Minecraft ${smoke.minecraftVersion}.\n$text" } + + /* + * The two staged guild files, asked about over the console. + * + * The one in the backup directory is a complete, valid guild record, so a plugin that read + * the backup directory as a data folder would know it. The one in the data folder is the + * same shape and has to be known, which is what shows the ordinary path still works. + */ + askConsole(server, log, "guild admin bank balance BackupSentinel", "That Guild does not exist!") + askConsole(server, log, "guild admin bank balance StoredSentinel", "has a bank balance of") } finally { server.outputStream.write("stop\n".toByteArray()) server.outputStream.flush() @@ -889,6 +933,31 @@ fun checkServerLoads(smoke: SmokeEnvironment, launcher: File, runDir: File) { } } +/** + * Runs a console command and waits for the answer to appear in the server's output. + * + * Only the output written after the command was sent counts, so an earlier answer to the same + * question cannot make this pass. + */ +fun askConsole(server: Process, log: StringBuilder, command: String, expected: String) { + val from = synchronized(log) { log.length } + + server.outputStream.write((command + "\n").toByteArray()) + server.outputStream.flush() + + val deadline = System.currentTimeMillis() + 30_000 + while (System.currentTimeMillis() < deadline) { + val answered = synchronized(log) { log.substring(from).contains(expected) } + if (answered || !server.isAlive) break + Thread.sleep(200) + } + + val text = synchronized(log) { log.toString() } + check(text.substring(from).contains(expected)) { + "The console command \"$command\" did not answer with \"$expected\".\n$text" + } +} + /* * The smoke environments need a Paper build of their own as well. * diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index fa1736c9a..ba6d28e51 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -6,7 +6,7 @@ how to run any of it. 141 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. -The runtime smoke suite (`tests/smoke.spec.ts`, 5 tests) is a separate, much smaller suite that +The runtime smoke suite (`tests/smoke.spec.ts`, 7 tests) is a separate, much smaller suite that runs on the older Minecraft versions the full suite cannot. It is listed under "Runtime smoke coverage" below. @@ -192,7 +192,7 @@ the server answering in another language for the rest of the run. ACF's `usePerI false)` is what makes that safe, and one player switching language leaves everyone else in English asserts it rather than assuming it. -## Runtime smoke coverage — `tests/smoke.spec.ts` (5) +## Runtime smoke coverage — `tests/smoke.spec.ts` (7) The full suite above is written against a modern server. The smoke suite is the part that has to hold on every version the plugin claims to support, so it is much smaller: the plugin and its @@ -206,11 +206,18 @@ money, a command answers, and a guild can be created. | The economy works | the economy moves money | | The command layer works | a guild command answers | | The plugin is usable | a guild can be created | +| A backup of unreadable data is not a record | a guild file in the backup directory is not loaded as a record | +| The ordinary storage path still works | a guild file in the data folder is loaded | The same spec runs on every environment, the modern ones included, so a smoke failure on 1.21.8 is as loud as one on 1.8.8. See `e2e-testing.md` for how the per-version environments are set up and what each one can and cannot prove. +The last two checks are the ones the recovery work needs. The build stages two guild files into +each smoke environment: one in the data folder, which the plugin is expected to load, and one in +the backup directory the recovery work writes to, which it must not. They are restricted to the +smoke environments, because a guild in the data folder would change what the full suite sees. + ## Fixture self-check — `tests/bank.spec.ts` `STARTING_BALANCE is what the fixtures actually fund` is not a Guilds test. It checks that the diff --git a/docs/e2e-testing.md b/docs/e2e-testing.md index c86b74b54..b624bddaa 100644 --- a/docs/e2e-testing.md +++ b/docs/e2e-testing.md @@ -101,8 +101,10 @@ instead: ``` It starts the server and asserts that Guilds and its hard dependencies enabled, that Guilds did -not disable itself, and that it found the economy and the permission provider. It cannot prove -that the command layer works, because that needs a player. +not disable itself, and that it found the economy and the permission provider. It also asks the +console about the two staged guild files, which is what stands in for a player there: the guild in +the backup directory has to be unknown to the plugin, and the guild in the data folder has to be +known to it. 26.1.2 is the same protocol as 26.1, which is the newest one minecraft-data knows, so the bot connects to that environment as 26.1. @@ -110,6 +112,26 @@ connects to that environment as 26.1. The legacy environments start the server through a shell script, and so does the 26.3 server check, so they do not run on Windows. +#### The staged guild files + +Two guild files are staged into each smoke environment's plugin data folder, both complete and +valid guild records: + +| File | What the plugin has to do with it | +| --- | --- | +| `plugins/Guilds/data/8d3f1a67-….json` | load it, the way it loads any guild in the data folder | +| `plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-….json` | leave it alone, because the backup directory is not a data folder | + +The first one is the ordinary path, so a check that it is loaded catches a storage change that +broke the reading of a good file. The second one is the recovery path: a plugin that treated the +backup directory as a data folder would load it as a guild, and the smoke suite asserts that it +does not. + +The local environment gets neither. A guild in its data folder would change what the full suite +sees, so the two checks that use these files are restricted to the smoke environments. A new +smoke environment has to be added to `SMOKE_ENVIRONMENTS` in `tests/smoke.spec.ts` as well, or +those two checks are skipped on it. + ### Running one file, or one test Plugwright filters by substring, through the generated config rather than a Gradle flag. The config diff --git a/src/test/e2e/server/plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json b/src/test/e2e/server/plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json new file mode 100644 index 000000000..09bd37325 --- /dev/null +++ b/src/test/e2e/server/plugins/Guilds/backups/corrupt/20240101-000000/guilds/1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3.json @@ -0,0 +1,42 @@ +{ + "id": "1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3", + "name": "BackupSentinel", + "prefix": "BKP", + "motd": "", + "guildMaster": { + "uuid": "1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3", + "role": { + "level": 0 + }, + "joinDate": 0, + "lastLogin": 0 + }, + "home": null, + "guildSkull": null, + "status": "Public", + "tier": { + "level": 1 + }, + "guildScore": { + "wins": 0, + "loses": 0 + }, + "balance": 0.0, + "members": [ + { + "uuid": "1f6b9c2e-5d84-4a37-9b6e-2c0a7f5d81e3", + "role": { + "level": 0 + }, + "joinDate": 0, + "lastLogin": 0 + } + ], + "invitedMembers": [], + "allies": [], + "pendingAllies": [], + "codes": [], + "vaults": [], + "lastDefended": 0, + "creationDate": 0 +} diff --git a/src/test/e2e/server/plugins/Guilds/data/8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json b/src/test/e2e/server/plugins/Guilds/data/8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json new file mode 100644 index 000000000..a61d0bcc5 --- /dev/null +++ b/src/test/e2e/server/plugins/Guilds/data/8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28.json @@ -0,0 +1,42 @@ +{ + "id": "8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28", + "name": "StoredSentinel", + "prefix": "STO", + "motd": "", + "guildMaster": { + "uuid": "8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28", + "role": { + "level": 0 + }, + "joinDate": 0, + "lastLogin": 0 + }, + "home": null, + "guildSkull": null, + "status": "Public", + "tier": { + "level": 1 + }, + "guildScore": { + "wins": 0, + "loses": 0 + }, + "balance": 0.0, + "members": [ + { + "uuid": "8d3f1a67-2b95-4c08-9e14-6a5d0c7b3f28", + "role": { + "level": 0 + }, + "joinDate": 0, + "lastLogin": 0 + } + ], + "invitedMembers": [], + "allies": [], + "pendingAllies": [], + "codes": [], + "vaults": [], + "lastDefended": 0, + "creationDate": 0 +} diff --git a/src/test/e2e/tests/smoke.spec.ts b/src/test/e2e/tests/smoke.spec.ts index cb3cf6153..79d602937 100644 --- a/src/test/e2e/tests/smoke.spec.ts +++ b/src/test/e2e/tests/smoke.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '@plugwright/runner'; import { MSG } from '../plugins/support/expected.js'; +import { guildList } from '../plugins/support/state.js'; import { stripColors } from '../plugins/support/text.js'; /** @@ -20,6 +21,27 @@ import { stripColors } from '../plugins/support/text.js'; /** The plugins Guilds refuses to start without, as they appear in the server log. */ const REQUIRED_PLUGINS = ['Vault', 'Essentials', 'LuckPerms'] as const; +/** + * The environments the recovery fixtures are staged into. + * + * The build stages them into the smoke environments only: a guild file in the data folder would + * change what the full suite on the local environment sees. A new smoke environment has to be + * added here as well, or the two checks below are skipped on it. + */ +const SMOKE_ENVIRONMENTS = [ + 'smoke-1.8.8', + 'smoke-1.12.2', + 'smoke-1.16.5', + 'smoke-1.21.8', + 'smoke-26.1.2', +] as const; + +/** The guild in the backup directory. The plugin must never load it. */ +const BACKUP_GUILD = 'BackupSentinel'; + +/** The guild in the data folder. The plugin is expected to load it. */ +const STORED_GUILD = 'StoredSentinel'; + test('Guilds and its dependencies are up', async ({ server }) => { const log = server.session.consoleLog.slice().join('\n'); @@ -41,6 +63,30 @@ test('Vault has both an economy and a permission provider', async ({ server }) = expect(vaults).not.toContain('Economy: null'); }); +test( + 'a guild file in the backup directory is not loaded as a record', + { environments: [...SMOKE_ENVIRONMENTS] }, + async ({ player }) => { + const names = (await guildList(player)).map((guild) => guild.name); + + // The file is a complete, valid guild record, so a plugin that read the backup directory + // as a data folder would have it in the list under this name. + expect(names).not.toContain(BACKUP_GUILD); + }, +); + +test( + 'a guild file in the data folder is loaded', + { environments: [...SMOKE_ENVIRONMENTS] }, + async ({ player }) => { + const names = (await guildList(player)).map((guild) => guild.name); + + // The counterpart of the check above: the ordinary path still works, so the recovery work + // did not stop the plugin from reading the folder it is supposed to read. + expect(names).toContain(STORED_GUILD); + }, +); + test('the economy moves money', async ({ player, guilds }) => { await guilds.fund(player, 2500); From 11a724bdf6f75b813cd0eea6acc77abf2e40be38 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 13:20:12 +0000 Subject: [PATCH 7/9] docs(e2e): carry the merged coverage totals The coverage inventory now counts 162 tests across 14 spec files, which is what the branches add up to. The section headings carry the merged numbers, and the section for the startup-loading spec is a placeholder: its table belongs to the pull request that adds the spec file. --- docs/e2e-coverage.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index ba6d28e51..006a51cd4 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -141 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run +162 tests across 14 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. The runtime smoke suite (`tests/smoke.spec.ts`, 7 tests) is a separate, much smaller suite that @@ -21,7 +21,7 @@ meaningful. | The economy and permission providers Vault needs are registered | Vault's `Economy` and `Permission` services resolve | | Guilds commands are reachable and its language is English | `/guild` answers, and does so in English | -## A. Guild lifecycle — `tests/lifecycle.spec.ts` (19) +## A. Guild lifecycle — `tests/lifecycle.spec.ts` (21) | Requirement | Test | | --- | --- | @@ -36,7 +36,7 @@ meaningful. | Status and information commands | toggles the guild between public and private / changes the guild prefix / sets, shows and clears the guild MOTD / reports guild membership through the guild list and rejects guild-scoped commands elsewhere | | Name blacklist | rejects a guild name that is a substring of a blacklisted word / accepts a guild name that contains a blacklisted word | -## B. Membership management — `tests/membership.spec.ts` (19) +## B. Membership management — `tests/membership.spec.ts` (21) | Requirement | Test | | --- | --- | @@ -55,7 +55,7 @@ meaningful. | Guild capacity | a full guild refuses another member | | Permissions after leaving or being removed | a player who leaves loses the permissions their guild granted them / a player who is kicked loses the permissions their guild granted them | -## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (7) +## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (8) | Requirement | Test | | --- | --- | @@ -132,7 +132,7 @@ meaningful. | Alliance chat between guilds | `chat.spec.ts`, ally chat reaches an allied guild and not the third party | | Cleanup on deletion | a deleted guild drops out of its allies | -## H. Invite codes — `tests/codes.spec.ts` (12, of which 5 are one `describe.serial` chain) +## H. Invite codes — `tests/codes.spec.ts` (18, of which 5 are one `describe.serial` chain) | Requirement | Test | | --- | --- | @@ -192,6 +192,11 @@ the server answering in another language for the rest of the run. ACF's `usePerI false)` is what makes that safe, and one player switching language leaves everyone else in English asserts it rather than assuming it. +## Startup loading — `tests/startup-loading.spec.ts` (4) + +The section belongs to the pull request that adds the spec file. Only the heading is here, so that +the counts in this document add up to the total above. + ## Runtime smoke coverage — `tests/smoke.spec.ts` (7) The full suite above is written against a modern server. The smoke suite is the part that has to From cc6352c16443c6c48d476304bf2be6a3c94b7435 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 14:20:09 +0000 Subject: [PATCH 8/9] test(e2e): say why the server disconnected a bot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bot the server kicked is reported as "the connection had already ended (socketClosed)", which says nothing about what the server objected to. The kick carries a reason — "Flying is not enabled on this server", for a bot a teleport left in mid-air — and the end event overwrites it with the fact that the socket closed. Keeping the two apart makes the next occurrence readable. --- src/test/e2e/plugins/support/state.ts | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/test/e2e/plugins/support/state.ts b/src/test/e2e/plugins/support/state.ts index 5518dbe56..9eae7aaf1 100644 --- a/src/test/e2e/plugins/support/state.ts +++ b/src/test/e2e/plugins/support/state.ts @@ -46,6 +46,9 @@ const allowance = new WeakMap(); /** Why a bot's connection ended, once it has. Keyed per bot. */ const disconnectedBecause = new WeakMap(); + +/** Why the server disconnected a bot, when it said. Keyed per bot. */ +const kickedBecause = new WeakMap(); const watched = new WeakSet(); /** @@ -64,17 +67,35 @@ function watchConnection(player: PlayerWrapper): void { on(event: string, handler: (...args: any[]) => void): void; }; bot.on('end', (reason: unknown) => disconnectedBecause.set(player, reasonText(reason))); - bot.on('kicked', (reason: unknown) => disconnectedBecause.set(player, reasonText(reason))); + bot.on('kicked', (reason: unknown) => kickedBecause.set(player, reasonText(reason))); bot.on('error', (reason: unknown) => disconnectedBecause.set(player, reasonText(reason))); } function reasonText(reason: unknown): string { if (reason == null) return 'the connection closed'; if (reason instanceof Error) return reason.message; - if (typeof reason === 'string') return reason; + if (typeof reason === 'string') return disconnectReasonText(reason); return JSON.stringify(reason); } +/** + * The text of a disconnect reason. + * + * A modern server sends its reason as a JSON document rather than as a plain string, so + * "Flying is not enabled on this server" arrives wrapped in one. Unwrapping it is the difference + * between a failure that says what the server objected to and one that says the socket closed. + */ +function disconnectReasonText(reason: string): string { + if (!reason.startsWith('{')) return reason; + + try { + const parsed = JSON.parse(reason) as { text?: string; description?: string }; + return parsed.text ?? parsed.description ?? reason; + } catch { + return reason; + } +} + /** * Sends one command, holding off just long enough that a burst of them cannot trip Paper's chat * throttle. Every helper in the fixture and every spec goes through here rather than @@ -88,7 +109,7 @@ function reasonText(reason: unknown): string { export async function run(player: PlayerWrapper, command: string): Promise { watchConnection(player); - const reason = disconnectedBecause.get(player); + const reason = kickedBecause.get(player) ?? disconnectedBecause.get(player); if (reason !== undefined) { throw new Error( `${player.username}'s connection had already ended (${reason}); ` + From c06f855cbf8eb55004033ee31aeec0973aa6d04d Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 14:37:38 +0000 Subject: [PATCH 9/9] test(e2e): let the E2E server allow flight Paper refuses a player whose client reports that it is not on the ground for 80 ticks, which is four seconds, unless the server allows flight. The homes spec teleports its bot into the air and then waits out a cooldown, and the server kicked the bot with "Flying is not enabled on this server" while it waited: the bot's socket was already closed when the command was about to be sent. The suite drives a throwaway server, so that server allows flight. Nothing in a test changes, and the plugin is not involved. --- build.gradle.kts | 93 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 31 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 081c0ac43..ec2d84095 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -831,31 +831,34 @@ fun smokeLauncherScript(javaPath: String, refusesNoGui: Boolean): String { } /** - * The server.properties the 1.8.8 environment runs with. + * The server.properties the E2E servers run with. * - * That server ships Netty 4.0, which decides whether it can use direct buffers by reading the - * address of a heap buffer, which no longer reads as zero on Java 9 and later. Without that support - * the native transport cannot work, and every connection the server accepts dies with "Unable to - * access address of buffer" — the bot never gets a reply and the smoke test fails on a socket - * error rather than on anything to do with Guilds. + * `allow-flight` is what keeps the server from kicking a bot that a teleport left in mid-air. + * Paper refuses a player whose client reports that it is not on the ground for 80 ticks, which is + * four seconds, unless the server allows flight: `MinecraftServer.isFlightAllowed()` is what + * makes Paper skip the check. The suite teleports bots into the air on purpose, so the throwaway + * server it drives allows it. Without this the server kicks the bot with "Flying is not enabled on + * this server" in the middle of a test that was only waiting for a cooldown to expire. * - * Turning the native transport off puts the server on the same NIO transport every Windows server - * uses, which works on a Java 11 JVM. + * The 1.8.8 server also needs the native transport turned off. That server ships Netty 4.0, + * which decides whether it can use direct buffers by reading the address of a heap buffer, and + * that address no longer reads as zero on Java 9 and later. Without the support the native + * transport cannot work, and every connection the server accepts dies with "Unable to access + * address of buffer" — the bot never gets a reply and the smoke test fails on a socket error + * rather than on anything to do with Guilds. Turning the native transport off puts the server on + * the same NIO transport every Windows server uses, which works on a Java 11 JVM. */ -fun writeLegacyServerProperties(properties: File) { - val line = "use-native-transport=false" +fun writeE2eServerProperties(properties: File, values: Map) { val existing = if (properties.exists()) properties.readText() else "" - val updated = if (existing.lines().any { it.trim().startsWith("use-native-transport=") }) { - existing.lines().joinToString("\n") { - if (it.trim().startsWith("use-native-transport=")) line else it - } - } else { - listOf(existing.trimEnd(), line).filter { it.isNotEmpty() }.joinToString("\n") + val kept = existing.lines().filter { line -> + values.keys.none { key -> line.trim().startsWith("$key=") } } properties.parentFile.mkdirs() - properties.writeText(updated + System.lineSeparator()) + properties.writeText( + (kept + values.map { (key, value) -> "$key=$value" }).joinToString("\n", postfix = "\n") + ) } /** @@ -1007,6 +1010,30 @@ fun downloadNewestPaperBuild(version: String, serverJar: File, versionMarker: Fi * its own afterEvaluate block, which runs first. */ afterEvaluate { + /* + * The local environment's server needs the same properties the smoke environments get, and it + * is the one the full suite runs against. The homes spec teleports its bot into the air and + * then waits out a cooldown, and Paper kicks a player that stays in the air without flight + * permission. + */ + val localRunDir = (plugwright.environments.get("local")!!.spec as LocalEnvironmentSpec) + .runDir.get().asFile + val localProperties = File(localRunDir, "server.properties") + val localServerProperties = tasks.register("e2eServerPropertiesLocal") { + outputs.file(localProperties) + + doLast { + writeE2eServerProperties(localProperties, mapOf("allow-flight" to "true")) + } + } + + tasks.named("plugwrightProvisionLocal") { + dependsOn(localServerProperties) + } + localServerProperties.configure { + mustRunAfter(tasks.named("plugwrightCleanLocal")) + } + val smokeServerChecks = mutableListOf>() val smokeTestTasks = smokeEnvironments.map { smoke -> val entry = plugwright.environments.get(smoke.name)!! @@ -1058,25 +1085,29 @@ afterEvaluate { val clean = tasks.named("plugwrightClean$suffix") /* - * The server properties the versions that need them run with. The provision task rewrites - * the keys it knows about and leaves the rest alone, so writing this first is enough. + * The server properties the environment runs with. The provision task rewrites the keys it + * knows about and leaves the rest alone, so writing these first is enough. */ val properties = File(runDir, "server.properties") - if (smoke.needsNioTransport) { - val serverProperties = tasks.register("smokeServerProperties$suffix") { - outputs.file(properties) + val serverProperties = tasks.register("smokeServerProperties$suffix") { + outputs.file(properties) - doLast { - writeLegacyServerProperties(properties) - } + doLast { + writeE2eServerProperties( + properties, + buildMap { + put("allow-flight", "true") + if (smoke.needsNioTransport) put("use-native-transport", "false") + } + ) } + } - tasks.named("plugwrightProvision$suffix") { - dependsOn(serverProperties) - } - serverProperties.configure { - mustRunAfter(clean) - } + tasks.named("plugwrightProvision$suffix") { + dependsOn(serverProperties) + } + serverProperties.configure { + mustRunAfter(clean) } tasks.named("plugwrightProvision$suffix") {