---
name: testability-review
description: Reviews code for testability issues. Identifies hard-to-test constructs, hidden dependencies, and missing seams — then suggests concrete changes to make the code testable without rewriting it.
metadata:
  author: janmarkuslanger
  version: "1.0"
---

# Testability Review

When this skill is activated, analyze the specified code for testability and produce a `TESTABILITY_REVIEW.md` report. The goal is not to write tests — it is to identify what makes code *hard* to test and recommend targeted structural changes.

## Clarifying Questions

Before starting the review, ask the engineer the following questions. Skip any already clearly answered in the conversation. **Wait for the answers before proceeding.**

1. What is the scope — a specific module or feature, or the full codebase?
2. What is the current testing strategy — what types of tests exist today (unit, integration, e2e), and what is missing?
3. Are there specific components or areas known to be difficult or impossible to test?
4. Are there constraints on refactoring — e.g. public APIs that cannot change, no new dependencies allowed, performance-sensitive paths?

## Steps

1. Use the answers above to set scope and focus; fall back to full codebase only if scope was not specified
2. Read the relevant source files
3. Identify testability smells: hidden dependencies, static calls, global state, missing interfaces, logic buried in constructors, etc.
4. For each issue, assess the impact on test quality (false positives, slow tests, untestable branches)
5. Recommend the minimal structural change needed to make each piece testable — prefer surgical fixes over rewrites

## Output

Write a file called `TESTABILITY_REVIEW.md` in the project root with the following sections:

### 1. Scope
What was analyzed.

### 2. Testability Score
A quick overview table per analyzed component:

| Component | Testability | Biggest Blocker |
|-----------|-------------|----------------|
| OrderService | Poor | Direct DB instantiation in constructor |
| PriceCalculator | Good | None significant |

**Testability**: `Good` | `Fair` | `Poor` | `Untestable`

### 3. Issues Found
For each issue, one entry:

**Issue: {Short Name}**
- **Location**: file path + line number(s)
- **Pattern**: which testability smell this is (see list below)
- **Impact**: what kind of tests this prevents or degrades
- **Recommended fix**: the minimal code change — injection, interface extraction, seam introduction, etc. Show a before/after snippet if it helps.

**Testability smell patterns to check for:**
- `hidden-dependency`: dependency created inside the class (new X(), singletons, static calls) rather than injected
- `global-state`: shared mutable state (globals, module-level singletons, class-level statics)
- `logic-in-constructor`: business logic or I/O in constructors makes setup in tests expensive
- `no-interface`: concrete class used everywhere instead of an interface, blocking substitution in tests
- `non-determinism`: time, randomness, or network calls embedded without abstraction
- `missing-seam`: no injection point exists where a test double could be inserted
- `large-unit`: component does too many things, making isolation testing impossible without setting up unrelated state
- `private-everything`: relevant behavior is untestable because it is entirely buried in private methods with no public surface

### 4. Test Strategy Recommendation
Based on the issues found, what test strategy makes sense:

- **Unit tests**: which components are suitable for pure unit testing, and what needs to be stubbed
- **Integration tests**: which couplings are best validated with real collaborators
- **What to avoid**: test patterns that will produce brittle, slow, or misleading tests given the current structure

### 5. Prioritized Fix List
Order the issues by effort vs. impact:

| Issue | Effort | Impact | Priority |
|-------|--------|--------|----------|
| Inject DB dependency in OrderService | Low | High | Do first |

## Rules

- Focus on structural issues that block testing, not on missing test coverage
- Recommended fixes must preserve existing behavior — this is about making code *testable*, not refactoring for other reasons
- Do not recommend rewriting a component unless it is genuinely untestable without it
- Always include file paths and line numbers — vague findings are not actionable
- Distinguish between "hard to test" (worth fixing) and "inconvenient to test" (tolerable)