[Tests] Add unit tests for ManagerKeyImpl, PluginHookKeyImpl, StatisticCacheKey, CoreGuiOpenEvent#42
[Tests] Add unit tests for ManagerKeyImpl, PluginHookKeyImpl, StatisticCacheKey, CoreGuiOpenEvent#42DiamondDagger590 wants to merge 1 commit into
Conversation
…ey, CoreGuiOpenEvent Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KddWqLYU2aDVveN7douCYe
WalkthroughFour new JUnit 5 test classes are added with no production code changes: ChangesNew unit test coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 under No extensibility concerns found. All four files introduced by this diff are test-only ( |
Testing ReviewI'll systematically apply every checklist item to the three new test files and the production classes they exercise. File-by-File Analysis
|
Security ReviewNo security concerns found. The diff contains exclusively test code ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/com/diamonddagger590/mccore/event/gui/CoreGuiOpenEventTest.java`:
- Around line 57-144: Add two new test methods to the CoreGuiOpenEventTest class
to cover null-input edge cases for the CoreGuiOpenEvent constructor. Create one
test that passes a null playerUUID parameter to the CoreGuiOpenEvent constructor
to verify it throws an exception or handles null appropriately, and create
another test that passes a null gui parameter to verify the expected behavior
for that required input. These tests should follow the same naming and assertion
patterns as the existing test methods in the class.
In
`@src/test/java/com/diamonddagger590/mccore/registry/manager/ManagerKeyImplTest.java`:
- Around line 25-30: The ManagerKeyImpl.create() method is annotated with
`@NotNull` on its parameter, but there is no test verifying the behavior when null
is passed. Add a new test method that passes null as the argument to
ManagerKeyImpl.create() and assert that it throws the expected exception (such
as NullPointerException or IllegalArgumentException). This test should follow
the same naming convention and DisplayName pattern as the existing
create_returnsNonNullKey_whenGivenManagerClass test to ensure null-input edge
cases are properly covered per coding guidelines.
In
`@src/test/java/com/diamonddagger590/mccore/registry/plugin/PluginHookKeyImplTest.java`:
- Around line 25-30: Add a new test method to the PluginHookKeyImplTest class
that tests the null-input edge case for the PluginHookKeyImpl.create() factory
method. The test should pass null as the hook class parameter and verify that
the method throws the expected exception (such as NullPointerException or
IllegalArgumentException) that aligns with the `@NotNull` contract on the class
parameter. Use an appropriate assertion like assertThrows to capture and
validate the exception behavior.
In
`@src/test/java/com/diamonddagger590/mccore/statistic/cache/StatisticCacheKeyTest.java`:
- Around line 27-127: Add two test methods to verify null-input edge cases for
the StatisticCacheKey constructor. Create a test that attempts to instantiate
StatisticCacheKey with a null uuid parameter while providing a valid key, and
another test that attempts to instantiate it with a valid uuid but a null key
parameter. These tests should verify that the constructor properly rejects null
inputs and throws an appropriate exception (such as NullPointerException or
IllegalArgumentException) to enforce the non-null contract for both the uuid and
key fields.
- Around line 76-80: The test method hashCode_differs_whenKeysAreNotEqual()
incorrectly asserts that unequal StatisticCacheKey objects must have different
hash codes, which violates Java's hashCode contract. Java only requires that
equal objects have equal hash codes, not the reverse. Replace this test with one
that validates the actual contract: create two StatisticCacheKey objects with
identical parameters (same UUID and KEY values), then use assertEquals() to
assert that their hash codes are equal.
🪄 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: a519f5ba-f1c1-448a-8ff9-944900b7693d
📒 Files selected for processing (4)
src/test/java/com/diamonddagger590/mccore/event/gui/CoreGuiOpenEventTest.javasrc/test/java/com/diamonddagger590/mccore/registry/manager/ManagerKeyImplTest.javasrc/test/java/com/diamonddagger590/mccore/registry/plugin/PluginHookKeyImplTest.javasrc/test/java/com/diamonddagger590/mccore/statistic/cache/StatisticCacheKeyTest.java
| @Test | ||
| @DisplayName("Given a player UUID, GUI, and key, when creating event, then getPlayerUUID returns the UUID") | ||
| void getPlayerUUID_returnsCorrectUUID_whenEventIsCreated() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, GUI_KEY); | ||
| assertEquals(PLAYER_UUID, event.getPlayerUUID()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a GUI, when creating event, then getGui returns the same GUI instance") | ||
| void getGui_returnsSameInstance_whenEventIsCreated() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, GUI_KEY); | ||
| assertEquals(gui, event.getGui()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a non-null GUI key, when calling getGuiKey, then returns present Optional with correct key") | ||
| void getGuiKey_returnsPresentOptional_whenKeyIsProvided() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, GUI_KEY); | ||
| assertTrue(event.getGuiKey().isPresent()); | ||
| assertEquals(GUI_KEY, event.getGuiKey().get()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a null GUI key, when calling getGuiKey, then returns empty Optional") | ||
| void getGuiKey_returnsEmptyOptional_whenKeyIsNull() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, null); | ||
| assertTrue(event.getGuiKey().isEmpty()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a CoreGuiOpenEvent, when calling getHandlers, then returns non-null HandlerList") | ||
| void getHandlers_returnsNonNullHandlerList_whenCalled() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, null); | ||
| assertNotNull(event.getHandlers()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given CoreGuiOpenEvent class, when calling getHandlerList, then returns non-null static HandlerList") | ||
| void getHandlerList_returnsNonNullStaticHandlerList_whenCalled() { | ||
| HandlerList handlerList = CoreGuiOpenEvent.getHandlerList(); | ||
| assertNotNull(handlerList); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a CoreGuiOpenEvent, when calling getHandlers and getHandlerList, then they return the same instance") | ||
| void getHandlers_returnsSameInstanceAsStaticGetHandlerList() { | ||
| StubGui gui = new StubGui(); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, gui, null); | ||
| assertEquals(CoreGuiOpenEvent.getHandlerList(), event.getHandlers()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a keyed GUI with a key, when creating event with that key, then event reflects the key") | ||
| void event_reflectsGuiKey_whenKeyedGuiIsUsed() { | ||
| StubKeyedGui keyedGui = new StubKeyedGui(GUI_KEY); | ||
| NamespacedKey extractedKey = keyedGui.getGuiKey().orElse(null); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, keyedGui, extractedKey); | ||
| assertTrue(event.getGuiKey().isPresent()); | ||
| assertEquals(GUI_KEY, event.getGuiKey().get()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a keyed GUI without a key, when creating event, then event has empty guiKey") | ||
| void event_hasEmptyGuiKey_whenKeyedGuiHasNoKey() { | ||
| StubKeyedGui keyedGui = new StubKeyedGui(null); | ||
| NamespacedKey extractedKey = keyedGui.getGuiKey().orElse(null); | ||
| CoreGuiOpenEvent event = new CoreGuiOpenEvent(PLAYER_UUID, keyedGui, extractedKey); | ||
| assertTrue(event.getGuiKey().isEmpty()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given different player UUIDs, when creating events, then each event has its own UUID") | ||
| void events_haveDifferentUUIDs_whenCreatedWithDifferentPlayers() { | ||
| UUID uuid1 = UUID.fromString("00000000-0000-0000-0000-000000000001"); | ||
| UUID uuid2 = UUID.fromString("00000000-0000-0000-0000-000000000002"); | ||
| StubGui gui = new StubGui(); | ||
|
|
||
| CoreGuiOpenEvent event1 = new CoreGuiOpenEvent(uuid1, gui, null); | ||
| CoreGuiOpenEvent event2 = new CoreGuiOpenEvent(uuid2, gui, null); | ||
|
|
||
| assertEquals(uuid1, event1.getPlayerUUID()); | ||
| assertEquals(uuid2, event2.getPlayerUUID()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add constructor null-input edge-case tests for playerUUID and gui.
Current coverage only exercises nullability for guiKey. Please add tests for new CoreGuiOpenEvent(null, gui, ...) and new CoreGuiOpenEvent(uuid, null, ...) to lock expected behavior for required inputs (throwing or explicit handling).
As per coding guidelines, “Cover edge cases in tests: null inputs, empty collections, zero/negative numeric inputs, and max/limit values.”
🤖 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/event/gui/CoreGuiOpenEventTest.java`
around lines 57 - 144, Add two new test methods to the CoreGuiOpenEventTest
class to cover null-input edge cases for the CoreGuiOpenEvent constructor.
Create one test that passes a null playerUUID parameter to the CoreGuiOpenEvent
constructor to verify it throws an exception or handles null appropriately, and
create another test that passes a null gui parameter to verify the expected
behavior for that required input. These tests should follow the same naming and
assertion patterns as the existing test methods in the class.
Source: Coding guidelines
| @Test | ||
| @DisplayName("Given a manager class, when creating a key, then key is not null") | ||
| void create_returnsNonNullKey_whenGivenManagerClass() { | ||
| ManagerKey<TestManagerA> key = ManagerKeyImpl.create(TestManagerA.class); | ||
| assertNotNull(key); | ||
| } |
There was a problem hiding this comment.
Add null-input coverage for the factory contract.
ManagerKeyImpl.create(...) is annotated @NotNull on its parameter, but there is no test asserting behavior when null is passed. Add a dedicated test to lock this edge-case contract.
As per coding guidelines, "Cover edge cases in tests: null inputs, empty collections, zero/negative numeric inputs, and max/limit values."
Proposed test addition
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
@@
`@Test`
`@DisplayName`("Given a key compared to null, when checking equality, then returns false")
void equals_returnsFalse_whenComparedToNull() {
ManagerKey<TestManagerA> key = ManagerKeyImpl.create(TestManagerA.class);
assertNotEquals(null, key);
}
+
+ `@Test`
+ `@DisplayName`("Given a null manager class, when creating a key, then throws NullPointerException")
+ void create_throwsNullPointerException_whenGivenNullManagerClass() {
+ assertThrows(NullPointerException.class, () -> ManagerKeyImpl.create(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/registry/manager/ManagerKeyImplTest.java`
around lines 25 - 30, The ManagerKeyImpl.create() method is annotated with
`@NotNull` on its parameter, but there is no test verifying the behavior when null
is passed. Add a new test method that passes null as the argument to
ManagerKeyImpl.create() and assert that it throws the expected exception (such
as NullPointerException or IllegalArgumentException). This test should follow
the same naming convention and DisplayName pattern as the existing
create_returnsNonNullKey_whenGivenManagerClass test to ensure null-input edge
cases are properly covered per coding guidelines.
Source: Coding guidelines
| @Test | ||
| @DisplayName("Given a hook class, when creating a key, then key is not null") | ||
| void create_returnsNonNullKey_whenGivenHookClass() { | ||
| PluginHookKey<TestHookA> key = PluginHookKeyImpl.create(TestHookA.class); | ||
| assertNotNull(key); | ||
| } |
There was a problem hiding this comment.
Add null-input test for PluginHookKeyImpl.create(...).
The suite does not cover null input for the factory method despite the @NotNull contract on the class parameter. Add one edge-case test to pin expected behavior.
As per coding guidelines, "Cover edge cases in tests: null inputs, empty collections, zero/negative numeric inputs, and max/limit values."
Proposed test addition
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
@@
`@Test`
`@DisplayName`("Given a key compared to null, when checking equality, then returns false")
void equals_returnsFalse_whenComparedToNull() {
PluginHookKey<TestHookA> key = PluginHookKeyImpl.create(TestHookA.class);
assertNotEquals(null, key);
}
+
+ `@Test`
+ `@DisplayName`("Given a null hook class, when creating a key, then throws NullPointerException")
+ void create_throwsNullPointerException_whenGivenNullHookClass() {
+ assertThrows(NullPointerException.class, () -> PluginHookKeyImpl.create(null));
+ }
}📝 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.
| @Test | |
| @DisplayName("Given a hook class, when creating a key, then key is not null") | |
| void create_returnsNonNullKey_whenGivenHookClass() { | |
| PluginHookKey<TestHookA> key = PluginHookKeyImpl.create(TestHookA.class); | |
| assertNotNull(key); | |
| } | |
| package com.diamonddagger590.mccore.registry.plugin; | |
| import com.diamonddagger590.mccore.registry.PluginHookKey; | |
| import org.junit.jupiter.api.DisplayName; | |
| import org.junit.jupiter.api.Test; | |
| import static org.junit.jupiter.api.Assertions.assertEquals; | |
| import static org.junit.jupiter.api.Assertions.assertNotEquals; | |
| import static org.junit.jupiter.api.Assertions.assertNotNull; | |
| import static org.junit.jupiter.api.Assertions.assertThrows; | |
| class PluginHookKeyImplTest { | |
| `@Test` | |
| `@DisplayName`("Given a hook class, when creating a key, then key is not null") | |
| void create_returnsNonNullKey_whenGivenHookClass() { | |
| PluginHookKey<TestHookA> key = PluginHookKeyImpl.create(TestHookA.class); | |
| assertNotNull(key); | |
| } | |
| `@Test` | |
| `@DisplayName`("Given a key compared to null, when checking equality, then returns false") | |
| void equals_returnsFalse_whenComparedToNull() { | |
| PluginHookKey<TestHookA> key = PluginHookKeyImpl.create(TestHookA.class); | |
| assertNotEquals(null, key); | |
| } | |
| `@Test` | |
| `@DisplayName`("Given a null hook class, when creating a key, then throws NullPointerException") | |
| void create_throwsNullPointerException_whenGivenNullHookClass() { | |
| assertThrows(NullPointerException.class, () -> PluginHookKeyImpl.create(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/registry/plugin/PluginHookKeyImplTest.java`
around lines 25 - 30, Add a new test method to the PluginHookKeyImplTest class
that tests the null-input edge case for the PluginHookKeyImpl.create() factory
method. The test should pass null as the hook class parameter and verify that
the method throws the expected exception (such as NullPointerException or
IllegalArgumentException) that aligns with the `@NotNull` contract on the class
parameter. Use an appropriate assertion like assertThrows to capture and
validate the exception behavior.
Source: Coding guidelines
| @Test | ||
| @DisplayName("Given a UUID and key, when creating a cache key, then accessors return the same values") | ||
| void accessors_returnCorrectValues_whenKeyIsCreated() { | ||
| StatisticCacheKey cacheKey = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertEquals(UUID_A, cacheKey.uuid()); | ||
| assertEquals(KEY_KILLS, cacheKey.key()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two cache keys with the same UUID and key, when comparing, then they are equal") | ||
| void equals_returnsTrue_whenBothFieldsMatch() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertEquals(key1, key2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two cache keys with different UUIDs, when comparing, then they are not equal") | ||
| void equals_returnsFalse_whenUuidsDiffer() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_KILLS); | ||
| assertNotEquals(key1, key2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two cache keys with different NamespacedKeys, when comparing, then they are not equal") | ||
| void equals_returnsFalse_whenNamespacedKeysDiffer() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_A, KEY_DEATHS); | ||
| assertNotEquals(key1, key2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two cache keys with completely different fields, when comparing, then they are not equal") | ||
| void equals_returnsFalse_whenBothFieldsDiffer() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_DEATHS); | ||
| assertNotEquals(key1, key2); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two equal cache keys, when comparing hash codes, then they are equal") | ||
| void hashCode_isSame_whenKeysAreEqual() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertEquals(key1.hashCode(), key2.hashCode()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given two different cache keys, when comparing hash codes, then they differ") | ||
| void hashCode_differs_whenKeysAreNotEqual() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_DEATHS); | ||
| assertNotEquals(key1.hashCode(), key2.hashCode()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a cache key, when calling toString, then it contains uuid and key info") | ||
| void toString_containsFieldInfo_whenCalled() { | ||
| StatisticCacheKey cacheKey = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| String str = cacheKey.toString(); | ||
| assertNotNull(str); | ||
| assertEquals(true, str.contains(UUID_A.toString())); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a cache key compared to itself, when checking equality, then returns true") | ||
| void equals_returnsTrue_whenComparedToSelf() { | ||
| StatisticCacheKey cacheKey = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertEquals(cacheKey, cacheKey); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a cache key compared to null, when checking equality, then returns false") | ||
| void equals_returnsFalse_whenComparedToNull() { | ||
| StatisticCacheKey cacheKey = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertNotEquals(null, cacheKey); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a cache key used as a map key, when retrieving by equal key, then value is found") | ||
| void cacheKey_worksAsMapKey_whenUsedInHashMap() { | ||
| Map<StatisticCacheKey, String> map = new HashMap<>(); | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| map.put(key1, "value"); | ||
|
|
||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| assertEquals("value", map.get(key2)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Given a cache key used as a map key, when retrieving by different key, then value is not found") | ||
| void cacheKey_returnsNull_whenMapKeyDoesNotMatch() { | ||
| Map<StatisticCacheKey, String> map = new HashMap<>(); | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| map.put(key1, "value"); | ||
|
|
||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_KILLS); | ||
| assertEquals(null, map.get(key2)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add null-input edge-case tests for StatisticCacheKey construction.
The suite misses required null-input edge cases for uuid and key, which leaves the non-null contract unverified.
Suggested tests
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
@@
`@Test`
`@DisplayName`("Given a cache key used as a map key, when retrieving by different key, then value is not found")
void cacheKey_returnsNull_whenMapKeyDoesNotMatch() {
Map<StatisticCacheKey, String> map = new HashMap<>();
StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS);
map.put(key1, "value");
StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_KILLS);
assertEquals(null, map.get(key2));
}
+
+ `@Test`
+ `@DisplayName`("Given a null UUID, when creating a cache key, then throws NullPointerException")
+ void constructor_throwsNullPointerException_whenUuidIsNull() {
+ assertThrows(NullPointerException.class, () -> new StatisticCacheKey(null, KEY_KILLS));
+ }
+
+ `@Test`
+ `@DisplayName`("Given a null NamespacedKey, when creating a cache key, then throws NullPointerException")
+ void constructor_throwsNullPointerException_whenNamespacedKeyIsNull() {
+ assertThrows(NullPointerException.class, () -> new StatisticCacheKey(UUID_A, null));
+ }
}As per coding guidelines, "Cover edge cases in tests: null inputs, empty collections, zero/negative numeric inputs, and max/limit values".
🤖 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/statistic/cache/StatisticCacheKeyTest.java`
around lines 27 - 127, Add two test methods to verify null-input edge cases for
the StatisticCacheKey constructor. Create a test that attempts to instantiate
StatisticCacheKey with a null uuid parameter while providing a valid key, and
another test that attempts to instantiate it with a valid uuid but a null key
parameter. These tests should verify that the constructor properly rejects null
inputs and throws an appropriate exception (such as NullPointerException or
IllegalArgumentException) to enforce the non-null contract for both the uuid and
key fields.
Source: Coding guidelines
| @DisplayName("Given two different cache keys, when comparing hash codes, then they differ") | ||
| void hashCode_differs_whenKeysAreNotEqual() { | ||
| StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS); | ||
| StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_DEATHS); | ||
| assertNotEquals(key1.hashCode(), key2.hashCode()); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Java, does the hashCode contract require unequal objects to have different hash codes?
💡 Result:
No, the Java hashCode contract does not require unequal objects to have different hash codes [1][2]. According to the official Java documentation for the Object class, the general contract of the hashCode method specifies three main rules [1][2]: 1. Consistency: The method must consistently return the same integer during an execution of an application, provided no information used in equals comparisons is modified [1][2]. 2. Equality: If two objects are equal according to the equals method, they must produce the same hash code [1][2]. 3. Inequality (The "No Requirement" Rule): It is not required that unequal objects produce distinct hash codes [1][2]. While it is not a requirement to have distinct hash codes for unequal objects, the documentation notes that producing distinct results (a uniform distribution) is beneficial because it can improve the performance of hash tables [1][2]. Instances where unequal objects share the same hash code are commonly referred to as "hash collisions" and are a standard consideration in the design of hashing algorithms [3].
Citations:
- 1: https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/Object.html
- 2: https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/Object.html
- 3: https://stackoverflow.com/questions/57670060/how-hashcode-is-generated-for-a-string-object-in-java
Fix test assertion to match Java hashCode contract.
The assertion assertNotEquals(key1.hashCode(), key2.hashCode()) tests an invalid contract. Java's hashCode specification only requires consistency and compatibility with equals(); unequal objects are explicitly allowed to share hash codes. Hash collisions are normal and expected.
Suggested fix
- `@DisplayName`("Given two different cache keys, when comparing hash codes, then they differ")
- void hashCode_differs_whenKeysAreNotEqual() {
- StatisticCacheKey key1 = new StatisticCacheKey(UUID_A, KEY_KILLS);
- StatisticCacheKey key2 = new StatisticCacheKey(UUID_B, KEY_DEATHS);
- assertNotEquals(key1.hashCode(), key2.hashCode());
+ `@DisplayName`("Given a cache key, when calling hashCode repeatedly, then it is consistent")
+ void hashCode_isConsistent_whenCalledRepeatedly() {
+ StatisticCacheKey key = new StatisticCacheKey(UUID_A, KEY_KILLS);
+ assertEquals(key.hashCode(), key.hashCode());
}🤖 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/statistic/cache/StatisticCacheKeyTest.java`
around lines 76 - 80, The test method hashCode_differs_whenKeysAreNotEqual()
incorrectly asserts that unequal StatisticCacheKey objects must have different
hash codes, which violates Java's hashCode contract. Java only requires that
equal objects have equal hash codes, not the reverse. Replace this test with one
that validates the actual contract: create two StatisticCacheKey objects with
identical parameters (same UUID and KEY values), then use assertEquals() to
assert that their hash codes are equal.
2 participants
Summary
create(),managerClass()getter, record accessormanager(), equality/hashCode for same and different classes, toString output, self-equality, and null comparisoncreate(),hookClass()getter, record accessorclazz(), equality/hashCode for same and different classes, toString output, self-equality, and null comparisonuuid(),key()), equality for matching fields, inequality when UUID differs / NamespacedKey differs / both differ, hashCode consistency, toString output, self-equality, null comparison, and HashMap key behavior (lookup by equal key succeeds, lookup by different key returns null)getPlayerUUID(),getGui()instance identity,getGuiKey()returning present Optional when key is non-null,getGuiKey()returning empty Optional when key is null,getHandlers()/getHandlerList()non-null and identity, KeyedGui integration (with and without key), and events with different player UUIDsTest counts
All 713 tests pass (671 existing + 42 new).
Test plan
./gradlew test🤖 Generated with Claude Code
https://claude.ai/code/session_01KddWqLYU2aDVveN7douCYe
Generated by Claude Code
Summary by CodeRabbit