[Tests] Improve coverage for ItemPluginType, CustomBlockWrapper, CustomItemWrapper, CustomEntityWrapper

[DiamondDagger590]

Summary

• New test class: ItemPluginTypeGetCustomItemTest — 15 tests covering all three enum constants' getCustomItem() lambdas (NEXO, ITEMS_ADDER, NONE), including hook-present, hook-absent, vanilla fallback, and invalid-name-to-stone fallback paths
• Enhanced CustomBlockWrapperTest — 16 new tests across 7 nested classes: blockName vanilla material path, itemBuilder (material/custom with hook/custom without hook), equals(Block), Block constructor, static customModels, drops, and removeBlock
• Enhanced CustomItemWrapperTest — 10 new tests across 4 nested classes: itemName vanilla material path, itemBuilder, ItemStack constructor, and static customModels
• Enhanced CustomEntityWrapperTest — 10 new tests across 4 nested classes: entityName vanilla entity type path, equals(Entity), Entity constructor, and static customModels

Test plan

• All new and existing tests pass (gradle test — BUILD SUCCESSFUL)
• No production code changes — test-only diff
• Testing audit persona reviewed with no blocking concerns
• MockBukkit + CorePluginTestHelper setup added where needed for ItemBuilder/CorePlugin.getInstance() dependencies

🤖 Generated with Claude Code

https://claude.ai/code/session_01XBQ6c6i9tMccciPqVZcSiS

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

@DiamondDagger590, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 28d7adbc-8f2d-4002-a6f6-637613fea993

📥 Commits

Reviewing files that changed from the base of the PR and between ca6b94b and bbdf09b.

📒 Files selected for processing (4)

• src/test/java/com/diamonddagger590/mccore/builder/item/ItemPluginTypeGetCustomItemTest.java
• src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java
• src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java
• src/test/java/com/diamonddagger590/mccore/util/item/CustomItemWrapperTest.java

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/eager-thompson-u3tsyc

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Testing Review

I'll systematically apply every checklist item to the diff.

---

Analysis

Production files changed

The diff contains only test file changes — no production source files are modified. The tests exercise existing production classes: ItemPluginType, CustomBlockWrapper, CustomItemWrapper, CustomEntityWrapper.

---

Checklist Application

Coverage Completeness

The tests are net-new additions for existing production methods. Coverage is substantive. However, several gaps exist.

---

CONCERN: itemBuilder tests in CustomBlockWrapperTest and CustomItemWrapperTest assert only assertNotNull(builder) — they never verify what material or item the builder was constructed with.
WHY: A builder that wraps AIR and a builder that wraps EMERALD both pass assertNotNull. The assertion cannot distinguish correct behavior from incorrect behavior. The test for "hook provides item → returns hook item builder" is indistinguishable from "hook does not provide item → returns AIR builder." These tests cannot fail on a regression in itemBuilder logic.
WHERE: CustomBlockWrapperTest#ItemBuilderTests, CustomItemWrapperTest#ItemBuilderTests

---

CONCERN: itemBuilder_returnsAirBuilder_whenNoHooksRegistered and itemBuilder_returnsAirBuilder_whenHookDoesNotProvideItem do not assert that the resulting builder actually produces an AIR item.
WHY: The display name claims an AIR fallback is returned, but no assertion verifies the material. The test name sets an expectation that the code never has to satisfy.
WHERE: CustomBlockWrapperTest#itemBuilder_returnsAirBuilder_whenNoHooksRegistered, CustomBlockWrapperTest#itemBuilder_returnsAirBuilder_whenHookDoesNotProvideItem, CustomItemWrapperTest#itemBuilder_returnsAirBuilder_whenNoHooksRegistered, CustomItemWrapperTest#itemBuilder_returnsAirBuilder_whenHookDoesNotProvideItem

---

CONCERN: itemBuilder_returnsHookItemBuilder_whenHookProvidesItem does not assert the built item's material matches what the hook returns.
WHY: The test registers a hook that returns DIAMOND_SWORD (item) or EMERALD (block), but only asserts assertNotNull(builder). A broken itemBuilder that always returns a builder for AIR would still pass this test.
WHERE: CustomBlockWrapperTest#itemBuilder_returnsHookItemBuilder_whenHookProvidesItem, CustomItemWrapperTest#itemBuilder_returnsHookItemBuilder_whenHookProvidesItem

---

CONCERN: customModels_returnsEmptySet_whenNoHooksRegistered in CustomBlockWrapperTest and CustomEntityWrapperTest passes a Mockito.mock(Block.class) / Mockito.mock(Entity.class) without stubbing getType(), yet these tests call MockBukkit in setUp. If the production customModels method touches the block's type at all on this path, this will throw a NPE/stub error rather than produce a meaningful assertion failure.
WHY: Incomplete stub setup means the test is brittle and may pass vacuously if production code short-circuits before calling getType(). No assertion verifies that the empty set is the correct empty set from an identifiable code path.
WHERE: CustomBlockWrapperTest#CustomModels#customModels_returnsEmptySet_whenNoHooksRegistered, CustomEntityWrapperTest#CustomModelsTests#customModels_returnsEmptySet_whenNoHooksRegistered

---

CONCERN: drops in CustomBlockWrapperTest only has a single test: "hook registered → returns hook drops." There is no test for the no-hook path (empty drops), and no test for entityBreaking being non-null.
WHY: The no-hook path is an edge case that should be tested. The null entity parameter is hardcoded in the sole test, but the non-null case is never exercised. This is a coverage gap in a method that has branching logic.
WHERE: CustomBlockWrapperTest#Drops

---

CONCERN: ItemPluginType.NONE.getCustomItem with both hooks present — the test registers both a Nexo hook and an ItemsAdder hook but only tests the case where neither recognizes the item. There is no test for the ordering/priority: what happens when both hooks claim to recognize the same item?
WHY: If NONE iterates hooks in registration order and both isItem() return true, the behavior of which hook wins is untested. This is a behavioral edge case relevant to NONE's auto-detection logic.
WHERE: ItemPluginTypeGetCustomItemTest#NoneGetCustomItem

---

CONCERN: ItemPluginTypeGetCustomItemTest does not test getCustomItem(null) for any ItemPluginType variant.
WHY: The checklist requires null-input edge cases for non-trivial public methods. getCustomItem takes a String parameter; if the production lambda calls .equals() or Material.matchMaterial() on a null input, it will throw a NullPointerException. No test guards against this regression.
WHERE: ItemPluginTypeGetCustomItemTest (all nested classes)

---

CONCERN: CustomEntityWrapperTest now uses Mockito.mock(Entity.class) in several new tests (e.g., EqualsEntity, EntityConstructor, CustomModelsTests). MockBukkit is initialized in setUp / torn down in tearDown, but Entity is a Bukkit interface for which MockBukkit provides EntityMock. Using Mockito.mock(Entity.class) where MockBukkit's EntityMock exists violates the checklist rule against mocking Bukkit classes when a real MockBukkit implementation is available.
WHY: EntityMock provides a real, stateful implementation. Using Mockito.mock(Entity.class) bypasses MockBukkit's lifecycle management and can produce inconsistent behavior with server-side entity tracking. The test also spins up MockBukkit (MockBukkit.mock()) but then uses Mockito for Bukkit types — defeating the purpose of MockBukkit.
WHERE: CustomEntityWrapperTest#EqualsEntity, CustomEntityWrapperTest#EntityConstructor, CustomEntityWrapperTest#CustomModelsTests

---

CONCERN: CustomBlockWrapperTest uses Mockito.mock(Block.class) in EqualsBlock, BlockConstructor, CustomModels, Drops, and RemoveBlock. Block is a Bukkit interface; MockBukkit provides real world/block implementations via ServerMock. MockBukkit is initialized in setUp.
WHY: Same rule violation as above — MockBukkit is loaded but Mockito is used for Bukkit types where MockBukkit provides real implementations. This is specifically flagged in the checklist.
WHERE: CustomBlockWrapperTest#EqualsBlock, CustomBlockWrapperTest#BlockConstructor, CustomBlockWrapperTest#CustomModels, CustomBlockWrapperTest#Drops, CustomBlockWrapperTest#RemoveBlock

---

CONCERN: CustomItemWrapperTest uses Mockito.mock(ItemStack.class) in ItemStackConstructor and CustomModelsTests. ItemStack is a concrete Bukkit class that MockBukkit provides support for (new ItemStack(Material.X) works directly in MockBukkit context).
WHY: The checklist explicitly prohibits using Mockito to mock a Bukkit class when MockBukkit provides a real implementation. ItemStack can be instantiated directly as new ItemStack(Material.DIAMOND_SWORD) — already done elsewhere in the same file.
WHERE: CustomItemWrapperTest#ItemStackConstructor, CustomItemWrapperTest#CustomModelsTests

---

CONCERN: TestCustomBlockPluginHook.isCustomBlockOfType hardcodes "nexo:ruby_ore" as the recognized type string. Tests in BlockConstructor that assert wrapper.customBlock().get() equals "nexo:ruby_ore" are passing because the hook's blockModels() returns Set.of("nexo:ruby_ore"), not because isCustomBlockOfType was exercised. The relationship between blockModels returning a model name and the wrapper storing that name as its custom block string is what's actually being tested — but the test comment/display name says "hook recognizing it" which is misleading about which method drives the stored value.
WHY: This is a structural problem: if production code uses blockModels to determine the stored custom block name, the test assertion on wrapper.customBlock().get() is only valid so long as blockModels returns exactly one element. The multi-model case (Set with 2+ entries) is not tested — what does customBlock() return then?
WHERE: CustomBlockWrapperTest#BlockConstructor#constructor_setsCustomBlock_whenHookRecognizesBlock

---

CONCERN: CustomEntityWrapperTest#EntityConstructor#constructor_setsCustomEntity_whenHookRecognizesEntity has the same structural problem: asserts wrapper.customEntity().get() equals "mythicmobs:fire_dragon" where the hook's entityModels returns Set.of("mythicmobs:fire_dragon"). No test covers what customEntity() returns when entityModels returns a set with multiple entries.
WHY: Coverage gap for multi-model edge case in entity wrapper construction.
WHERE: CustomEntityWrapperTest#EntityConstructor#constructor_setsCustomEntity_whenHookRecognizesEntity

---

CONCERN: StubNexoHook and StubItemsAdderHook are defined as static inner classes of ItemPluginTypeGetCustomItemTest, and TestCustomItemPluginHook is defined separately in both CustomBlockWrapperTest and CustomItemWrapperTest. All three serve the same conceptual role (stubbing CustomItemHook). These are not placed in src/testFixtures/java/.
WHY: The checklist requires shared fixtures in src/testFixtures/java/ so downstream repos (McRPG) can depend on them. Duplicating stub hook implementations across test files instead of placing them in test fixtures creates maintenance burden and prevents reuse.
WHERE: ItemPluginTypeGetCustomItemTest (StubNexoHook, StubItemsAdderHook), CustomBlockWrapperTest (TestCustomItemPluginHook), CustomItemWrapperTest (TestCustomItemPluginHook)

---

CONCERN: ItemPluginTypeGetCustomItemTest uses @BeforeEach/@AfterEach with MockBukkit.mock() and MockBukkit.unmock() at the outer class level. The @Nested inner classes inherit this setup. However, registerHookWithKey uses reflection to directly mutate the hooks map in PluginHookRegistry. If RegistryResetExtension.resetRegistry() does not clear this map, hooks registered in one test method may leak into the next test method within the same outer class instance.
WHY: Test isolation depends entirely on RegistryResetExtension.resetRegistry() cleaning up reflectively-mutated state. If that reset is incomplete, tests that pass in isolation may fail when run as a suite, and tests that expect no hooks may find leftover hooks from previous tests.
WHERE: ItemPluginTypeGetCustomItemTest#registerHookWithKey

---

CONCERN: ItemPluginType.NEXO.getCustomItem and ItemPluginType.ITEMS_ADDER.getCustomItem — there is no test for what happens when the hook's item() method returns a non-empty Optional but isItem() returns false (i.e., the hook says it doesn't recognize the name, but item() would return something). This tests the interaction between isItem and item() in the NEXO/ITEMS_ADDER branches specifically.
WHY: The NEXO and ITEMS_ADDER enum lambdas likely check isItem before calling item(). If production code skips the isItem guard, this is an untested behavioral assumption.
WHERE: ItemPluginTypeGetCustomItemTest#NexoGetCustomItem, ItemPluginTypeGetCustomItemTest#ItemsAdderGetCustomItem

---

Summary

Production files changed: None (all changes are in test files)

Test files present:

• src/test/java/com/diamonddagger590/mccore/builder/item/ItemPluginTypeGetCustomItemTest.java (new)
• src/test/java/com/diamonddagger590/mccore/util/item/CustomBlockWrapperTest.java (modified)
• src/test/java/com/diamonddagger590/mccore/util/item/CustomEntityWrapperTest.java (modified)
• src/test/java/com/diamonddagger590/mccore/util/item/CustomItemWrapperTest.java (modified)

Coverage gaps:

1. itemBuilder tests assert only non-null — cannot detect material/item regressions
2. No null-input test for getCustomItem(null) on any ItemPluginType
3. No test for both hooks simultaneously recognizing the same item in NONE
4. No-hook path for CustomBlockWrapper.drops untested
5. Multi-model edge case in CustomBlockWrapper and CustomEntityWrapper constructors untested
6. Mockito.mock() used for Bukkit types (Block, Entity, ItemStack) where MockBukkit provides real implementations
7. Stub hook fixtures duplicated across test classes instead of placed in src/testFixtures/java/
8. Hook state leakage risk via reflective map mutation not fully isolated by RegistryResetExtension
9. isItem=false + item()=present path not tested for NEXO/ITEMS_ADDER branches

[github-actions]

Security Review

No security concerns found.

The diff is entirely test code (src/test/java/...). No production DAO methods, DDL migration functions, MiniMessage deserialization call sites, ChatResponse implementations, Slot.onClick() overrides, or permission-gated operations are introduced or modified. The custom item/block/entity strings used in test stubs (e.g., "nexo:ruby_ore", "mythicmobs:fire_dragon") are hardcoded string literals within test fixtures and never flow into production framework paths. No checklist item applies.