test: extend test coverage (#73)

Author: tkhquang

AGENTS.md

@@ -223,6 +223,8 @@ dispatcher.emit_safe(PlayerStateChanged{.health = player->health});
 - **Shutdown orchestration tests:** `tests/test_shutdown.cpp` tests the `DMK_Shutdown()` function from `DetourModKit.hpp`, verifying idempotent shutdown, hot-reload re-initialization, and correct teardown ordering across all subsystems.
 - **Integration tests:** `tests/test_hook_integration.cpp` tests cross-module hooking against `tests/fixtures/hook_target_lib.cpp` (built as a DLL). The DLL exports `extern "C"` functions with volatile magic constants for stable AOB patterns. Includes hot-reload integration tests that exercise full hook/teardown/re-hook cycles, multi-type reload, enable/disable toggling, and repeated cycle stress.
 - **Platform tests:** `tests/test_platform.cpp` tests internal loader-lock detection and module pinning utilities from `src/platform.hpp`.
+- **Decoder tests:** `tests/test_x86_decode.cpp` tests the internal header `src/x86_decode.hpp` (RIP-relative E9 / EB / FF25 resolvers consumed by `Scanner`). The test file adds `src/` to its include path and drives each decoder with hand-crafted byte buffers.
+- **Worker tests:** `tests/test_worker.cpp` covers the `StoppableWorker` RAII `std::jthread` wrapper, including the empty-body early return, swallowed `std::exception` and unknown-exception paths, and idempotent `request_stop()` / `shutdown()`. The loader-lock detach arm is untestable from user code (only reached under DllMain) and is accepted as such.
 - **Test fixture pattern:** Each suite uses a `::testing::Test` subclass with `SetUp()`/`TearDown()` for temp file cleanup. Temp file paths must include the process ID (`_getpid()`) and a counter to avoid collisions when CTest runs tests in parallel as separate processes.
 - **VMT hook test lifetime:** GoogleTest destroys test-body locals *before* calling `TearDown()`. VMT tests must explicitly call `remove_all_vmt_hooks()` (or `remove_vmt_hook`) before target objects go out of scope. Do not rely on `TearDown()` for VMT cleanup when the hooked object is a test-body local.
 - **Coverage gate:** 80% minimum line coverage enforced in CI. All PRs must pass.

docs/tests/README.md

@@ -280,6 +280,21 @@ This is expected when casting `FARPROC` from `GetProcAddress` to a typed functio
 
 These tests enable the test-only `debug_snapshot_use_count()` accessor via `#define DMK_EVENT_DISPATCHER_INTERNAL_TESTING 1` at the top of the translation unit. The macro is not part of the public API and must not be defined in consumer code.
 
+## x86 Control-Flow Decoder Tests
+
+`tests/test_x86_decode.cpp` exercises the internal header `src/x86_decode.hpp`, which is consumed by `Scanner` for RIP-relative jump/call resolution. The decoders are small and pure, so each branch is driven by crafting a byte buffer and calling the decoder directly:
+
+| Test | What it proves |
+| ---- | -------------- |
+| `DecodeE9Rel32_WrongOpcodeRejected` / `DecodeEbRel8_WrongOpcodeRejected` | Opcode-mismatch short-circuit returns `std::nullopt` without reading the displacement. |
+| `DecodeE9Rel32_ValidForwardDisplacement` / `DecodeE9Rel32_ValidBackwardDisplacement` | `base + 5 + disp32` is computed for both positive and negative displacements. |
+| `DecodeEbRel8_NegativeDisplacementSignExtended` | `0xFE` on the displacement byte decodes as `-2`, proving the `std::int8_t` cast sign-extends correctly. |
+| `DecodeFf25Indirect_WrongFirstByteRejected` / `DecodeFf25Indirect_WrongSecondByteRejected` | Both halves of the compound `FF 25` opcode predicate are rejected independently. |
+| `DecodeFf25Indirect_UnreadableSlotRejected` | A displacement that places the indirect slot outside the user address range returns `std::nullopt`. |
+| `DecodeFf25Indirect_SlotProducesDestination` | Happy path: the slot pointer is materialised and returned verbatim. An aligned struct lays out the instruction and slot so the RIP-relative displacement is independent of padding. |
+
+The decoder header lives under `src/` (not the public include tree), so the test file adds `src/` to its include path and uses `DetourModKit::detail::` directly.
+
 ## Benchmark Harness
 
 `tests/bench_event_dispatcher.cpp` is a standalone microbenchmark executable. It is deliberately not a gtest binary so it can run under any build configuration (release, release+PGO, ASAN, etc.) without dragging in the gtest runtime.
@@ -310,7 +325,9 @@ tests/
 ├── fixtures/
 │   └── hook_target_lib.cpp     # Fixture DLL (exported functions for integration tests)
 ├── test_async_logger.cpp       # Async logger tests
+├── test_bootstrap.cpp          # DllMain lifecycle and instance-gate tests
 ├── test_config.cpp             # Configuration tests
+├── test_config_watcher.cpp     # INI hot-reload watcher tests
 ├── test_event_dispatcher.cpp   # Event dispatcher tests (incl. fast-path and snapshot stability)
 ├── test_filesystem.cpp         # Filesystem tests
 ├── test_format.cpp             # Format utilities tests
@@ -325,7 +342,9 @@ tests/
 ├── test_scanner.cpp            # AOB scanner tests
 ├── test_shutdown.cpp           # DMK_Shutdown orchestration tests
 ├── test_string.cpp             # String utilities tests
-└── test_win_file_stream.cpp    # Win32 file stream tests
+├── test_win_file_stream.cpp    # Win32 file stream tests
+├── test_worker.cpp             # StoppableWorker jthread RAII tests
+└── test_x86_decode.cpp         # x86 control-flow instruction decoders (internal)
 
 docs/tests/
 ├── README.md                   # This guide

tests/test_async_logger.cpp

@@ -470,6 +470,67 @@ TEST(AsyncLoggerConfigTest, NullMutex_Throws)
     EXPECT_THROW(AsyncLogger(config, file_stream, nullptr), std::invalid_argument);
 }
 
+TEST(AsyncLoggerConfigTest, Validate_DefaultIsValid)
+{
+    AsyncLoggerConfig config;
+    EXPECT_TRUE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsNonPowerOfTwoCapacity)
+{
+    AsyncLoggerConfig config;
+    config.queue_capacity = 5;
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsCapacityBelowTwo)
+{
+    AsyncLoggerConfig config;
+    config.queue_capacity = 1;
+    EXPECT_FALSE(config.validate());
+    config.queue_capacity = 0;
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsZeroBatch)
+{
+    AsyncLoggerConfig config;
+    config.batch_size = 0;
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsNonPositiveFlushInterval)
+{
+    AsyncLoggerConfig config;
+    config.flush_interval = std::chrono::milliseconds{0};
+    EXPECT_FALSE(config.validate());
+    config.flush_interval = std::chrono::milliseconds{-1};
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsZeroSpinBackoff)
+{
+    AsyncLoggerConfig config;
+    config.spin_backoff_iterations = 0;
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsNonPositiveBlockTimeout)
+{
+    AsyncLoggerConfig config;
+    config.block_timeout_ms = std::chrono::milliseconds{0};
+    EXPECT_FALSE(config.validate());
+    config.block_timeout_ms = std::chrono::milliseconds{-5};
+    EXPECT_FALSE(config.validate());
+}
+
+TEST(AsyncLoggerConfigTest, Validate_RejectsZeroBlockMaxSpin)
+{
+    AsyncLoggerConfig config;
+    config.block_max_spin_iterations = 0;
+    EXPECT_FALSE(config.validate());
+}
+
 TEST(DynamicMPMCQueueTest, InvalidCapacity_NotPowerOfTwo)
 {
     EXPECT_THROW(DynamicMPMCQueue(3), std::invalid_argument);
@@ -598,36 +659,6 @@ TEST(LogMessageTest, LargeMessage_HeapPath)
     EXPECT_NE(log_msg.overflow, nullptr);
 }
 
-TEST(AsyncLoggerConfigTest, Validate_AllReturnValues)
-{
-    AsyncLoggerConfig valid_config;
-    EXPECT_TRUE(valid_config.validate());
-
-    AsyncLoggerConfig zero_cap;
-    zero_cap.queue_capacity = 0;
-    EXPECT_FALSE(zero_cap.validate());
-
-    AsyncLoggerConfig cap_one;
-    cap_one.queue_capacity = 1;
-    EXPECT_FALSE(cap_one.validate());
-
-    AsyncLoggerConfig bad_cap;
-    bad_cap.queue_capacity = 100;
-    EXPECT_FALSE(bad_cap.validate());
-
-    AsyncLoggerConfig zero_batch;
-    zero_batch.batch_size = 0;
-    EXPECT_FALSE(zero_batch.validate());
-
-    AsyncLoggerConfig zero_interval;
-    zero_interval.flush_interval = std::chrono::milliseconds{0};
-    EXPECT_FALSE(zero_interval.validate());
-
-    AsyncLoggerConfig zero_spin;
-    zero_spin.spin_backoff_iterations = 0;
-    EXPECT_FALSE(zero_spin.validate());
-}
-
 TEST(AsyncLoggerConfigTest, Validate_ValidSpinBackoff)
 {
     AsyncLoggerConfig config;

tests/test_bootstrap.cpp

@@ -71,6 +71,84 @@ TEST(BootstrapUnitTest, RequestShutdownBeforeAttachIsNoOp)
     EXPECT_NO_THROW(Bootstrap::request_shutdown());
 }
 
+TEST(BootstrapUnitTest, ModuleHandleReturnsNullBeforeAttach)
+{
+    EXPECT_EQ(Bootstrap::module_handle(), nullptr);
+}
+
+TEST(BootstrapUnitTest, NullHModuleSkipsDisableThreadLibraryCalls)
+{
+    CallbackSignals sig;
+    Bootstrap::ModInfo info{};
+    info.prefix = "BS_TEST";
+    info.log_file = "bs_test_nullmod.log";
+    info.game_process_name = "DefinitelyNotTheCurrentProcess_null.exe";
+    info.instance_mutex_prefix = "BS_Test_Mutex_NullMod_";
+
+    const BOOL result = Bootstrap::on_dll_attach(
+        nullptr,
+        info,
+        [&sig]() noexcept
+        {
+            sig.init_calls.fetch_add(1, std::memory_order_relaxed);
+            return true;
+        },
+        [&sig]() noexcept
+        {
+            sig.shutdown_calls.fetch_add(1, std::memory_order_relaxed);
+        });
+
+    EXPECT_EQ(result, FALSE);
+    EXPECT_EQ(sig.init_calls.load(), 0);
+    EXPECT_EQ(sig.shutdown_calls.load(), 0);
+}
+
+TEST(BootstrapUnitTest, EmptyProcessNamePassesGateButMutexCollisionStillFails)
+{
+    // Pre-own the mutex name so the attach still fails without arming the
+    // shutdown event or the worker thread.
+    const std::string_view prefix = "BS_Test_Mutex_EmptyGate_";
+
+    wchar_t expected_name[128]{};
+    std::wstring wprefix;
+    wprefix.reserve(prefix.size());
+    for (char c : prefix)
+    {
+        wprefix.push_back(static_cast<wchar_t>(static_cast<unsigned char>(c)));
+    }
+    const int n = wsprintfW(expected_name, L"%s%lu", wprefix.c_str(),
+                            GetCurrentProcessId());
+    ASSERT_GT(n, 0);
+
+    HANDLE pre_owned = CreateMutexW(nullptr, FALSE, expected_name);
+    ASSERT_NE(pre_owned, nullptr);
+    ASSERT_EQ(GetLastError(), 0u);
+
+    CallbackSignals sig;
+    Bootstrap::ModInfo info{};
+    info.prefix = "BS_TEST";
+    info.log_file = "bs_test_emptygate.log";
+    info.game_process_name = "";
+    info.instance_mutex_prefix = prefix;
+
+    const BOOL result = Bootstrap::on_dll_attach(
+        GetModuleHandleW(nullptr),
+        info,
+        [&sig]() noexcept
+        {
+            sig.init_calls.fetch_add(1, std::memory_order_relaxed);
+            return true;
+        },
+        [&sig]() noexcept
+        {
+            sig.shutdown_calls.fetch_add(1, std::memory_order_relaxed);
+        });
+
+    EXPECT_EQ(result, FALSE);
+    EXPECT_EQ(sig.init_calls.load(), 0);
+    CloseHandle(pre_owned);
+}
+
 TEST(BootstrapUnitTest, ProcessGateMismatchReturnsFalse)
 {
     CallbackSignals sig;
@@ -230,3 +308,55 @@ TEST_F(BootstrapIntegrationTest, HappyPathAttachInitShutdown)
 
     attached = false;
 }
+
+TEST_F(BootstrapIntegrationTest, InitAndShutdownExceptionsAreCaught)
+{
+    // Drain any globals left set by a prior successful attach (HappyPath
+    // leaves g_shutdown_event / g_worker_thread non-null for its design);
+    // this is the first on_dll_detach call in the process and will win
+    // the CAS and clear the static handles.
+    Bootstrap::on_dll_detach(FALSE);
+
+    const std::string exe_name = current_exe_basename();
+    ASSERT_FALSE(exe_name.empty());
+
+    Bootstrap::ModInfo info{};
+    info.prefix = "BS_TEST";
+    info.log_file = "bs_test_throws.log";
+    info.game_process_name = exe_name;
+    info.instance_mutex_prefix = "BS_Test_Mutex_Throws_";
+
+    auto init_fn = [this]() -> bool
+    {
+        sig.init_calls.fetch_add(1, std::memory_order_relaxed);
+        {
+            std::lock_guard lock(sig.m);
+            sig.init_done.store(true, std::memory_order_release);
+        }
+        sig.cv.notify_all();
+        throw std::runtime_error("init failure");
+    };
+
+    auto shutdown_fn = [this]()
+    {
+        sig.shutdown_calls.fetch_add(1, std::memory_order_relaxed);
+        {
+            std::lock_guard lock(sig.m);
+            sig.shutdown_done.store(true, std::memory_order_release);
+        }
+        sig.cv.notify_all();
+        throw std::runtime_error("shutdown failure");
+    };
+
+    const BOOL result = Bootstrap::on_dll_attach(
+        GetModuleHandleW(nullptr), info, init_fn, shutdown_fn);
+    ASSERT_EQ(result, TRUE);
+    attached = true;
+
+    ASSERT_TRUE(sig.wait_for_init(kTestTimeout));
+    EXPECT_EQ(sig.init_calls.load(), 1);
+
+    Bootstrap::request_shutdown();
+    ASSERT_TRUE(sig.wait_for_shutdown(kTestTimeout));
+    EXPECT_EQ(sig.shutdown_calls.load(), 1);
+}