Skip to content

[Tests] Add unit tests for CommandRegistrar and CoreCommandManager, improve DAO updateTable edge cases - #126

Closed
DiamondDagger590 wants to merge 1 commit into
developfrom
claude/eager-thompson-9o21a1
Closed

DiamondDagger590 wants to merge 1 commit into
developfrom
claude/eager-thompson-9o21a1

Conversation

@DiamondDagger590

@DiamondDagger590 DiamondDagger590 commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • CommandRegistrarTest (new): Tests that register() correctly creates a CoreCommandManager, passes the plugin from the BootstrapContext, registers it in ManagerRegistry, and throws IllegalArgumentException on duplicate registration. Uses mockConstruction to intercept CoreCommandManager construction (which requires Paper runtime).
  • CoreCommandManagerTest (new): Tests all public methods — getCommandManager(), getAnnotationParser(), plugin(), registerConfirmationCommand(), and getConfirmationManager(). Covers the IllegalStateException paths for both double-registration and unregistered-access. Uses MockedStatic<PaperCommandManager> + MockedConstruction<AnnotationParser> to construct a real instance without Paper runtime. CoreCommandManager coverage: 20% → 100% line, 0% → 100% branch.
  • MutexDAOTest: Added updateTable_performsNoUpdate_whenVersionIsAboveCurrent edge case test (version 5 > CURRENT_TABLE_VERSION).
  • PlayerSettingDAOTest: Added matching updateTable_performsNoUpdate_whenVersionIsAboveCurrent edge case and missing never() static import.

Coverage Impact

Class Line Before Line After Branch Before Branch After
CommandRegistrar 50.0% 100.0% 100% 100%
CoreCommandManager 20.0% 100.0% 0.0% 100.0%
Overall 80.6% 81.0% 70.7% 71.0%

Test Plan

  • All 3 CommandRegistrarTest tests pass
  • All 7 CoreCommandManagerTest tests pass
  • All existing + new MutexDAOTest tests pass (17 total)
  • All existing + new PlayerSettingDAOTest tests pass
  • Full test suite passes (./gradlew clean test)
  • No production code changes — test-only PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01SSnBQANZdTxssxLVi5Fs3T


Generated by Claude Code

Summary by CodeRabbit

  • Tests
    • Added coverage for command registration, manager initialization, plugin association, and confirmation-manager registration and retrieval.
    • Added checks for database table updates when the stored version is newer, including verifying that no update is executed.

…AO updateTable edge cases

- Add CommandRegistrarTest: verifies CoreCommandManager registration in
  ManagerRegistry, plugin passthrough, and duplicate registration guard
- Add CoreCommandManagerTest: covers constructor, getters, confirmation
  manager registration/retrieval, and double-registration/unregistered
  IllegalStateException paths (0% → 100% branch coverage)
- Add MutexDAO updateTable test for version above current
- Add PlayerSettingDAO updateTable test for version above current
- Add missing `never()` import to PlayerSettingDAOTest

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SSnBQANZdTxssxLVi5Fs3T
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Claude finished @DiamondDagger590's task in 2m 2s —— View job


Claude PR Review

  • Gather context (PR diff, changed files)
  • Route to review lenses — test-only PR (no src/main changes), so only review-testing applies; production-focused lenses (security/architecture/error-handling/performance/extensibility) have no applicable surface
  • Run review-testing lens (in progress)
  • Consolidate findings
  • Post final review

Working...

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: DiamondDagger590/McCore/.coderabbit.yaml

Review profile: QUIET

Plan: Advanced

Run ID: c0d4b493-5907-48b1-b204-b4267252305d

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbf220 and aa7e693.

📒 Files selected for processing (4)
  • src/test/java/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.java
  • src/test/java/com/diamonddagger590/mccore/command/CoreCommandManagerTest.java
  • src/test/java/com/diamonddagger590/mccore/database/table/impl/MutexDAOTest.java
  • src/test/java/com/diamonddagger590/mccore/database/table/impl/PlayerSettingDAOTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request adds tests for command registration and CoreCommandManager behavior. It also adds tests for DAO update behavior when the supplied table version is above the current version.

Changes

Command Manager Tests

Layer / File(s) Summary
Registrar setup and registration tests
src/test/java/com/diamonddagger590/mccore/bootstrap/registrar/CommandRegistrarTest.java
Tests initialize and reset ManagerRegistry. They verify manager registration, plugin forwarding, and rejection of a second call to the same registrar.
CoreCommandManager construction and confirmation-manager tests
src/test/java/com/diamonddagger590/mccore/command/CoreCommandManagerTest.java
Tests verify constructed instances and plugin access. They also check confirmation-manager registration and retrieval before and after registration.

DAO Table-Version Tests

Layer / File(s) Summary
Above-current-version update tests
src/test/java/com/diamonddagger590/mccore/database/table/impl/MutexDAOTest.java, src/test/java/com/diamonddagger590/mccore/database/table/impl/PlayerSettingDAOTest.java
Tests verify that updateTable binds the table name but does not execute an update for table version 5.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to aa7e6

The added command-manager and DAO tests present no identified merge-blocking issue.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: new unit tests for CommandRegistrar and CoreCommandManager, plus DAO updateTable edge-case tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

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.

2 participants