[Tests] Add unit tests for BootstrapContext, IllegalSlotAssignmentException, TableVersionHistoryDAO, MutexDAO#35
Conversation
…eption, TableVersionHistoryDAO, MutexDAO https://claude.ai/code/session_0183JW46S7Paz6fmccowQB3S
WalkthroughThis PR adds comprehensive test coverage for four domain classes by introducing Mockito as a test dependency and creating four new test suites with nested test classes covering accessors, persistence operations, state transitions, and exception behavior. ChangesTest Coverage Expansion
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 infrastructure (Mockito dependencies and new test classes); no production source files are modified. No extensibility concerns found. The entire diff consists of:
No public API surfaces, abstract classes, interfaces, method signatures, registry keys, |
Testing ReviewI'll systematically apply every checklist item to the diff. FindingsCONCERN: CONCERN: CONCERN: CONCERN: CONCERN: CONCERN: CONCERN: CONCERN: Neither CONCERN: CONCERN: CONCERN: No tests are added for CONCERN: CONCERN: CONCERN: CONCERN: CONCERN: No tests are present in SummaryProduction files changed: Test files present: Coverage gaps: Migration DDL not verified (only version-tracking write verified); "no update" branches do not assert absence of writes; null/empty inputs untested for DAO public methods; "user does not exist" path for |
Security ReviewNo security concerns found. The diff contains exclusively test infrastructure: Mockito dependency additions in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@build.gradle.kts`:
- Around line 68-70: Update the pinned Mockito version by changing the
mockitoVersion variable used by testImplementation (symbol: mockitoVersion and
the two testImplementation("org.mockito:mockito-core:$mockitoVersion") /
testImplementation("org.mockito:mockito-junit-jupiter:$mockitoVersion") entries)
to the latest Java‑21 compatible release (e.g., "5.23.0"); additionally, if your
tests use inline mocking, add/replace the dependency with mockito-inline or
configure the Mockito Java agent (mockito-inline or -javaagent setup) so the
inline mock maker continues to work under Java 21.
In
`@src/test/java/com/diamonddagger590/mccore/bootstrap/BootstrapContextTest.java`:
- Around line 25-83: The test method names in BootstrapContextTest
(pluginAccessor, startupProfileAccessor, prodProfileAccessor, equalContexts,
differentProfiles, differentPlugins, hashCodeConsistency,
toStringContainsClassName) do not follow the givenContext_whenAction_thenOutcome
convention; rename each method to match that pattern (for example:
givenPlugin_whenPluginAccessor_thenReturnsPlugin,
givenPluginAndTestProfile_whenStartupProfileAccessor_thenReturnsTest,
givenPluginAndProdProfile_whenStartupProfileAccessor_thenReturnsProd,
givenSamePluginAndProfile_whenEquals_thenAreEqual,
givenDifferentProfiles_whenEquals_thenNotEqual,
givenDifferentPlugins_whenEquals_thenNotEqual,
givenEqualContexts_whenHashCode_thenMatch,
givenContext_whenToString_thenContainsClassName) while leaving the `@DisplayName`
annotations intact and updating any internal references or IDE-generated usages.
- Line 82: Replace the equality assertion that checks a boolean with an explicit
truth assertion: change the usage of assertEquals(true,
str.contains("BootstrapContext")) to use assertTrue on the condition (i.e., call
assertTrue(str.contains("BootstrapContext"))) so the test uses the clearer
assertion method (reference: BootstrapContextTest, the variable str and the
boolean check str.contains("BootstrapContext")).
🪄 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: 953d6229-eadc-4be0-91f4-6688d83aefe9
📒 Files selected for processing (5)
build.gradle.ktssrc/test/java/com/diamonddagger590/mccore/bootstrap/BootstrapContextTest.javasrc/test/java/com/diamonddagger590/mccore/database/table/impl/MutexDAOTest.javasrc/test/java/com/diamonddagger590/mccore/database/table/impl/TableVersionHistoryDAOTest.javasrc/test/java/com/diamonddagger590/mccore/exception/gui/IllegalSlotAssignmentExceptionTest.java
| val mockitoVersion = "5.14.2" | ||
| testImplementation("org.mockito:mockito-core:$mockitoVersion") | ||
| testImplementation("org.mockito:mockito-junit-jupiter:$mockitoVersion") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of Mockito compatible with Java 21 as of June 2026?
💡 Result:
As of June 12, 2026, the latest stable version of Mockito is 5.23.0, which was released on March 11, 2026 [1][2][3]. Mockito 5.x is compatible with Java 21 [4][5][6]. However, starting with Java 21, the JDK introduced stricter restrictions on libraries attaching Java agents to their own JVM [4][5][6]. Consequently, users may need to explicitly configure Mockito as a -javaagent in their build configuration (such as Gradle or Maven) to ensure the inline mock maker functions correctly [4][5][6]. Official documentation provides specific examples for configuring this in Gradle and Maven to maintain compatibility with these JDK security restrictions [4][5][6].
Citations:
- 1: https://github.com/mockito/mockito/releases
- 2: https://mvnrepository.com/artifact/org.mockito/mockito-core
- 3: https://github.com/mockito/mockito/releases/tag/v5.23.0
- 4: https://javadoc.io/static/org.mockito/mockito-core/5.21.0/org.mockito/org/mockito/Mockito.html
- 5: https://javadoc.io/static/org.mockito/mockito-core/5.16.1/org.mockito/org/mockito/Mockito.html
- 6: https://github.com/mockito/mockito/blob/main/mockito-core/src/main/java/org/mockito/Mockito.java
Update Mockito to the latest stable compatible with Java 21
In build.gradle.kts (lines 68-70), the project pins Mockito to 5.14.2. The latest stable Mockito release as of June 2026 is 5.23.0 (Java 21 compatible). Updating should improve security/bugfix coverage.
val mockitoVersion = "5.14.2"
testImplementation("org.mockito:mockito-core:$mockitoVersion")
testImplementation("org.mockito:mockito-junit-jupiter:$mockitoVersion")Also, if the test suite uses inline mocking, Java 21 may require mockito-inline/-javaagent configuration to keep the inline mock maker working.
🤖 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 `@build.gradle.kts` around lines 68 - 70, Update the pinned Mockito version by
changing the mockitoVersion variable used by testImplementation (symbol:
mockitoVersion and the two
testImplementation("org.mockito:mockito-core:$mockitoVersion") /
testImplementation("org.mockito:mockito-junit-jupiter:$mockitoVersion") entries)
to the latest Java‑21 compatible release (e.g., "5.23.0"); additionally, if your
tests use inline mocking, add/replace the dependency with mockito-inline or
configure the Mockito Java agent (mockito-inline or -javaagent setup) so the
inline mock maker continues to work under Java 21.
| void pluginAccessor() { | ||
| var context = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| assertEquals(mockPlugin, context.plugin()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a plugin and TEST profile, When constructing BootstrapContext, Then startupProfile() returns TEST") | ||
| void startupProfileAccessor() { | ||
| var context = new BootstrapContext<>(mockPlugin, StartupProfile.TEST); | ||
| assertEquals(StartupProfile.TEST, context.startupProfile()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a plugin and PROD profile, When constructing BootstrapContext, Then startupProfile() returns PROD") | ||
| void prodProfileAccessor() { | ||
| var context = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| assertEquals(StartupProfile.PROD, context.startupProfile()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two BootstrapContexts with same plugin and profile, When comparing, Then they are equal") | ||
| void equalContexts() { | ||
| var context1 = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| var context2 = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| assertEquals(context1, context2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two BootstrapContexts with different profiles, When comparing, Then they are not equal") | ||
| void differentProfiles() { | ||
| var context1 = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| var context2 = new BootstrapContext<>(mockPlugin, StartupProfile.TEST); | ||
| assertNotEquals(context1, context2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two BootstrapContexts with different plugins, When comparing, Then they are not equal") | ||
| void differentPlugins() { | ||
| var context1 = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| var context2 = new BootstrapContext<>(anotherMockPlugin, StartupProfile.PROD); | ||
| assertNotEquals(context1, context2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two equal BootstrapContexts, When computing hashCode, Then they match") | ||
| void hashCodeConsistency() { | ||
| var context1 = new BootstrapContext<>(mockPlugin, StartupProfile.TEST); | ||
| var context2 = new BootstrapContext<>(mockPlugin, StartupProfile.TEST); | ||
| assertEquals(context1.hashCode(), context2.hashCode()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a BootstrapContext, When calling toString, Then it contains the class name") | ||
| void toStringContainsClassName() { | ||
| var context = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| String str = context.toString(); | ||
| assertNotNull(str); | ||
| assertEquals(true, str.contains("BootstrapContext")); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Test method naming convention not followed across all test files.
The coding guidelines require test methods to follow the givenContext_whenAction_thenOutcome naming pattern. This convention is violated consistently across all four test files: BootstrapContextTest.java, MutexDAOTest.java, TableVersionHistoryDAOTest.java, and IllegalSlotAssignmentExceptionTest.java. While the @DisplayName annotations correctly follow Given/When/Then structure and provide good readability, the method names themselves should also follow the stated convention for consistency across the codebase.
The shared root cause is a single naming pattern choice applied throughout this PR. Adopting the required convention in all four files will improve discoverability and align with established project standards.
As per coding guidelines: "Follow the givenContext_whenAction_thenOutcome naming convention for test methods (e.g., givenEmptyPage_whenGetSlots_thenReturnsEmpty)."
🤖 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/BootstrapContextTest.java`
around lines 25 - 83, The test method names in BootstrapContextTest
(pluginAccessor, startupProfileAccessor, prodProfileAccessor, equalContexts,
differentProfiles, differentPlugins, hashCodeConsistency,
toStringContainsClassName) do not follow the givenContext_whenAction_thenOutcome
convention; rename each method to match that pattern (for example:
givenPlugin_whenPluginAccessor_thenReturnsPlugin,
givenPluginAndTestProfile_whenStartupProfileAccessor_thenReturnsTest,
givenPluginAndProdProfile_whenStartupProfileAccessor_thenReturnsProd,
givenSamePluginAndProfile_whenEquals_thenAreEqual,
givenDifferentProfiles_whenEquals_thenNotEqual,
givenDifferentPlugins_whenEquals_thenNotEqual,
givenEqualContexts_whenHashCode_thenMatch,
givenContext_whenToString_thenContainsClassName) while leaving the `@DisplayName`
annotations intact and updating any internal references or IDE-generated usages.
Source: Coding guidelines
| var context = new BootstrapContext<>(mockPlugin, StartupProfile.PROD); | ||
| String str = context.toString(); | ||
| assertNotNull(str); | ||
| assertEquals(true, str.contains("BootstrapContext")); |
There was a problem hiding this comment.
Use assertTrue instead of assertEquals(true, ...).
Replace assertEquals(true, str.contains("BootstrapContext")) with assertTrue(str.contains("BootstrapContext")) for clearer intent and better failure messages.
✨ Proposed fix
- assertEquals(true, str.contains("BootstrapContext"));
+ assertTrue(str.contains("BootstrapContext"));🤖 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/BootstrapContextTest.java`
at line 82, Replace the equality assertion that checks a boolean with an
explicit truth assertion: change the usage of assertEquals(true,
str.contains("BootstrapContext")) to use assertTrue on the condition (i.e., call
assertTrue(str.contains("BootstrapContext"))) so the test uses the clearer
assertion method (reference: BootstrapContextTest, the variable str and the
boolean check str.contains("BootstrapContext")).
2 participants
Summary
Infrastructure change
Added Mockito 5.14.2 as a
testImplementationdependency to enable mocking JDBC components (Connection, PreparedStatement, ResultSet), Database, CorePlugin, BaseGui, Slot, and CorePlayer for testing classes that depend on framework types.Classes covered (avoiding PR #27, PR #30, PR #31, PR #32, PR #33, and PR #34 overlap)
PR #27 covers GetItemRequest, StatisticEntry, ReloadableContent, ReloadableContentManager, StartupProfile. PR #30 covers RegistryAccess, StatisticModifyEvent, PostStatisticModifyEvent, Transaction. PR #31 covers database events, GUI events, SQLiteDatabaseDriver, DatabaseDriver, ManagerKey/PluginHookKey constants. PR #32 covers player events, CorePlayer, Credentials, ConnectionDetails, CorePlayerOfflineException. PR #33 covers PlayerStatisticData, ReloadableParser, ItemPluginType, ChainPlayerContextFilter. PR #34 covers BatchTransaction, FailSafeTransaction, ItemBuilderConfigurationKeys, RegistryKeyImpl. This PR targets entirely different classes with no overlap.
Coverage impact
4 classes improved: 2 brought from 0% to 100%, 2 brought from 0% to ~97.7%. Overall project coverage: 31.9% → 34.2% (+2.3pp, 94 new lines covered).
Test plan
./gradlew testhttps://claude.ai/code/session_0183JW46S7Paz6fmccowQB3S
Generated by Claude Code
Summary by CodeRabbit
BootstrapContext,MutexDAO,TableVersionHistoryDAO, andIllegalSlotAssignmentExceptionclasses with Mockito-based unit tests.