Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 by Drownek · Pull Request #430 · BentoBoxWorld/Challenges · GitHub
Skip to content

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21 - #430

Merged
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build
Aug 13, 2026
Merged

Automate Challenges jar compilation for Plugwright E2E tests and enforce Java 21#430
tastybento merged 7 commits into
BentoBoxWorld:developfrom
Drownek:refactor/e2e-build

Conversation

@Drownek

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Automates JAR building for E2E tests: Replaces the need for developers to manually run mvnw package before running plugwrightTest. The newly introduced buildChallenges Gradle task automatically runs the Maven wrapper to build the Challenges.jar dynamically as a test dependency.
  • Java Toolchains Integration: Replaces manual Java Toolchain resolution with a proper, global java { toolchain { ... } } setup. Plugwright gracefully inherits this out-of-the-box, removing boilerplate code.
  • Enforces Java 21+: Ensures the Maven build explicitly requires Java 21+ (via Maven Enforcer), failing early with a clear message if an older JDK is detected in the environment.
  • Cleans up build.gradle.kts: Leverages idiomatic Gradle fileTree APIs for resolving the output .jar instead of verbose manual file traversal, making the build script significantly more readable.

How to test

Simply run ./gradlew plugwrightTest (or gradlew.bat plugwrightTest on Windows) inside the e2e directory. The project will seamlessly compile the Maven artifact and boot the Paper server for E2E testing without any manual prerequisites.

Previously, developers had to manually run 'mvnw package' before running e2e tests. This commit adds a 'buildChallenges' Gradle task that runs the Maven wrapper automatically, resolving the Java toolchain and finding the compiled jar without manual intervention.
@mrfloris

Copy link
Copy Markdown

build.gradle.kts executes ./mvnw, but the PR adds mvnw with Git mode 100644, not 100755. E2E testing will fail with Permission denied on Unix systems and GitHub’s Ubuntu runner.

buildChallenges declares build/Challenges.jar as an output but declares no source, resource, or pom.xml inputs. Once that JAR exists, Gradle may mark the task UP-TO-DATE even after plugin code changes. This undermines the entire purpose of the feature. Declare the Maven project files as inputs or deliberately make the task always run.

The workflow still explicitly runs Maven, then plugwrightTest invokes buildChallenges, which runs Maven again: e2e.yml.

Existing target JARs can break local testing. fileTree(...).singleFile fails if target/ contains multiple matching JARs, which can happen after changing the project version without running clean.

The Maven enforcer accepts Java 21 or newer, but the Gradle toolchain requests exactly Java 21. There is no toolchain download resolver configured, so developers who only have Java 25/26 can receive “no matching Java installation” despite satisfying the stated requirement.

Verdict though; sensible feature, and i dont see anything dodgy with it. So once a human dev has made some logical changes i think this could be considered by tasty.

- Fix mvnw execution permissions for CI/Unix
- Declare task inputs for buildChallenges to fix caching
- Use clean package in Maven to prevent singleFile crashing on stale artifacts
- Resolve JavaToolchain dynamically only if Gradle runs on < Java 21, allowing devs on Java 22+ to build out of the box
- Remove redundant Maven run from e2e GitHub Actions workflow
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review, good catches. Pushed a fix: mvnw is executable again, the Gradle task has its inputs declared, and clean is back in the Maven command.

On the Java version conflict: I changed the Gradle toolchain logic so it only requests Java 21 if the daemon's on <21. So devs on 17 still get it working out of the box, and reviewers on 22+ don't hit a strict lock. Also cut the redundant Maven step in e2e.yml.

Tested on 22 and 17, both fine.

@tastybentotastybento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and thanks @mrfloris for the first pass — those were good catches.

Context for anyone reading: I'm using Challenges as a vehicle to test plugwright. That's still a new approach and I'm waiting to see how it gets maintained, so I'm deliberately conservative about how much of the repo gets restructured around it. But the direction here is sensible and I'd like to take it.

I checked the branch out and actually ran it on macOS (Apple Silicon, Homebrew JDKs) rather than just reading the diff. Confirming the fixes hold up:

CheckResult
mvnw file mode100755 — fixed
mvnw contentbyte-identical to upstream maven-wrapper-distribution-3.3.2
mvnw.cmd contentidentical to upstream modulo line endings (LF — but e2e/gradlew.bat is already LF in this repo, so pre-existing, not yours)
.mvn/wrapper/maven-wrapper.jarbyte-identical to the official maven-wrapper-3.3.2.jar on Maven Central (sha256 3d8f20ce…39c7a8)
./gradlew buildChallenges on JDK 21builds, copies the right jar to e2e/build/Challenges.jar
same from a JDK 17 daemonresolved a local Java 21 toolchain and built
re-run with no changeUP-TO-DATE
edit a .java, re-runrebuilds
edit locales/en-US.yml, re-runrebuilds
.singleFile vs original-Challenges-*.jar and -sources.jar in target/correctly picks the shaded jar

So the exec bit and the bogus up-to-date checking are genuinely fixed, not just claimed fixed. The redundant Maven step is gone from e2e.yml. The enforcer is harmless — every workflow already runs JDK 21 and <release>21</release> already fails on older JDKs, so it's purely a nicer error message, which is a fine small win.

Two things I'd like before I merge, both inline below:

  1. Regenerate the wrapper scripts-only, to drop the committed binary.
  2. Lose the clean — it's doing a lot of work just to keep .singleFile happy.

The rest of the inline comments are optional polish, take them or leave them.


Unrelated pre-existing issue I found while testing, flagging so nobody chases it as a regression here:./gradlew in e2e/ won't start at all on a machine whose default JDK is 25 — Gradle 8.10 doesn't understand that version and you get a bare * What went wrong: 25.0.1 with no further explanation. This reproduces on unmodified develop, so it is not caused by this PR. @mrfloris I suspect that's the macOS problem you hit. Bumping e2e/gradle/wrapper/gradle-wrapper.properties to Gradle 9.x fixes it — happy to take that as a separate follow-up.

I wasn't able to complete a full plugwrightTest run locally: port 25565 was occupied by another server on my machine, so Paper couldn't bind. That's my environment, not the PR.

@@ -0,0 +1,2 @@
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you regenerate this scripts-only?

mvn wrapper:wrapper -Dtype=only-script -Dmaven=3.9.12

That drops .mvn/wrapper/maven-wrapper.jar entirely — mvnw becomes a self-contained script that fetches the distribution itself. Same behaviour, but I'd rather not carry a 63KB binary in the repo that has to be re-vetted by hand every time it's bumped. (I checked this one and it's clean — byte-identical to Maven Central — but that's a check someone has to remember to repeat.)

It also gets us off 3.9.6, which is from January 2024. 3.9.12 is what I have locally.

While you're in here: distributionSha256Sum is worth setting too, so the downloaded Maven distribution is verified rather than trusted.

Comment threade2e/build.gradle.kts Outdated
val isWindows = System.getProperty("os.name").lowercase().contains("win")
val executable = if (isWindows) listOf("cmd", "/c", "mvnw.cmd") else listOf("./mvnw")
// Use 'clean' to avoid multiple jars causing singleFile to fail
commandLine(executable + listOf("-q", "clean", "package", "-DskipTests"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clean is here only to stop .singleFile below choking on stale jars, and it's an expensive way to buy that. Every source change now forces a full recompile of the whole addon, and it wipes target/ — including surefire-reports and any jar someone has built to drop on a live server for manual testing.

Making the resolution robust instead means you don't need clean at all — see my comment on the doLast block.

Comment threade2e/build.gradle.kts Outdated
val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.singleFile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.singleFile is the thing forcing clean up on line 56. Picking the newest match is just as correct after a package and removes that constraint:

val builtJar = fileTree("../target") {
include("Challenges-*.jar")
exclude("*sources*", "*javadoc*")
}.files.maxByOrNull { it.lastModified() }
?:throwGradleException("No Challenges jar produced in ../target")

With that, line 56 can go back to -q package -DskipTests and the dev loop stays incremental.

(For what it's worth, I verified the current filter does correctly skip original-Challenges-*.jar and -sources.jaroriginal-… doesn't match the Challenges-* include. So this is about the clean, not about the filter being wrong.)

Comment threade2e/build.gradle.kts
workingDir = file("..")

inputs.dir(file("../src"))
inputs.file(file("../pom.xml"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor completeness point: .mvn/, mvnw and mvnw.cmd aren't inputs, so bumping the Maven version wouldn't retrigger the build. Cheap to add:

inputs.dir(file("../.mvn"))
inputs.file(file("../mvnw"))

Not a blocker — the src + pom.xml inputs are the ones that matter day to day, and I confirmed those work.

Comment threade2e/build.gradle.kts

// Pass the correct JAVA_HOME to Maven if we needed a custom toolchain
if (javaLauncherProvider != null) {
environment["JAVA_HOME"] = javaLauncherProvider.get().metadata.installationPath.asFile.absolutePath

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: javaLauncherProvider.get() here resolves the toolchain at configuration time, which forces the lookup (and potentially a provision) whenever the task is realised, even on a no-op run. Moving it into doFirst { environment("JAVA_HOME", …) } keeps it lazy and is friendlier to the configuration cache if this build ever turns that on.

Separately, the else branch overriding JAVA_HOME with java.home is a no-op in the normal case (Exec inherits the environment) but does override a JAVA_HOME the developer set deliberately when the daemon JVM was chosen via org.gradle.java.home. Narrow edge case, just noting it.

Comment threade2e/build.gradle.kts
// If Gradle is running on Java 17, try to find a Java 21+ toolchain to satisfy Maven/Paper.
// If Gradle is already running on Java 21+ (e.g., 22, 23), don't force a strict toolchain lock.
val currentJava = JavaVersion.current()
val javaLauncherProvider = if (currentJava < JavaVersion.VERSION_21) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrows @mrfloris's toolchain point nicely, but doesn't fully close it: a developer on Java 17 with no JDK 21 installed still gets "no matching toolchains found" and no way to recover automatically. Gradle can provision one if you add the foojay resolver to e2e/settings.gradle.kts:

plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}

Optional, but it's one line and it makes the "just run ./gradlew plugwrightTest" promise in the PR description actually true on a clean machine.

(On my Mac Gradle did find the Homebrew JDK 21 from a 17 daemon, so this only bites people who genuinely don't have a 21 anywhere.)

Comment threadpom.xml Outdated
<configuration>
<rules>
<requireJavaVersion>
<version>[21,)</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing: [21,) duplicates <java.version>21</java.version> up at line 42, so this silently goes stale when the project bumps. [${java.version},) keeps them in sync.

…JDKs
Gradle 8.10 cannot start on a JDK it does not recognise. On a machine
whose default JDK is 25, every ./gradlew invocation in e2e/ died before
evaluating the build script with nothing but:
* What went wrong:
25.0.1
This pre-dates the rest of this PR (it reproduces on develop unmodified),
but it defeats the "just run ./gradlew plugwrightTest" workflow this PR is
adding, so fix it here rather than leave the new entry point broken for
anyone not pinned to an older JDK.
- Wrapper regenerated with the documented two-pass `wrapper` task run, so
gradle-wrapper.jar is the real 9.7.0 one. Its SHA-256 is
7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d,
matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
- distributionSha256Sum pinned so the downloaded distribution is verified
rather than trusted.
- buildChallenges switched from `by tasks.registering(Exec::class)` to
`tasks.register<Exec>(...)`. Gradle 9.6 deprecated the delegate form;
without this the bump emits three deprecation warnings and flags the
build as incompatible with Gradle 10.
Verified on macOS/arm64 with JDK 25 as the default: ./gradlew
buildChallenges succeeds with no deprecation warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyrUvHMduHzV4hFj84PExn
@tastybento

Copy link
Copy Markdown
Member

Pushed a commit to this branch directly (be00869) rather than leaving the Gradle thing as a vague follow-up — it was blocking the very workflow this PR adds, so it belongs here.

What it does: bumps the e2e/ Gradle wrapper 8.10 → 9.7.0. Gradle 8.10 can't start on a JDK it doesn't recognise, so on any machine whose default JDK is 25, every ./gradlew in e2e/ died before even evaluating the build script, with nothing but:

* What went wrong:
25.0.1

To be clear @Drownek, this was not your bug — it reproduces on unmodified develop. But "just run ./gradlew plugwrightTest" is the promise of this PR, and it wasn't true for anyone on a current JDK. @mrfloris I'm fairly confident this is the macOS problem you ran into.

Three parts:

  • Wrapper regenerated using the documented two-passwrapper run, so gradle-wrapper.jar is genuinely the 9.7.0 one. (First pass under 8.10 only rewrites the properties and leaves an 8.10 jar behind — easy trap.) Its SHA-256 is 7a9ce74cff467ca1bf60a4fcd9f05185acceda4d0f382434d393e17864262c5d, matching the checksum Gradle publishes for gradle-9.7.0-wrapper.jar.
  • distributionSha256Sum pinned, so the downloaded distribution is verified rather than trusted. Same reasoning as my comment about distributionSha256Sum on the Maven wrapper.
  • buildChallenges moved from by tasks.registering(Exec::class) to tasks.register<Exec>("buildChallenges"). Gradle 9.6 deprecated the delegate form; without this the bump emits three deprecation warnings and marks the build incompatible with Gradle 10. Only reason I touched your file.

Verified on macOS/arm64, JDK 25 as the default, full suite:

 Total: 4
Passed: 4
Failed: 0
PASS bot can interact with the server
PASS confirmation prompts tell the player how to answer (#329)
PASS open-anywhere setting toggles in the admin settings GUI (#349)
PASS include-undeployed setting toggles in the admin settings GUI (#179)
BUILD SUCCESSFUL

That's the whole chain — buildChallenges → jar deploy → Paper boot → Mineflayer bots — so plugwright 2.0.2 is fine on Gradle 9, no changes needed on your side for it.

Sorry for pushing to your branch without asking first; shout if you'd rather I'd left it separate and I'll happily pull it back out.

That leaves just the two asks from my review: the scripts-only Maven wrapper, and dropping clean in favour of picking the newest jar. Do those and I'm happy to merge.

- Switch Maven wrapper to script-only type, dropping the committed binary jar
- Remove clean from buildChallenges Maven args as requested
- Pick newest built jar dynamically to avoid singleFile conflicts
@Drownek

Copy link
Copy Markdown
ContributorAuthor

Thanks @tastybento for testing this out on macOS

I've pushed a new commit addressing both points:

  1. Regenerated the Maven wrapper as script-only (distributionType=only-script) and removed the binary .mvn/wrapper/maven-wrapper.jar.
  2. Dropped clean from the buildChallenges Maven command and updated the task to dynamically pick the newest JAR from target/ by its modification timestamp

Should be good to merge

Finishes the scripts-only wrapper migration:
- Bump the wrapper distribution 3.9.6 (Jan 2024) -> 3.9.12 and set
distributionSha256Sum. With the wrapper jar gone, mvnw fetches a 9 MB
Maven distribution at build time, so without a checksum there was no
committed artifact *and* no verification. 3.9.12 also ships newer
jansi/guava, which silences the sun.misc.Unsafe restricted-method
warnings 3.9.6 emits on every build under JDK 25.
- Add ../.mvn and ../mvnw as buildChallenges inputs. The wrapper pins the
Maven version, so bumping it has to retrigger the build - without this
the commit above would not have rebuilt anything.
- Use [${java.version},) in the enforcer rule so it tracks the property at
the top of the pom instead of going stale. Verified that POM
interpolation resolves this to the pom property, not the JVM's
java.version system property.
- Drop a stray blank line left by removing the Maven step from e2e.yml.
Verified on macOS/arm64: mvn test 522/522 on both JDK 21 and 25, and the
full plugwrightTest suite 4/4 green from a clean target/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SBMBsT7vUtgBXUk3gtajgt
@tastybento
tastybento merged commit e3fdf1e into BentoBoxWorld:developAug 13, 2026
0 of 2 checks passed
@tastybento

Copy link
Copy Markdown
Member

Merged — thanks @Drownek, this is a genuinely nice quality-of-life win. ./gradlew plugwrightTest now does the whole thing from a clean checkout, which is exactly what I wanted out of it. And thanks again @mrfloris for the first pass; the exec-bit and up-to-date-checking catches were the two that mattered most.

I verified the last round on macOS/arm64 with JDK 25 as the default before merging:

CheckResult
mvnw vs upstream maven-wrapper-distribution-3.3.4-only-scriptbyte-identical
mvnw.cmdidentical modulo LF (matches gradlew.bat, pre-existing)
Version bump leaving both 1.8.0 and 1.9.0 jars in target/picks 1.9.0 — confirmed via addon.yml inside the deployed jar
Dev loop, clean gone17s cold → 6s warm
plugwrightTest, clean target/4/4 green
mvn test, JDK 21 and 25522/522

Nice detail worth recording: the sources jar gets an mtime identical to the second to the shaded jar, so maxByOrNull { lastModified } on its own would be a coin flip between them. The exclude("*sources*") is load-bearing, not decorative — worth not tidying away later.

I pushed one more commit (64a98f4) rather than sending you round again for small stuff:

  • Bumped the distribution 3.9.6 → 3.9.12 and set distributionSha256Sum. This was the half of my wrapper comment that got missed, and it mattered more after the only-script switch than before: with the jar gone, mvnw fetches a 9 MB Maven distribution at build time, so there was no longer a committed artifact and no verification of the thing replacing it. 3.9.12 also ships newer jansi/guava, which silences the eight lines of sun.misc.Unsafe warnings 3.9.6 emits on every build under JDK 25.
  • Added ../.mvn and ../mvnw as task inputs. Directly coupled to the above — without them the version bump would not have retriggered a build. Verified it does now.
  • [${java.version},) in the enforcer. I nearly talked myself out of my own suggestion here: mvn help:evaluate -Dexpression=java.version returns the JVM system property (25.0.1), which would have inverted the rule. But POM plugin-configuration interpolation resolves it differently — I set <java.version> to 99 and the enforcer duly rejected JDK 25, so the pom property is what wins. Safe, and it now tracks line 42 instead of going stale.
  • Dropped a stray blank line left behind by removing the Maven step from e2e.yml.

Left as follow-ups, no action needed from you: the foojay resolver and the lazy JAVA_HOME in doFirst. Both are design calls rather than fixes and I would rather make them separately.

Two things for anyone reading the red check rather than the logs:

  1. The Build failure is the fork/SONAR_TOKEN limitation, not this PR. Secrets are not exposed to fork PRs, so the scanner cannot authenticate. Same on the dependabot PR, and Improve Traditional Chinese (zh-TW) translation #427/Update zh-TW.yml: complete translation coverage for v1.8.0 #429 merged through it.
  2. TryToCompleteTest.testRewardChance0NoItems failed once and then passed on a rerun of the identical commit (522/522). Pre-existing intermittent flake, unrelated to build config. I had a quick look: with rewardChance 0, shouldRewardItems() is deterministically false, and every reward path is gated by it — except the level-completion rewards at TryToComplete.java:429, which call addItem ungated whenever tryCompleteLevel returns a level. So a leaked stub on that mock would produce exactly this failure. Whether level rewards should bypass the reward-chance gate is a separate question worth answering. I will raise an issue; nothing for you to do here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Drownek@mrfloris@tastybento