[MAJOR] §5: Multiple test targets disabled without tracking issues

[mvillmow]

Finding

Severity: MAJOR
Section: 5
Evidence: CMakeLists.txt:387-431 (async_agents_tests, chaos_engineering_tests, test_message_bus_async.cpp disabled)
Principle: TDD

async_agents_tests and chaos_engineering_tests are commented out as 'DISABLED: Phase 6/5 features not yet implemented'. test_message_bus_async.cpp is commented out as 'Async API not yet implemented'. These cover async agent behavior and chaos engineering — both core advertised capabilities. No tracking issue numbers are referenced for re-enablement.

---

Part of #504

[mvillmow]

Implementation Plan

Now I have enough information to write a thorough implementation plan.

---

Implementation Plan: Issue #511 — Re-enable Disabled Test Targets

1. Objective

Three test targets and one test source file are commented out in CMakeLists.txt without tracking issue references. The goal is to:

1. Add // TODO(#511) annotations to every disabled block so they are grep-discoverable.
2. Assess whether each target's blocking dependencies now exist in the codebase.
3. Re-enable targets whose dependencies are fully implemented.
4. Leave targets that are still blocked commented out, but with proper annotations referencing issue [MAJOR] §5: Multiple test targets disabled without tracking issues #511.

---

2. Approach

Assessment results from codebase exploration:

Disabled Target	Dependencies	Status
test_message_bus_async (unit)	WorkStealingScheduler, MessageBus async API	ALL EXIST — re-enable
async_agents_tests (e2e)	AsyncAgent, WorkStealingScheduler, TaskAgent	ALL EXIST — re-enable
chaos_engineering_tests (e2e)	FailureInjector, SimulatedNetwork (partition API), keystone_simulation	SimulatedNetwork partition API header EXISTS (createPartition, isPartitioned, etc.) — re-enable
multi_component_tests (e2e)	multi_component_test.cpp source file	FILE MISSING — annotate only

All three test source-file-based targets (test_message_bus_async, async_agents_tests, chaos_engineering_tests) have their required implementation dependencies in the codebase. The multi_component_tests target references a non-existent source file and must remain disabled with a proper annotation.

Strategy:

• Re-enable 3 targets by uncommenting their CMake blocks.
• Annotate the multi_component_tests block with TODO(#511) since multi_component_test.cpp is missing.
• Use commit prefix fix(tests): for re-enablement.
• Verify each target compiles cleanly before marking done.

---

3. Files to Create

None — all test files already exist.

---

4. Files to Modify

CMakeLists.txt — lines 387–444

Four changes, applied in order:

A. Re-enable async_agents_tests (lines 387–394)

Replace the commented-out block:

# E2E Tests - Async Agents (Coroutines) DISABLED: Phase 6 async features not yet
# implemented add_executable(async_agents_tests tests/e2e/async_agents_test.cpp
# tests/e2e/full_async_hierarchy_test.cpp)
#
# target_link_libraries(async_agents_tests keystone_agents keystone_core
# keystone_concurrency GTest::gtest_main)
#
# gtest_discover_tests(async_agents_tests)

With:

# E2E Tests - Async Agents (Coroutines)
# TODO(#511): Re-enabled; AsyncAgent and WorkStealingScheduler are fully implemented
add_executable(async_agents_tests
  tests/e2e/async_agents_test.cpp
  tests/e2e/full_async_hierarchy_test.cpp)
target_link_libraries(async_agents_tests
  keystone_agents keystone_core keystone_concurrency GTest::gtest_main)
gtest_discover_tests(async_agents_tests)

B. Annotate multi_component_tests (lines 415–422) — remains disabled

Replace:

# E2E Tests - Multi-Component Hierarchy DISABLED: Phase 4 multi-component
# features not yet implemented add_executable(multi_component_tests
# tests/e2e/multi_component_test.cpp)
# ...

With:

# E2E Tests - Multi-Component Hierarchy
# TODO(#511): Blocked — tests/e2e/multi_component_test.cpp does not exist.
#             Re-enable once the source file is created.
# add_executable(multi_component_tests tests/e2e/multi_component_test.cpp)
# target_link_libraries(multi_component_tests
#   keystone_agents keystone_core keystone_concurrency GTest::gtest_main)
# gtest_discover_tests(multi_component_tests)

C. Re-enable chaos_engineering_tests (lines 424–431)

Replace the commented-out block with:

# E2E Tests - Chaos Engineering
# TODO(#511): Re-enabled; FailureInjector and SimulatedNetwork partition API exist
add_executable(chaos_engineering_tests
  tests/e2e/chaos_engineering_test.cpp)
target_link_libraries(chaos_engineering_tests
  keystone_agents keystone_core keystone_concurrency
  keystone_simulation GTest::gtest_main)
gtest_discover_tests(chaos_engineering_tests DISCOVERY_TIMEOUT 10)

D. Re-enable test_message_bus_async.cpp in unit_tests (line ~439)

Find:

# DISABLED: Async API not yet implemented
# tests/unit/test_message_bus_async.cpp

Replace with:

# TODO(#511): Re-enabled; MessageBus async API (ISchedulerIntegration) is implemented
tests/unit/test_message_bus_async.cpp

(This adds the file back to the unit_tests executable's source list.)

---

5. Implementation Order

1. Read CMakeLists.txt lines 380–450 — confirm exact formatting and indentation before editing.
2. Re-enable test_message_bus_async.cpp in the unit_tests source list (lowest-risk, unit test).
3. Re-enable async_agents_tests CMake target block.
4. Re-enable chaos_engineering_tests CMake target block.
5. Annotate multi_component_tests with TODO(#511) tracking reference (remains commented out).
6. Build the debug preset (cmake --preset debug && cmake --build --preset debug) to confirm all four targets compile without errors.
7. Run the newly enabled tests (ctest --preset debug -R "async_agents_tests|chaos_engineering_tests|unit_tests" --output-on-failure) to confirm they pass or identify failures needing a follow-up issue.
8. If any tests fail at runtime, do NOT re-disable them. Instead:
   • Keep them enabled (compilation proves the API exists).
   • Open a separate tracking issue for the runtime failure.
   • Add a DISABLED_ prefix on the specific failing TEST_F (GTest's built-in mechanism) with a comment referencing the new issue number.
9. Commit with message: fix(tests): re-enable async, chaos, and message-bus async test targets (#511)

---

6. Verification

# Step 1: Clean configure
cmake --preset debug

# Step 2: Build (must produce zero errors and zero -Werror violations)
cmake --build --preset debug

# Step 3: Run the re-enabled targets
ctest --preset debug -R "unit_tests" --output-on-failure
ctest --preset debug -R "async_agents_tests" --output-on-failure
ctest --preset debug -R "chaos_engineering_tests" --output-on-failure

# Step 4: Confirm multi_component_tests is still absent from CTest list
ctest --preset debug -N | grep multi_component
# Expected: no output

# Step 5: Confirm TODO(#511) annotation exists for multi_component_tests
grep -n "TODO(#511)" CMakeLists.txt
# Expected: at least one match on the multi_component_tests block

Acceptance criteria:

• cmake --build --preset debug exits 0.
• async_agents_tests, chaos_engineering_tests, and unit_tests (including the async file) appear in ctest -N output.
• multi_component_tests does not appear in ctest -N output.
• grep "TODO(#511)" CMakeLists.txt returns the multi_component annotation line.
• No previously-passing tests regress.

---

7. Skills Used

Directly invoked during planning:

• Explore (subagent) — used twice to survey the disabled CMake blocks, locate test source files, and verify whether each dependency (WorkStealingScheduler, AsyncAgent, FailureInjector, SimulatedNetwork, MessageBus async API, multi_component_test.cpp) actually exists in the codebase.

Team knowledge base skills referenced (from Prior Learnings):

• enable-disabled-mojo-tests — guided the "read the actual test file first" and "verify blockers are resolved before enabling" workflow.
• resolve-blocked-todos — informed the decision to verify each blocker is closed before re-enabling, and the approach of opening a follow-up issue rather than re-disabling if tests fail at runtime.
• placeholder-test-staleness-annotations — provided the TODO(#511) annotation pattern for the still-blocked multi_component_tests target, and the guidance to reference the tracking issue number in the comment.
• cmake-library-main-gtest-conflict — context for understanding how GTest target wiring can silently fail; informed the decision to keep test_message_bus_async.cpp as part of the existing unit_tests executable rather than creating a new target.

---

Generated by Claude Code Planner