Skip to content

[CI test] Combined ports of CivMC PRs #916/#917/#918 - #2

Closed
grepsedawk wants to merge 5 commits into
eden-mainfrom
eden-test-combined
Closed

[CI test] Combined ports of CivMC PRs #916/#917/#918#2
grepsedawk wants to merge 5 commits into
eden-mainfrom
eden-test-combined

Conversation

@grepsedawk

Copy link
Copy Markdown
Owner

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 check is green; this PR exists so CI runs the same suite.

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread gradle/libs.versions.toml
[versions]
paper = "1.21.8-R0.1-SNAPSHOT"
junit = "5.8.2"
junit = "6.0.3"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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.

Suggested change
junit = "6.0.3"
junit-jupiter = "5.10.3"
junit-platform = "1.10.3"

Comment thread gradle/libs.versions.toml
Comment on lines 46 to +49
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" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Update the JUnit dependency definitions to use the split version references (junit-jupiter and junit-platform) to avoid resolution errors.

Suggested change
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" }

Comment on lines +7 to +12
static int decayPerHumanInterval(int humanIntervalMin, int decayIntervalMin, int decayAmount) {
if (decayIntervalMin <= 0) {
return 0;
}
return (humanIntervalMin / decayIntervalMin) * decayAmount;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To prevent unexpected behavior or negative values from misconfiguration, we should also guard against non-positive values for humanIntervalMin and decayAmount.

Suggested change
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;
}

@grepsedawk grepsedawk closed this Jun 1, 2026
Sign up for free to 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.

1 participant