Private
Public Access
0
0

conductor(track): superpowers review section 4 — test-driven-development (deep-dive)

This commit is contained in:
2026-07-05 13:14:26 -04:00
parent ef80a60965
commit e41a79e33c
@@ -186,7 +186,64 @@ The project follows the discipline with one architectural difference (4-tier hie
## 4. Test-Driven Development
<!-- Section 4 deep-dive (200-400 LOC). Skill: test-driven-development. Verdict pending. -->
## 4. Test-Driven Development
### 4.1 What the skill prescribes
The `test-driven-development` skill is rigid — explicitly classified as "Rigid (TDD, debugging): Follow exactly. Don't adapt away discipline" per the skill's self-classification. The Iron Law is unambiguous: "NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST." Violating the letter of the rules is violating the spirit of the rules. The Red-Green-Refactor cycle is the only acceptable flow: RED (write failing test) → Verify RED (watch it fail correctly) → GREEN (minimal code to pass) → Verify GREEN (all green) → REFACTOR (clean up, keep tests green) → Next. The skill's "common rationalizations" table is exhaustive ("Too simple to test", "I'll test after", "Deleting X hours is wasteful", "TDD is dogmatic, being pragmatic means adapting") with explicit counters. The verification checklist is mandatory: every function has a test, watched each test fail, minimal code to pass, all tests pass, output pristine, real code (not mocks), edge cases covered.
### 4.2 Mapping to the project's existing pattern
The project has TDD as a load-bearing discipline with extensive elaboration. The project's TDD enforcement is multi-layered:
| TDD skill rule | Project equivalent | Where |
|---|---|---|
| Iron Law: no production code without failing test | `conductor/workflow.md` §"Task Workflow" step 4 "Write Failing Tests (Red Phase)": "CRITICAL: Run the tests and confirm that they fail as expected. This is the 'Red' phase of TDD. Do not proceed until you have failing tests." | `conductor/workflow.md` step 4 |
| Red-Green-Refactor cycle | Same — step 4 = Red, step 5 = "Implement to Pass Tests (Green Phase)", step 6 = Refactor (optional but recommended) | `conductor/workflow.md` steps 4-6 |
| "Watch it fail" | Explicit: "Run the tests and confirm that they fail as expected" | `conductor/workflow.md` step 4 |
| "No exceptions without partner permission" | `AGENTS.md` Critical Anti-Patterns: "Do not skip TDD - write failing tests before implementing functionality" | `AGENTS.md` "Critical Anti-Patterns" |
| "Delegation: tier-3 worker writes the test" | `conductor/workflow.md` step 4 + step 5: "**Delegate Test Creation:** Do NOT write test code directly. Spawn a Tier 3 Worker via the OpenCode Task tool with `subagent_type: 'tier3-worker'` and a **surgical prompt** specifying WHERE (file:line range), WHAT (test to create), HOW (which assertions/fixtures to use), and SAFETY (thread constraints if applicable)." | `conductor/workflow.md` steps 4-5 |
| "Mocking anti-patterns" | `conductor/product-guidelines.md` "Testing Requirements" → "Structural Testing Contract": "Ban on arbitrary core mocking with `unittest.mock.patch` (unless explicitly authorized for a specific boundary test). All integration and end-to-end testing must use the `live_gui` fixture." | `conductor/product-guidelines.md` "Testing Requirements" |
| "Verification Checklist" (test fails for expected reason, etc.) | `conductor/product-guidelines.md` "Quality Gates" + the Phase Completion Verification and Checkpointing Protocol | `conductor/workflow.md` "Phase Completion Verification" |
| "Skip markers are documentation, not avoidance" | `conductor/workflow.md` "Skip-Marker Policy" — explicit policy with review checklist; `AGENTS.md` "Do not use `@pytest.mark.skip` as an excuse to AVOID fixing the underlying bug" | `conductor/workflow.md` "Skip-Marker Policy" + `AGENTS.md` |
| "Debugging Integration: bug → write failing test reproducing it" | `conductor/workflow.md` "systematic-debugging" reference + the verification-before-completion pattern | `conductor/workflow.md` + `AGENTS.md` |
| "Live GUI testing" (the project's elaboration) | `tests/conftest.py` defines 7 conftest fixtures (`isolate_workspace`, `reset_paths`, `reset_ai_client`, `vlogger`, `kill_process_tree`, `mock_app`, `live_gui`); the `live_gui` fixture is session-scoped and uses the Hook API for end-to-end testing | `tests/conftest.py` + `docs/guide_testing.md` |
### 4.3 Where the project already follows the discipline
- **Every track on the project follows the Red-Green-Refactor cycle.** Examples from the chronology: `mcp_architecture_refactor_20260606` (Phase 1 = audit tests pass = baseline; Phase 2 = write new tests; Phase 3 = refactor; etc.); `data_oriented_error_handling_20260606` (Phase 1 = baseline green; subsequent phases add tests before refactoring); `default_layout_install_20260629` (Phase 1 = failing test for "panels invisible on empty INI"; subsequent phases = minimal fix).
- **The "don't keep code as reference" rule is enforced.** The user's `cruft_elimination_20260627` track explicitly removed legacy code without TDD justification; the `default_layout_install_followup_20260629` track added failing tests for stale INI handling.
- **The "no mocks for core infrastructure" rule is enforced** via the audit script `scripts/audit_main_thread_imports.py` (main-thread-purity invariant) and the Structural Testing Contract (`live_gui` standard).
- **The "verification before completion" discipline** is enforced via the Phase Completion Verification and Checkpointing Protocol (must run automated tests + API hooks + user confirmation before checkpoint).
- **The 251 test files** (per `docs/guide_testing.md`) are evidence of the project's TDD commitment.
### 4.4 Where the project doesn't follow the discipline
- **"Throwaway prototypes" exception** is acknowledged in the skill but the project doesn't have an explicit "prototype" affordance. The TDD skip is rare in the project; when it happens (e.g., the `cruft_elimination_20260627` track's audit-script work), it's documented as an exception.
- **"Generated code" exception** — n/a for this project (no code generation from LLMs to runtime code).
- **"Configuration files" exception** — partially followed. TOML config changes (`presets.toml`, `personas.toml`, `manual_slop.toml`) are made without TDD in some tracks (e.g., `presets_ai_settings_ux_20260311`); the convention is that config changes don't need new tests if the existing config-loading tests still pass.
- **"No exceptions without partner permission"** — partially followed. The TDD skill's exception list is small ("Throwaway prototypes", "Generated code", "Configuration files") and each requires partner permission. The project has more flexible exception usage; the partner is the user, but the user doesn't have to give explicit permission for each config-file change.
- **The skill's "Test passes immediately → you're testing existing behavior, fix test"** rule has surfaced in the project as the `isolation-pass verification fallacy` (per `conductor/workflow.md` §"Isolated-Pass Verification Fallacy"): tests that pass in isolation but fail in batch. The project has a dedicated rule for this; the skill does not.
### 4.5 Recommendations summary
The project follows the TDD discipline strictly. The four elaborations (tier-3-worker delegation; live_gui standard; skip-marker policy; isolated-pass verification rule) go *beyond* the skill. The deferred rebuild may want to:
- **LOW:** Add a note to `conductor/workflow.md` §"Task Workflow" step 4 + step 5 that the "tier-3 worker dispatches the failing test" rule is the project's elaboration of the skill's "write the test first" rule. This is documentation, not a behavior change.
- **LOW:** Codify the "configuration files" exception explicitly. Currently the project applies this exception case-by-case; codifying the rule would reduce ambiguity.
**Verdict.**
| Field | Value |
|---|---|
| **Primary** | `PARITY` |
| **Integration tag** | `INTEGRATED` |
| **Section size** | deep-dive |
| **Cross-refs** | nagent_review_20260608 §10 ("durable tests" — nagent's principle that tests are durable artifacts); fable_review_20260617 §13 ("Genuinely Useful Patterns" includes the project's testing discipline); intent_dsl_survey_20260612 §6 (the survey's "AI-agent properties" include the project's testing rigor) |
**Rationale.** The project's 12-step task workflow + AGENTS.md "Critical Anti-Patterns" + Structural Testing Contract is a faithful re-encoding of the TDD skill with four project-specific elaborations. The iron law is enforced via step 4's "Do not proceed until you have failing tests" rule. The Red-Green-Refactor cycle is the workflow steps 4-6.
**Recommended change.** *(blank — no rebuild action. The project is in alignment.)*
## 5. Verification Before Completion