tas/Codewalkers
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test: Remove redundant and dead tests (-743 lines)

Browse Source 
Delete 3 files:
- completion-detection.test.ts (private method tests, covered by crash-race-condition)
- completion-race-condition.test.ts (covered by mutex-completion + crash-race-condition)
- real-e2e-crash.test.ts (dead: expect(true).toBe(true), hardcoded paths)

Remove individual tests:
- crash-race-condition.test.ts #4 (weaker duplicate of #2)
- mock-manager.test.ts duplicate "(second test)" for detail_complete
- process-manager.test.ts 2 "logs comprehensive" tests with empty assertions
- edge-cases.test.ts 2 Q&A tests redundant with recovery-scenarios

Update test-inventory.md to reflect removals.
This commit is contained in:
Lukas May
2026-03-02 12:57:27 +09:00
parent a2ab4c4a84
commit 1540039c52
8 changed files with 15 additions and 743 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
docs/test-inventory.md
View File

@@ -2,7 +2,7 @@
Comprehensive catalog of every test in the suite, what it verifies, and where coverage is weak or redundant.
**44 test files** | **35 unit** | **7 E2E** | **4 integration (mocked)** | **5 integration (real providers)**
**41 test files** | **33 unit** | **7 E2E** | **3 integration (mocked)** | **5 integration (real providers)**
Last audited: 2026-03-02
@@ -93,35 +93,7 @@ Subjective 0–10 scores. 0 = no coverage, 5 = happy paths only, 10 = comprehens
---
#### `completion-detection.test.ts` — readSignalCompletion (private method)
| Test | Verifies |
|------|----------|
| detects "questions" status | signal.json parsed, returns content |
| detects "done" status | Same |
| detects "error" status | Same |
| returns null when no signal.json | Missing file returns null |
| returns null for "running" status | Non-terminal status ignored |
| handles malformed signal.json | Bad JSON returns null |
**Concern**: All 6 tests access private method via `(outputHandler as any).readSignalCompletion`. Real filesystem (good). **Partially redundant with `completion-race-condition.test.ts` tests 2-3** which test the same scenarios through the public `handleCompletion` method.
---
#### `completion-race-condition.test.ts` — handleCompletion mutex
| Test | Verifies |
|------|----------|
| prevent concurrent completion via mutex | Only 1 of 2 concurrent calls executes |
| handle completion with questions status | Final status `waiting_for_input` |
| handle completion when done | Final status `idle` |
| clean up lock even on failure | Mutex released after error |
**Concern**: Module-level mock repo mutated per-test without `beforeEach` reset. Tests 2-3 overlap with `completion-detection.test.ts` but are strictly better (test public API).
---
#### `mutex-completion.test.ts` — handleCompletion mutex (focused)
#### `mutex-completion.test.ts` — handleCompletion mutex
| Test | Verifies |
|------|----------|
@@ -130,7 +102,7 @@ Subjective 0–10 scores. 0 = no coverage, 5 = happy paths only, 10 = comprehens
| clean up on exception | Lock released on error |
| per-agent mutex isolation | Different agentIds run concurrently |
**Excellent focused design.** 4 complementary scenarios. Uses real timers (50ms delays). **Partial overlap with `completion-race-condition.test.ts` test 1** — both test the mutex concurrent blocking.
**Excellent focused design.** 4 complementary scenarios. Uses real timers (50ms delays).
---
@@ -172,28 +144,26 @@ Subjective 0–10 scores. 0 = no coverage, 5 = happy paths only, 10 = comprehens
| getAgentWorkdir | Path computation |
| createProjectWorktrees | Worktree creation for initiative projects |
| createProjectWorktrees / throws on failure | Error propagation |
| createProjectWorktrees / logs details | Completes without error (weak) |
| createStandaloneWorktree | Path returned |
| createStandaloneWorktree / throws on failure | Error propagation |
| spawnDetached / validates cwd exists | existsSync called |
| spawnDetached / throws if cwd missing | Error thrown |
| spawnDetached / passes correct cwd | spawn args verified |
| spawnDetached / logs spawn info | Completes (weak) |
| spawnDetached / writes prompt file | PROMPT.md written |
| buildSpawnCommand / native prompt mode | Command+args assembled |
| buildSpawnCommand / flag prompt mode | Command+args assembled |
| buildResumeCommand / flag style | Resume args assembled |
| buildResumeCommand / no resume support | Error thrown |
**6 `vi.mock()` calls.** Most brittle test file. "Logs comprehensive" tests are empty assertions. Missing: `killProcess`, `cleanupWorktree`.
**6 `vi.mock()` calls.** Most brittle test file. Missing: `killProcess`, `cleanupWorktree`.
---
#### `mock-manager.test.ts` — MockAgentManager (testing the test double)
39 tests covering: default scenario, configured delay, error/questions scenarios, resume, stop, list, get/getByName, scenario overrides, event emission order, name uniqueness, constructor options, agent modes (execute/discuss/plan/detail), structured questions.
43 tests covering: default scenario, configured delay, error/questions scenarios, resume, stop, list, get/getByName, scenario overrides, event emission order, name uniqueness, constructor options, agent modes (execute/discuss/plan/detail), structured questions.
**Thorough.** One duplicate: "should emit stopped event with detail_complete reason" appears twice with "(second test)" suffix.
**Thorough.**
---
@@ -319,7 +289,7 @@ All use `createTestHarness()` with in-memory DB, MockAgentManager, MockWorktreeM
**`recovery-scenarios.test.ts`** — 9 tests. Queue state in DB, in-progress task recovery after crash, blocked state persistence, merge queue recovery, agent Q&A (question/resume, structured questions, status transitions, multiple questions).
**`edge-cases.test.ts`** — 14 tests. Agent crash (events, task status, error message), agent waiting/resume (3 tests), task blocking (4 tests), merge conflict handling (4 tests).
**`edge-cases.test.ts`** — 12 tests. Agent crash (events, task status, error message), agent waiting (1 test), task blocking (4 tests), merge conflict handling (4 tests).
**`extended-scenarios.test.ts`** — 6 tests. Conflict hand-back round-trip (full lifecycle, context preservation, sequential conflicts), multi-agent parallel work (parallel completion, merge ordering, mixed outcomes).
@@ -329,12 +299,10 @@ All use `createTestHarness()` with in-memory DB, MockAgentManager, MockWorktreeM
**`agent-workdir-verification.test.ts`** — 2 tests. Skipped by default (`REAL_WORKDIR_TESTS`). Spawns real Claude agent, verifies working directory path via diagnostic files and agent-created files.
**`crash-race-condition.test.ts`** — 4 tests. Always runs. Tests signal.json-based completion detection vs crash marking. Tests 2 and 4 overlap (same setup, test 4 has weaker assertions). Accesses private methods via `as any`.
**`crash-race-condition.test.ts`** — 3 tests. Always runs. Tests signal.json-based completion detection vs crash marking. Covers questions, no-signal crash, and slim-wildebeest scenario. Accesses private methods via `as any`.
**`real-claude.test.ts`** — 3 tests. Skipped by default. Direct Claude CLI contract tests: done/questions/error status outputs validated against Zod schema. ~$0.10.
**`real-e2e-crash.test.ts`** — 1 test. **Dead test**: `expect(true).toBe(true)`. Hardcoded absolute paths. Diagnostic script, not a regression test.
---
### Real Provider Tests (`src/test/integration/real-providers/`)
@@ -353,24 +321,15 @@ All skipped by default. ~$0.50 total per full run.
---
## Redundancy Map
## Remaining Redundancy
Tests covering the same scenario multiple times. **Bold** = the better version to keep.
Tests covering the same scenario multiple times.
| Scenario | Tests |
|----------|-------|
| Signal.json completion detection (questions→waiting) | `completion-detection.test.ts` #1, **`completion-race-condition.test.ts` #2** |
| Signal.json completion detection (done→idle) | `completion-detection.test.ts` #2, **`completion-race-condition.test.ts` #3** |
| Mutex concurrent blocking | `completion-race-condition.test.ts` #1, **`mutex-completion.test.ts` #1** (more focused) |
| Mutex cleanup on error | `completion-race-condition.test.ts` #4, **`mutex-completion.test.ts` #3** (more focused) |
| Agent Q&A resume flow | `edge-cases.test.ts` #5, **`recovery-scenarios.test.ts` #5** (has options) |
| Agent status transitions (running→waiting→idle) | `edge-cases.test.ts` #6, **`recovery-scenarios.test.ts` #8** (more thorough) |
| Task blocking (DB status) | `edge-cases.test.ts` #10, `recovery-scenarios.test.ts` #3 |
| Merge conflict → resolution task | `edge-cases.test.ts` #13, **`extended-scenarios.test.ts` #1** (full round-trip) |
| Task blocking (DB status) | `edge-cases.test.ts`, `recovery-scenarios.test.ts` |
| Merge conflict → resolution task | `edge-cases.test.ts`, `extended-scenarios.test.ts` (full round-trip) |
| Conflict resolution task creation | `coordination/manager.test.ts` (5 tests), `conflict-resolution-service.test.ts` (10 tests) |
| MockAgentManager detail_complete reason | `mock-manager.test.ts` has duplicate test "(second test)" |
| Crash marking with no signal | `crash-race-condition.test.ts` #2, `crash-race-condition.test.ts` #4 (weaker) |
| `real-e2e-crash.test.ts` #1 | Dead: `expect(true).toBe(true)` — contributes nothing |
---
@@ -417,8 +376,6 @@ Tests covering the same scenario multiple times. **Bold** = the better version t
- **`agent/process-manager.test.ts`** — 6 `vi.mock()` calls. Any import restructuring breaks tests.
- **`agent/manager.test.ts`** — 5 `vi.mock()` calls. Same concern.
- **`preview/manager.test.ts`** — 6 `vi.mock()` calls. Same concern.
- **`completion-detection.test.ts`** — Tests private method `readSignalCompletion` via `as any`.
- **`completion-race-condition.test.ts`** — Monkey-patches private methods. Module-level mock mutated without reset.
- **`crash-race-condition.test.ts`** — Accesses private `handleCompletion` via `as any`.
### Low Fragility (behavioral tests)
@@ -441,19 +398,7 @@ The test suite uses 4 different mocking patterns with no shared convention:
|---------|-------|---------|
| `vi.mock()` module-level | manager.test.ts, process-manager.test.ts, preview/manager.test.ts | Import-structure coupled |
| Factory function (`createMockX()`) | output-handler.test.ts, coordination/*.test.ts, server/*.test.ts | Cleaner but copy-pasted |
| Inline const object | completion-race-condition.test.ts, mutex-completion.test.ts | Simple but mutated per-test |
| Inline const object | mutex-completion.test.ts | Simple but mutated per-test |
| Hand-rolled class | harness.ts (MockWorktreeManager, CapturingEventBus) | Best for shared infrastructure |
`createMockEventBus()` is independently defined in 5+ files. Should be extracted to shared test utility.
---
## Dead / Worthless Tests
| Test | File | Reason |
|------|------|--------|
| `expect(true).toBe(true)` | `real-e2e-crash.test.ts` | Can never fail. Hardcoded paths. |
| "logs comprehensive spawn information" | `process-manager.test.ts` | Asserts nothing meaningful |
| "logs comprehensive worktree creation details" | `process-manager.test.ts` | Asserts nothing meaningful |
| "should demonstrate the race condition exists" | `crash-race-condition.test.ts` #4 | Only asserts `updateCalls.length > 0` |
| "should emit stopped event (second test)" | `mock-manager.test.ts` | Exact duplicate of previous test |