[Unit Tests] Board events, ContentPackRegisteredEvent, and FakeBlockBreakEvent coverage#246
[Unit Tests] Board events, ContentPackRegisteredEvent, and FakeBlockBreakEvent coverage#246DiamondDagger590 wants to merge 1 commit into
Conversation
WalkthroughThree new test files add comprehensive JUnit 5 coverage for event system components. BoardEventCoverageTest validates six board and quest event types; ContentPackRegisteredEventTest covers content pack event accessors; FakeBlockBreakEventTest verifies mutable state and Bukkit inheritance. All tests use mocks for setup and assert constructor injection, handler lists, and behavioral contracts. ChangesEvent System Test Coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Extensibility ReviewBreaking change risk: NONE — This diff adds only test classes with no changes to production API surface. After reviewing all modified files, every change is confined to the No extensibility concerns found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/us/eunoians/mcrpg/event/content/ContentPackRegisteredEventTest.java`:
- Line 14: The test class ContentPackRegisteredEventTest should not extend
McRPGBaseTest; remove the "extends McRPGBaseTest" from the class declaration and
any unused imports brought in by that inheritance, and ensure no reliance on
inherited fixtures or setup (remove or replace any references to McRPGBaseTest
methods/fields in ContentPackRegisteredEventTest so it remains a plain unit test
of the event POJO).
In `@src/test/java/us/eunoians/mcrpg/event/fake/FakeBlockBreakEventTest.java`:
- Around line 20-21: In FakeBlockBreakEventTest replace the Mockito-created
Bukkit mocks with MockBukkit implementations: stop using mock(Block.class) and
mock(Player.class) and instead instantiate a BlockMock and PlayerMock (e.g., new
PlayerMock(...) and new BlockMock(...) or obtain them from the MockBukkit
server/arena setup used in your tests) so Bukkit behavior is simulated
correctly; update imports to use be.seeseemelk.mockbukkit.* (or the project's
MockBukkit package) and adjust any stubbing/assertions that relied on
Mockito-specific behavior to use the MockBukkit API (for example methods on
PlayerMock and BlockMock) so the test runs under MockBukkit lifecycle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd572b9c-bdd5-4f78-b595-79e375344b3e
📒 Files selected for processing (3)
src/test/java/us/eunoians/mcrpg/event/board/BoardEventCoverageTest.javasrc/test/java/us/eunoians/mcrpg/event/content/ContentPackRegisteredEventTest.javasrc/test/java/us/eunoians/mcrpg/event/fake/FakeBlockBreakEventTest.java
| import static org.junit.jupiter.api.Assertions.assertSame; | ||
| import static org.mockito.Mockito.mock; | ||
|
|
||
| class ContentPackRegisteredEventTest extends McRPGBaseTest { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Remove McRPGBaseTest extension for faster test execution.
This test doesn't require Bukkit server interaction or McRPGPlayer infrastructure—it only validates a simple event POJO. Extending McRPGBaseTest adds unnecessary bootstrap overhead (server mock, plugin initialization). As per coding guidelines, McRPGBaseTest is required only for tests that need those facilities.
⚡ Proposed fix to remove the extension
-class ContentPackRegisteredEventTest extends McRPGBaseTest {
+class ContentPackRegisteredEventTest {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class ContentPackRegisteredEventTest extends McRPGBaseTest { | |
| class ContentPackRegisteredEventTest { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/us/eunoians/mcrpg/event/content/ContentPackRegisteredEventTest.java`
at line 14, The test class ContentPackRegisteredEventTest should not extend
McRPGBaseTest; remove the "extends McRPGBaseTest" from the class declaration and
any unused imports brought in by that inheritance, and ensure no reliance on
inherited fixtures or setup (remove or replace any references to McRPGBaseTest
methods/fields in ContentPackRegisteredEventTest so it remains a plain unit test
of the event POJO).
| Block mockBlock = mock(Block.class); | ||
| Player mockPlayer = mock(Player.class); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Prefer MockBukkit implementations over Mockito mocks for Bukkit classes.
Use PlayerMock and BlockMock from MockBukkit instead of Mockito mock() for better Bukkit behavior simulation and alignment with project test conventions.
♻️ Proposed refactor
+import be.seeseemelk.mockbukkit.block.BlockMock;
+import be.seeseemelk.mockbukkit.entity.PlayerMock;
+
import org.bukkit.block.Block;
import org.bukkit.entity.Player;
import org.junit.jupiter.api.BeforeEach; `@BeforeEach`
void setUp() {
- Block mockBlock = mock(Block.class);
- Player mockPlayer = mock(Player.class);
+ Block mockBlock = new BlockMock(org.bukkit.Material.STONE);
+ Player mockPlayer = new PlayerMock(server, "testPlayer");
event = new FakeBlockBreakEvent(mockBlock, mockPlayer);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/us/eunoians/mcrpg/event/fake/FakeBlockBreakEventTest.java`
around lines 20 - 21, In FakeBlockBreakEventTest replace the Mockito-created
Bukkit mocks with MockBukkit implementations: stop using mock(Block.class) and
mock(Player.class) and instead instantiate a BlockMock and PlayerMock (e.g., new
PlayerMock(...) and new BlockMock(...) or obtain them from the MockBukkit
server/arena setup used in your tests) so Bukkit behavior is simulated
correctly; update imports to use be.seeseemelk.mockbukkit.* (or the project's
MockBukkit package) and adjust any stubbing/assertions that relied on
Mockito-specific behavior to use the MockBukkit API (for example methods on
PlayerMock and BlockMock) so the test runs under MockBukkit lifecycle.
Source: Coding guidelines
2 participants
Summary
@Nestedclasses covering all board event DTOs (BoardOfferingAcceptEvent,BoardOfferingExpireEvent,BoardOfferingGenerateEvent,BoardRotationEvent,PersonalOfferingGenerateEvent,TemplateQuestGenerateEvent). Tests cover constructor storage, getter delegation,Cancellablecontract,HandlerListidentity, list mutability/immutability contracts, and defensive copy verification.getContentPack(),getHandlers(), andgetHandlerList()identity.hasPassedChecksdefault value,setPassedCheckstoggle, revertibility, andBlockBreakEventinheritance.All three packages (
event/board/,event/content/,event/fake/) were previously at 0-17% coverage.Test plan
./gradlew verifiedShadowJar— BUILD SUCCESSFUL)https://claude.ai/code/session_01B6qDw2LWgyf67eaqa6V4y2
Generated by Claude Code
Summary by CodeRabbit