[Tests] Add unit tests for CommandRegistrar, Gui.refreshGUI, improve CorePlayer/CoreBootstrap coverage - #86
Conversation
…ove CorePlayer/CoreBootstrap coverage - Add CommandRegistrarTest: verify CoreCommandManager creation and registration using MockedConstruction to bypass PaperCommandManager runtime dependency - Add GuiRefreshDefaultMethodTest: cover Gui.refreshGUI() default method (0% → 100%) - CoreBootstrapTest: add PROD-path test with MockedConstruction for full line coverage - CorePlayerTest: add multi-hook isAfk tests and fix equals/hashCode branch coverage by using direct player.equals() calls instead of reversed assertNotEquals Coverage improvements: - CommandRegistrar: 50% → 100% line - Gui.refreshGUI: 0% → 100% line - CorePlayer: 83.3% → 100% branch, 96.7% → 100% line - CoreBootstrap: 95.7% → 100% line Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qoVrf8G5pup7SBnpnHAhb
|
Claude finished @DiamondDagger590's task in 3m 11s —— View job McCore PR ReviewVerdict: No blocking issues found This PR is test-only (224 additions, 0 deletions across 4 test files, no
No security/architecture/error-handling/performance/extensibility lenses were run since no
|
📝 WalkthroughWalkthroughThe PR adds tests for command-manager registration during bootstrap, GUI refresh delegation, multiple AFK hooks, and direct ChangesTest coverage expansion
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (3)
src/test/java/com/diamonddagger590/mccore/gui/GuiRefreshDefaultMethodTest.java-50-52 (1)
50-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the non-null annotation to the constructor parameter.
Line 50 dereferences
paintCountinpaintInventory(). Add@NotNullto document the required contract.Proposed fix
- TestGui(AtomicInteger paintCount) { + TestGui(`@NotNull` AtomicInteger paintCount) {As per coding guidelines, “Use
@NotNullannotation from IntelliJ v12 on all non-null return types and parameters.”🤖 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/com/diamonddagger590/mccore/gui/GuiRefreshDefaultMethodTest.java` around lines 50 - 52, Add the IntelliJ v12 `@NotNull` annotation to the paintCount parameter of the TestGui constructor, preserving the existing constructor behavior and imports.Source: Coding guidelines
src/test/java/com/diamonddagger590/mccore/player/CorePlayerTest.java-57-70 (1)
57-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the required nullability annotation and Javadoc.
Line 66 accepts a non-null
CorePlayerwithout@NotNull. Add the annotation and document the public override with@paramand@returntags.As per coding guidelines:
Use@NotNullannotation from IntelliJ v12 on all non-null return types and parametersandWrite Javadoc on all public methods with@PARAMand@returnsemantics.Proposed fix
+ /** + * Checks whether this test hook reports the player as AFK. + * + * `@param` corePlayer the player to check + * `@return` the configured AFK result + */ `@Override` - public boolean isAfk(CorePlayer corePlayer) { + public boolean isAfk(`@NotNull` CorePlayer corePlayer) {🤖 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/com/diamonddagger590/mccore/player/CorePlayerTest.java` around lines 57 - 70, Update SecondTestAfkPluginHook.isAfk(CorePlayer) to annotate its non-null corePlayer parameter with the project’s IntelliJ v12 `@NotNull` annotation, and add Javadoc for the public override including `@param` and `@return` tags.Source: Coding guidelines
src/test/java/com/diamonddagger590/mccore/player/CorePlayerTest.java-199-206 (1)
199-206: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid the direct
equals(null)call.PMD reports
EqualsNullat Line 206. UseObjects.equals(player, null)if branch coverage requires invokingCorePlayer.equals; otherwise remove the redundant direct assertion.Proposed fix
+import java.util.Objects; ... - assertFalse(player.equals(null)); + assertFalse(Objects.equals(player, null));🤖 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/com/diamonddagger590/mccore/player/CorePlayerTest.java` around lines 199 - 206, Remove the redundant direct player.equals(null) assertion in equals_returnsFalse_whenComparedToNull, retaining the null-inequality assertion; if direct CorePlayer.equals coverage is required, replace it with Objects.equals(player, null) instead.Source: Linters/SAST tools
🧹 Nitpick comments (1)
src/test/java/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.java (1)
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the test method names with the repository convention.
Rename
register_registersCoreCommandManagerto include its condition, such asregister_registersCoreCommandManager_whenBootstrapContextProvided.Rename
register_constructsCoreCommandManager_withContextPlugintoregister_constructsCoreCommandManager_whenContextPluginProvided.As per path instructions, test methods must use the
methodUnderTest_expectedOutcome_whenConditionconvention.Also applies to: 67-67
🤖 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/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.java` at line 51, Rename the test methods to follow the repository convention: change register_registersCoreCommandManager to register_registersCoreCommandManager_whenBootstrapContextProvided, and register_constructsCoreCommandManager_withContextPlugin to register_constructsCoreCommandManager_whenContextPluginProvided. Do not alter the test behavior.Source: Path instructions
🤖 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.
Other comments:
In
`@src/test/java/com/diamonddagger590/mccore/gui/GuiRefreshDefaultMethodTest.java`:
- Around line 50-52: Add the IntelliJ v12 `@NotNull` annotation to the paintCount
parameter of the TestGui constructor, preserving the existing constructor
behavior and imports.
In `@src/test/java/com/diamonddagger590/mccore/player/CorePlayerTest.java`:
- Around line 57-70: Update SecondTestAfkPluginHook.isAfk(CorePlayer) to
annotate its non-null corePlayer parameter with the project’s IntelliJ v12
`@NotNull` annotation, and add Javadoc for the public override including `@param`
and `@return` tags.
- Around line 199-206: Remove the redundant direct player.equals(null) assertion
in equals_returnsFalse_whenComparedToNull, retaining the null-inequality
assertion; if direct CorePlayer.equals coverage is required, replace it with
Objects.equals(player, null) instead.
---
Nitpick comments:
In
`@src/test/java/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.java`:
- Line 51: Rename the test methods to follow the repository convention: change
register_registersCoreCommandManager to
register_registersCoreCommandManager_whenBootstrapContextProvided, and
register_constructsCoreCommandManager_withContextPlugin to
register_constructsCoreCommandManager_whenContextPluginProvided. Do not alter
the test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: dbba0dbe-c4f1-4feb-88a6-95b2ed5b7824
📒 Files selected for processing (4)
src/test/java/com/diamonddagger590/mccore/bootstrap/CoreBootstrapTest.javasrc/test/java/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.javasrc/test/java/com/diamonddagger590/mccore/gui/GuiRefreshDefaultMethodTest.javasrc/test/java/com/diamonddagger590/mccore/player/CorePlayerTest.java
2 participants
Summary
CommandRegistrarTest— testsCommandRegistrar.register()usingMockedConstructionto bypassPaperCommandManagerruntime dependency. VerifiesCoreCommandManageris created and registered inManagerRegistrywith the correct plugin argument.GuiRefreshDefaultMethodTest— testsGui.refreshGUI()default method, which delegates topaintInventory(). Uses a concreteTestGuiwith anAtomicIntegercounter to verify delegation.CoreBootstrapTest— added PROD-path test usingMockedConstruction<CoreCommandManager>to properly cover theCommandRegistrarcall without relying on exception-swallowing. Also improved assertion quality (assertEqualsoverassertTruefor size checks).CorePlayerTest— added multi-hookisAfktests (second hook returns true, all hooks return false) and fixedequals/hashCodebranch coverage by adding directplayer.equals()calls (the originalassertNotEqualswith reversed argument order never exercisedCorePlayer.equals()).Coverage improvements
CommandRegistrarGui.refreshGUI()CorePlayerCoreBootstrapTest plan
./gradlew clean test— BUILD SUCCESSFUL)./gradlew shadowJar)methodUnderTest_expectedOutcome_whenCondition)@DisplayNameannotationsRegistryResetExtensionused for singleton state managementGenerated by Claude Code
Summary by CodeRabbit