[Tests] Add unit tests for ManagerRegistry, PluginHookRegistry, TimeProvider, DriverRegistry

.claude/commands/review-testing.md

@@ -7,6 +7,7 @@ Adopt the Testing Auditor Persona for McCore. McCore is a framework library with
 **Coverage Completeness**
 - For every new public method with non-trivial logic (>3 lines), is there a corresponding unit or integration test?
 - Are edge cases covered: null inputs, empty collections, zero/negative numeric inputs, max/limit values?
+- For config-driven values (`ReloadableContent` subclasses), is the code path tested with a value of `0` and at the maximum?
 - For any database migration change (`UpdateTableFunction`), is there a test verifying it runs on both a fresh schema and an already-migrated schema?
 - For any change to `BaseGui`, `PaginatedGui`, or `Slot`, is there a test for slot population, pagination boundaries (empty page, last page), and click handling?
 - If a bug was fixed, is there a regression test?
@@ -21,16 +22,17 @@ Adopt the Testing Auditor Persona for McCore. McCore is a framework library with
 - Is MockBukkit set up and torn down correctly (`MockBukkit.mock()` / `MockBukkit.unmock()`) — not leaked across tests?
 - Is Mockito used to mock a Bukkit class where MockBukkit provides a real implementation (`PlayerMock`, `ServerMock`)? Use the real implementation.
 - Is `MockBukkit.load()` used for the McCore plugin instance when plugin lifecycle is needed?
+- Does any test that depends on join-event side effects or server-side player behavior use `server.addPlayer()` rather than constructing `PlayerMock` directly?
 
 **Bukkit-Dependent vs. Pure-Java Separation**
-- Does any class mix pure logic with Bukkit API calls where only the pure logic is tested? Extract and unit-test the pure logic separately.
-- Does any test spin up MockBukkit but call zero Bukkit APIs? It should be a plain JUnit test instead.
+- Does any class mix pure logic with Bukkit API calls where only the pure logic is tested? Extract the pure logic into a testable helper and unit-test it separately.
+- Does any test spin up MockBukkit but use neither MockBukkit server interaction nor any Bukkit APIs? In that case, a plain JUnit test would suffice — but this check only applies if truly neither is needed.
 
 **Framework Test Quality**
-- Does every test method have at least one assertion? A test with no assertion cannot fail.
+- Does every test method have at least one assertion (`assertEquals`, `assertNotNull`, `assertTrue`, `assertThrows`, etc.)? A test with no assertion cannot fail.
 - Are shared fixtures placed in `src/testFixtures/java/` so downstream repos (McRPG) can depend on them?
-- Does every test method follow the `givenContext_whenAction_thenOutcome` naming convention?
-- Does every test method carry a `@DisplayName` annotation with a human-readable sentence describing the scenario?
+- Does every test method follow the `methodUnderTest_expectedOutcome_whenCondition` naming convention (e.g., `register_throwsIllegalArgument_whenManagerAlreadyRegistered`)? The `_whenCondition` suffix is optional when the context is obvious from the action and outcome alone.
+- Does every test method carry a `@DisplayName` annotation with a human-readable sentence in Given/When/Then format (e.g., `@DisplayName("Given a registered manager, when registering again, then throws IllegalArgumentException")`)?
 
 ## Instructions
 

.cursor/rules/persona-testing.mdc

@@ -17,6 +17,7 @@ You are a test engineer reviewing whether this McCore framework change is adequa
 **Coverage Completeness**
 - [ ] For every new public method with non-trivial logic (>3 lines), is there a corresponding unit or integration test?
 - [ ] Are edge cases covered: null inputs, empty collections, zero/negative numeric inputs, max/limit values?
+- [ ] For config-driven values (`ReloadableContent` subclasses), is the code path tested with a value of `0` and at the maximum?
 - [ ] For any change to database migration logic (`UpdateTableFunction`), is there a test that verifies the migration runs successfully on both a fresh schema and an already-migrated schema?
 - [ ] For any change to `BaseGui`, `PaginatedGui`, or `Slot`, is there a test verifying slot population, pagination boundary behavior (empty page, last page), and click handling?
 - [ ] If a bug was fixed, is there a regression test?
@@ -31,16 +32,17 @@ You are a test engineer reviewing whether this McCore framework change is adequa
 - [ ] Is MockBukkit set up and torn down correctly (`MockBukkit.mock()` / `MockBukkit.unmock()`) — not leaked across tests?
 - [ ] Is Mockito used to mock a Bukkit class where MockBukkit already provides a real implementation (e.g., `PlayerMock`, `ServerMock`)? Use the real MockBukkit implementation.
 - [ ] Is `MockBukkit.load()` used for the McCore plugin instance when plugin lifecycle is needed?
+- [ ] Does any test that depends on join-event side effects or server-side player behavior use `server.addPlayer()` rather than constructing `PlayerMock` directly?
 
 **Bukkit-Dependent vs. Pure-Java Separation**
-- [ ] Does any class mix pure logic (math, data transformation, string processing) with Bukkit API calls, with only the pure logic tested? Extract and unit-test the pure logic separately.
-- [ ] Does any test spin up MockBukkit but call zero Bukkit APIs? It should be a plain JUnit test instead to reduce overhead.
+- [ ] Does any class mix pure logic (math, data transformation, string processing) with Bukkit API calls, with only the pure logic tested? Extract the pure logic into a testable helper and unit-test it separately.
+- [ ] Does any test spin up MockBukkit but use neither MockBukkit server interaction nor any Bukkit APIs? In that case, a plain JUnit test would suffice — but this check only applies if truly neither is needed.
 
 **Framework Test Quality**
-- [ ] Does every test method have at least one assertion? A test with no assertion cannot fail.
+- [ ] Does every test method have at least one assertion (`assertEquals`, `assertNotNull`, `assertTrue`, `assertThrows`, etc.)? A test with no assertion cannot fail.
 - [ ] Are shared fixtures or plugin setup helpers placed in `src/testFixtures/java/` so downstream repos (`McRPG`) can depend on them without duplicating setup?
-- [ ] Does every test method follow the `givenContext_whenAction_thenOutcome` naming convention (e.g., `givenEmptyPage_whenGetSlots_thenReturnsEmpty`)?
-- [ ] Does every test method carry a `@DisplayName` annotation with a human-readable sentence describing the scenario?
+- [ ] Does every test method follow the `methodUnderTest_expectedOutcome_whenCondition` naming convention (e.g., `register_throwsIllegalArgument_whenManagerAlreadyRegistered`)? The `_whenCondition` suffix is optional when the context is obvious from the action and outcome alone.
+- [ ] Does every test method carry a `@DisplayName` annotation with a human-readable sentence in Given/When/Then format (e.g., `@DisplayName("Given a registered manager, when registering again, then throws IllegalArgumentException")`)?
 
 ## Output Format
 

src/test/java/com/diamonddagger590/mccore/database/driver/DatabaseDriverTypeTest.java

@@ -0,0 +1,64 @@
+package com.diamonddagger590.mccore.database.driver;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class DatabaseDriverTypeTest {
+
+    @Test
+    @DisplayName("Given SQLITE driver type, when getting driver name, then returns sqlite")
+    void getDriverName_returnsSqlite_forSqliteType() {
+        assertEquals("sqlite", DatabaseDriverType.SQLITE.getDriverName());
+    }
+
+    @Test
+    @DisplayName("Given valid driver name string, when calling fromString, then returns matching type")
+    void fromString_returnsMatchingType_whenValidNameProvided() {
+        Optional<DatabaseDriverType> result = DatabaseDriverType.fromString("sqlite");
+        assertTrue(result.isPresent());
+        assertEquals(DatabaseDriverType.SQLITE, result.get());
+    }
+
+    @Test
+    @DisplayName("Given valid driver name with different case, when calling fromString, then returns matching type")
+    void fromString_returnsMatchingType_whenCaseIsDifferent() {
+        Optional<DatabaseDriverType> result = DatabaseDriverType.fromString("SQLITE");
+        assertTrue(result.isPresent());
+        assertEquals(DatabaseDriverType.SQLITE, result.get());
+    }
+
+    @Test
+    @DisplayName("Given mixed case driver name, when calling fromString, then returns matching type")
+    void fromString_returnsMatchingType_whenMixedCase() {
+        Optional<DatabaseDriverType> result = DatabaseDriverType.fromString("SqLiTe");
+        assertTrue(result.isPresent());
+        assertEquals(DatabaseDriverType.SQLITE, result.get());
+    }
+
+    @Test
+    @DisplayName("Given invalid driver name, when calling fromString, then returns empty optional")
+    void fromString_returnsEmpty_whenInvalidNameProvided() {
+        Optional<DatabaseDriverType> result = DatabaseDriverType.fromString("mysql");
+        assertFalse(result.isPresent());
+    }
+
+    @Test
+    @DisplayName("Given empty string, when calling fromString, then returns empty optional")
+    void fromString_returnsEmpty_whenEmptyStringProvided() {
+        Optional<DatabaseDriverType> result = DatabaseDriverType.fromString("");
+        assertFalse(result.isPresent());
+    }
+
+    @Test
+    @DisplayName("Given values enumeration, when checking, then contains SQLITE")
+    void values_containsSqlite() {
+        assertTrue(Arrays.asList(DatabaseDriverType.values()).contains(DatabaseDriverType.SQLITE));
+    }
+}

src/test/java/com/diamonddagger590/mccore/database/driver/DriverRegistryTest.java

@@ -0,0 +1,138 @@
+package com.diamonddagger590.mccore.database.driver;
+
+import com.diamonddagger590.mccore.database.Credentials;
+import com.diamonddagger590.mccore.pair.Pair;
+import com.zaxxer.hikari.HikariDataSource;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class DriverRegistryTest {
+
+    private static class TestDriver implements DatabaseDriver {
+
+        private final boolean driverAvailable;
+
+        TestDriver(boolean driverAvailable) {
+            this.driverAvailable = driverAvailable;
+        }
+
+        @NotNull
+        @Override
+        public String getDatabaseDriverClass() {
+            return "org.sqlite.JDBC";
+        }
+
+        @NotNull
+        @Override
+        public String getConnectionUrl(@NotNull Credentials credentials) {
+            return "jdbc:sqlite:test.db";
+        }
+
+        @Override
+        public void populateDataSourceCredentials(@NotNull HikariDataSource dataSource, @NotNull Credentials credentials) {
+        }
+
+        @NotNull
+        @Override
+        public DatabaseDriverType getDriverType() {
+            return DatabaseDriverType.SQLITE;
+        }
+
+        @Override
+        public boolean tryDriver() {
+            return driverAvailable;
+        }
+
+        @NotNull
+        @Override
+        public List<Pair<String, String>> getDataSourceProperties() {
+            return List.of();
+        }
+    }
+
+    private DriverRegistry registry;
+
+    @BeforeEach
+    void setUp() {
+        registry = new DriverRegistry();
+    }
+
+    @Test
+    @DisplayName("Given a valid driver, when registering, then registration succeeds")
+    void register_succeeds_whenDriverIsValid() {
+        TestDriver driver = new TestDriver(true);
+        registry.register(driver);
+        assertTrue(registry.registered(driver));
+    }
+
+    @Test
+    @DisplayName("Given a driver whose tryDriver fails, when registering, then throws RuntimeException")
+    void register_throwsRuntimeException_whenDriverClassMissing() {
+        TestDriver driver = new TestDriver(false);
+        assertThrows(RuntimeException.class, () -> registry.register(driver));
+    }
+
+    @Test
+    @DisplayName("Given a registered driver, when checking registered by instance, then returns true")
+    void registered_returnsTrue_whenDriverIsRegistered() {
+        TestDriver driver = new TestDriver(true);
+        registry.register(driver);
+        assertTrue(registry.registered(driver));
+    }
+
+    @Test
+    @DisplayName("Given no registered drivers, when checking registered by instance, then returns false")
+    void registered_returnsFalse_whenDriverIsNotRegistered() {
+        assertFalse(registry.registered(new TestDriver(true)));
+    }
+
+    @Test
+    @DisplayName("Given a registered driver, when checking by driver type, then returns true")
+    void isDriverRegistered_returnsTrue_whenDriverTypeIsRegistered() {
+        registry.register(new TestDriver(true));
+        assertTrue(registry.isDriverRegistered(DatabaseDriverType.SQLITE));
+    }
+
+    @Test
+    @DisplayName("Given no registered drivers, when checking by driver type, then returns false")
+    void isDriverRegistered_returnsFalse_whenDriverTypeIsNotRegistered() {
+        assertFalse(registry.isDriverRegistered(DatabaseDriverType.SQLITE));
+    }
+
+    @Test
+    @DisplayName("Given a registered driver, when getting by driver type, then returns Optional with driver")
+    void getDriver_returnsPresent_whenDriverTypeIsRegistered() {
+        TestDriver driver = new TestDriver(true);
+        registry.register(driver);
+        Optional<DatabaseDriver> result = registry.getDriver(DatabaseDriverType.SQLITE);
+        assertTrue(result.isPresent());
+        assertEquals(driver, result.get());
+    }
+
+    @Test
+    @DisplayName("Given no registered drivers, when getting by driver type, then returns empty Optional")
+    void getDriver_returnsEmpty_whenDriverTypeIsNotRegistered() {
+        Optional<DatabaseDriver> result = registry.getDriver(DatabaseDriverType.SQLITE);
+        assertFalse(result.isPresent());
+    }
+
+    @Test
+    @DisplayName("Given a driver already registered for a type, when registering another driver for the same type, then overwrites the first")
+    void register_overwritesExistingDriver_whenSameDriverTypeRegisteredTwice() {
+        TestDriver first = new TestDriver(true);
+        TestDriver second = new TestDriver(true);
+        registry.register(first);
+        registry.register(second);
+        assertEquals(second, registry.getDriver(DatabaseDriverType.SQLITE).orElse(null));
+    }
+}

src/test/java/com/diamonddagger590/mccore/parser/ParserTest.java

@@ -360,7 +360,14 @@ void getValue_throwsParseError_whenOpenParenthesisUnmatched() {
     @Test
     @DisplayName("Given function without parenthesis, when evaluating, then throws ParseError")
     void getValue_throwsParseError_whenFunctionMissingParenthesis() {
-        assertThrows(ParseError.class, () -> new Parser("sin 5").getValue());
+        assertThrows(ParseError.class, () -> new Parser("sin").getValue());
+    }
+
+    @Test
+    @DisplayName("Given function name with space-separated argument, when evaluating, then treats as variable due to space stripping")
+    void getValue_treatsAsVariable_whenFunctionNameSpaceSeparatedFromArgument() {
+        // "sin 5" has its space stripped → "sin5" → not a function name, treated as variable → default 0.0
+        assertEquals(0.0, new Parser("sin 5").getValue(), 1e-10);
     }
 
     @Test