Release 1.6.2#397
Merged
Merged
Conversation
Both `importDatabaseFile` and `loadDownloadedChallenges` caught all exceptions, wrote a stacktrace to the console, and returned with no chat-visible feedback. The Poseidon zh-CN library payload incident showed why this hurts: admins saw the conversation's "successfully imported" message, then "no challenges available" with no clue where to look. Send a `challenges.errors.import-failed` message to the triggering player and include the exception's brief message, so they know to check the console instead of assuming the addon is broken. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI build for PR #397 erupted with 286 errors: `NoSuchElementException: No value for minecraft:acacia_hanging_sign` thrown from `org.bukkit.inventory.ItemType.<clinit>` inside `ItemStackMock.<clinit>`, which then poisoned `ItemStackMock` for the rest of the JVM. Root cause: ItemType's static initializer iterates Material and looks each entry up in the registry. If it runs before MockBukkit has populated the registry, the static init errors and every subsequent test that touches ItemStack fails with NoClassDefFoundError. AbstractChallengesTest already worked around this by touching `Tag.LEAVES` right after `MockBukkit.mock()`, but seven JUnit 5 panel tests didn't extend it and didn't carry the workaround. The flake didn't surface locally because filesystem ordering on macOS put a primed test first; ubuntu-latest's ordering didn't. Extract `PanelTestHelper.primeBukkitRegistry()` and call it right after `MockBukkit.mock()` in CommonPanelTest, CommonPagedPanelTest, ConversationUtilsTest, AdminPanelTest, ListChallengesPanelTest, GameModePanelTest, and MultiplePanelTest. The fix is order-independent — each affected test class now primes the registry itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot was right that my earlier `Tag.LEAVES`-in-PanelTestHelper patch
didn't actually fix anything — it just touched a Tag constant that was
already broken. The real flake is upstream: ChallengesAddonTest installs
`Mockito.mockStatic(Bukkit.class)` *without* MockBukkit, then runs code
during `addon.onLoad()` that triggers `org.bukkit.Tag.<clinit>`. With
Bukkit's static methods returning null stubs at that moment, every Tag
constant is permanently set to null for the JVM — there is no way to
re-initialize an interface's static fields once <clinit> has run.
Any test later in the suite that creates an ItemStack then walks:
ItemType.<clinit>
-> Registry.ITEM.getOrThrow("acacia_hanging_sign")
-> RegistryMock.loadIfEmpty -> ItemTypeMock.from(...)
-> Class.forName("BlockStateMetaMock") -> BlockStateMetaMock.<clinit>
-> MaterialTags.<clinit>
-> Objects.requireNonNull(Tag.ALL_SIGNS) // null -> NPE
The NPE wraps as ExceptionInInitializerError, RegistryMock catches and
swallows part of it, and `acacia_hanging_sign` is missing from the
registry forever after — hence the cascade of 286 errors on CI but a
clean local run, where the test order happens to put a properly-
MockBukkit-bracketed test first.
Reproduced locally with `mvn test -Dtest=ChallengesAddonTest,
CommonPagedPanelTest -Dsurefire.runOrder=alphabetical`.
Fix: install MockBukkit briefly at the very top of
ChallengesAddonTest.setUp() and prime Tag while it's active, then
unmock immediately. Tag's static constants are now real
MaterialTagMock instances and stay valid for the rest of the JVM,
regardless of what subsequent tests do with Bukkit. As a side effect,
this also clears up ChallengesAddonTest's own pre-existing
"Settings is null" failures when it runs early in the suite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The floating jitpack snapshot v1.21-SNAPSHOT resolves to an ephemeral git-described build whose jar/POM get evicted, breaking CI even on unchanged commits. Switch to the equivalent stable Maven Central coordinates (org.mockbukkit.mockbukkit:mockbukkit-v1.21:4.110.0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Patch release that surfaces import errors to admins instead of swallowing them silently.
Why: A recent incident with the translated Poseidon library entries had
descriptionstored as a string instead ofList<String>, which made Gson reject the payload with aJsonSyntaxException.ChallengesImportManagercaught it, wrote a stacktrace to the server console, and returned — leaving admins staring at "successfully imported" in chat and "no challenges available" right after, with no clue that anything went wrong or where to look. The weblink data has since been fixed (BentoBoxWorld/weblink#21), but the addon would happily go on hiding the next bad payload the same way.Fix: Both
importDatabaseFileandloadDownloadedChallengesnow sendchallenges.errors.import-failedto the triggering player after logging the stacktrace, including the exception's short message so they know to look at the console.Test plan
mvn clean verifypasses/[gm] admin challenges import <file>— chat shows the new red error message🤖 Generated with Claude Code