[CI test] Combined ports of CivMC PRs #916/#917/#918 - #2
Conversation
Holding an emerald previously cancelled right-click-block events on any interactable block, preventing chests from opening at all. Mirror vanilla food behavior instead: if the player is not sneaking, let the block interaction proceed normally; if sneaking, consume the emerald for XP as before.
Add a "Time remaining: N <unit>" line to pearl item lore directly beneath the existing health line. The interval count is computed from the configured decay rate, rounded up, and uses the same admin-configured unit string as the existing repair-cost lines, so operators control plurality and translation in one place. The line is hidden when the pearl is inactive (decay is already flagged separately as "suspended due to Inactivity"), when health has reached zero, or when decay is disabled in config. While here, hoist the shared decay-per-interval and unit values so the existing repair-cost loop reuses them instead of recomputing per material.
The test suite was effectively dead. useJUnitPlatform() was never configured in the parent plugins/build.gradle.kts, so every plugin's test task discovered zero tests and reported BUILD SUCCESSFUL even when src/test/ contained real JUnit 5 tests. The pinned JUnit (5.8.2) also fell out of alignment with the platform launcher Gradle ships, which would have failed discovery once useJUnitPlatform() was added anyway. And civmodcore-paper's existing ItemMetaTests crashed during class load because its static ItemStack(Material.STICK) field needed a Bukkit Material registry that no test ever bootstrapped. This change repairs the suite end to end: - Enable useJUnitPlatform() once at the parent so every plugin inherits it. Bump JUnit to 6.0.3 with the platform launcher pinned to the same version ref so the engine and launcher stay aligned. - Add MockBukkit to civmodcore-paper and bootstrap a ServerMock per test. Work around a paperweight quirk: the mojang-mapped server jar ships its own ServiceLoader providers for RegistryAccess, ServerBuildInfo and other Paper APIs, and Paper's loadAll() rejects duplicates. A small build-time task strips those entries from a copy of the mapped jar and substitutes it onto the test classpath, leaving NMS classes (which NBTTests needs) intact. - Disable testBaseComponent — its assertion presumed a distinction between legacy setDisplayName(String) and modern displayName( Component) that no longer holds in current Paper. Wire the result into CI: the existing Check All workflow now publishes a JUnit report and uploads the HTML test results on failure, so PR authors can see exactly what broke without re-running the build locally. The README gains a short Tests section pointing future contributors at where tests live, how to run them, and how the MockBukkit / paperweight workaround is set up. civmodcore-paper is now a working template anyone can copy when adding tests to other plugins.
There was a problem hiding this comment.
Code Review
This pull request introduces JUnit 5 testing infrastructure across several plugins, integrating MockBukkit for civmodcore-paper and adding comprehensive unit tests for exilepearl-paper. It also implements a "Time remaining" display in Exile Pearl lore and refactors interaction handling in OldEnchanting to allow sneaking players to interact with blocks. The reviewer identified a critical issue with the JUnit versioning scheme in libs.versions.toml where a non-existent version 6.0.3 was used, suggesting splitting the Jupiter and Platform versions. Additionally, a guard clause improvement was suggested for PearlDecayMath to handle non-positive configuration values robustly.
| [versions] | ||
| paper = "1.21.8-R0.1-SNAPSHOT" | ||
| junit = "5.8.2" | ||
| junit = "6.0.3" |
There was a problem hiding this comment.
JUnit 5 (Jupiter) and JUnit Platform use different versioning schemes (e.g., Jupiter 5.10.3 and Platform 1.10.3). There is no version 6.0.3 for either, and using a single version reference for both will cause dependency resolution to fail. We should split them into separate version properties.
| junit = "6.0.3" | |
| junit-jupiter = "5.10.3" | |
| junit-platform = "1.10.3" |
| junit-api = { group = "org.junit.jupiter", name = "junit-jupiter-api", version.ref = "junit" } | ||
| junit-engine = { group = "org.junit.jupiter", name = "junit-jupiter-engine", version.ref = "junit" } | ||
| junit-platform-launcher = { group = "org.junit.platform", name = "junit-platform-launcher", version.ref = "junit" } | ||
| mockbukkit = { group = "org.mockbukkit.mockbukkit", name = "mockbukkit-v1.21", version = "4.93.0" } |
There was a problem hiding this comment.
Update the JUnit dependency definitions to use the split version references (junit-jupiter and junit-platform) to avoid resolution errors.
| junit-api = { group = "org.junit.jupiter", name = "junit-jupiter-api", version.ref = "junit" } | |
| junit-engine = { group = "org.junit.jupiter", name = "junit-jupiter-engine", version.ref = "junit" } | |
| junit-platform-launcher = { group = "org.junit.platform", name = "junit-platform-launcher", version.ref = "junit" } | |
| mockbukkit = { group = "org.mockbukkit.mockbukkit", name = "mockbukkit-v1.21", version = "4.93.0" } | |
| junit-api = { group = "org.junit.jupiter", name = "junit-jupiter-api", version.ref = "junit-jupiter" } | |
| junit-engine = { group = "org.junit.jupiter", name = "junit-jupiter-engine", version.ref = "junit-jupiter" } | |
| junit-platform-launcher = { group = "org.junit.platform", name = "junit-platform-launcher", version.ref = "junit-platform" } | |
| mockbukkit = { group = "org.mockbukkit.mockbukkit", name = "mockbukkit-v1.21", version = "4.93.0" } |
| static int decayPerHumanInterval(int humanIntervalMin, int decayIntervalMin, int decayAmount) { | ||
| if (decayIntervalMin <= 0) { | ||
| return 0; | ||
| } | ||
| return (humanIntervalMin / decayIntervalMin) * decayAmount; | ||
| } |
There was a problem hiding this comment.
To prevent unexpected behavior or negative values from misconfiguration, we should also guard against non-positive values for humanIntervalMin and decayAmount.
| static int decayPerHumanInterval(int humanIntervalMin, int decayIntervalMin, int decayAmount) { | |
| if (decayIntervalMin <= 0) { | |
| return 0; | |
| } | |
| return (humanIntervalMin / decayIntervalMin) * decayAmount; | |
| } | |
| static int decayPerHumanInterval(int humanIntervalMin, int decayIntervalMin, int decayAmount) { | |
| if (decayIntervalMin <= 0 || humanIntervalMin <= 0 || decayAmount <= 0) { | |
| return 0; | |
| } | |
| return (humanIntervalMin / decayIntervalMin) * decayAmount; | |
| } |
This PR exercises the Gradle Check All workflow on the combined set of three ports:
Base:
eden-main(snapshot of EdenMinecraft/Civ's main).Head:
eden-test-combined(the three branches merged together).Local
./gradlew checkis green; this PR exists so CI runs the same suite.