cwilliams5/review-test-quality

Enter planning mode. Audit the AHK test suite for quality issues. Use maximum parallelism — spawn explore agents for independent test files.

Scope: AHK tests only — unit tests, GUI tests, live tests, lifecycle tests in tests/. NOT static analysis checks, NOT query tools, NOT test coverage (this is about quality of existing tests, not finding gaps).

Priority Issue — Production Code Copies

This is the highest-priority defect. Past reviews have found tests that copy-paste production logic into the test file instead of #Include-ing the production file. These tests always pass because they test the copy, not production. When production changes, the copy doesn't update — the test is useless.

Detection pattern: A test file that defines a function with the same name as a production function, but does NOT #Include the production file containing it. The test's "function" is a frozen copy.

For each test file, check:

1. What production files does it #Include?
2. Does it define any functions that shadow production functions? (Compare against query_function_visibility.ps1 to find the real definition)
3. Is the test calling real production code, or its own copy?

A test that defines mocks (GUI rendering, IPC sends, DWM calls) is fine — that's intentional shadowing per testing.md. The problem is when business logic is copied instead of included.

Other Quality Issues

Dead tests

Tests that can never fail regardless of production behavior:

• Assertions that test hardcoded values against themselves
• Tests that set up state and assert on the setup, not on production function output
• Tests where the "expected" value is computed by the same logic being tested

Duplicative tests

Multiple tests verifying the exact same behavior:

• Same function called with same inputs and same assertions, in different test files
• A unit test and a live test that test identical logic (the unit test is sufficient for pure logic; the live test should test integration aspects the unit test can't)

Broken assertions

Tests that appear to assert but don't actually fail on wrong values:

• Missing TestErrors++ after a failed comparison
• Conditions that log FAIL but don't increment the error counter
• Assertions comparing wrong variables (e.g., comparing input to input instead of input to output)

Stale tests

Tests for features or code paths that no longer exist:

• Tests for removed functions (function deleted from production, test still "passes" because it tests its own copy or mock)
• Tests for old behavior that was intentionally changed

Constraints

The test suite is designed for automated agentic execution — no user interaction. Any suggested modifications must follow existing patterns from test_utils.ahk and respect the parallel, timing-critical design. See testing.md for the full pattern guide.

Validation

After explore agents report back, validate every finding yourself. Test quality issues are subtle — what looks like a copy may actually be an intentional mock, and what looks dead may test an edge case.

For each candidate:

1. Cite evidence: "I verified by reading test_file.ahk lines X–Y and production_file.ahk lines A–B" — show both sides.
2. Trace the test path: Does the test call the real production function, or a local copy? Use query_function_visibility.ps1 to confirm where the function is defined.
3. Counter-argument: "What would make this test valuable despite appearances?" — Is it testing a subtle edge case? Is the "copy" actually an intentional mock? Would removing it lose coverage of an important path?
4. Observed vs inferred: Did you confirm the test calls a copy (by reading both files), or infer it from the absence of an #Include?

Plan Format

Section 1 — Production code copies (highest priority):

| Test File | Lines | Function Copied | Production Source | Fix | |-----------|-------|----------------|------------------|-----| | test_foo.ahk | 10–25 | CalculateScore() | src/shared/scoring.ahk:42 | Replace copy with #Include, add mock for GUI dependency |

Section 2 — Dead/broken tests:

| Test File | Lines | Issue | Evidence | Fix | |-----------|-------|-------|----------|-----| | test_bar.ahk | 88–95 | Asserts setup value, not production output | Expected value is hardcoded 3, same as setup | Rewrite to call production function and assert result |

Section 3 — Duplicative tests:

| Test A | Test B | Overlap | Recommendation | |--------|--------|---------|---------------| | test_unit_x.ahk:30 | test_live_y.ahk:55 | Both test ParseConfig() with same inputs | Keep unit test, refocus live test on integration behavior |

Section 4 — Stale tests (testing removed features):

| Test File | Lines | What It Tests | Status | Fix | |-----------|-------|--------------|--------|-----| | test_old.ahk | 1–50 | LegacyFormat() — removed in v0.7 | Always passes (tests own copy) | Delete |

Order by impact: production copies first (actively hiding bugs), then dead tests, then duplicative, then stale.

Ignore any existing plans — create a fresh one.