Test Review

Table of Contents

• Quick Start
• When to Use
• Required TodoWrite Items
• Progressive Loading
• Workflow
• Step 1: Detect Languages (`test-review:languages-detected`))
• Step 2: Inventory Coverage (`test-review:coverage-inventoried`))
• Step 3: Assess Scenario Quality (`test-review:scenario-quality`))
• Step 4: Plan Remediation (`test-review:gap-remediation`))
• Step 5: Log Evidence (`test-review:evidence-logged`))
• Test Quality Checklist (Condensed))
• Output Format
• Summary
• Framework Detection
• Coverage Analysis
• Quality Issues
• Remediation Plan
• Recommendation
• Integration Notes
• Exit Criteria

Test Review Workflow

Evaluate and improve test suites with TDD/BDD rigor.

Quick Start

/test-review

Verification: Run pytest -v to verify tests pass.

When To Use

• Reviewing test suite quality
• Analyzing coverage gaps
• Before major releases
• After test failures
• Planning test improvements

When NOT To Use

• Writing new tests - use parseltongue:python-testing
• Updating existing tests - use sanctum:test-updates

Required TodoWrite Items

1. test-review:languages-detected 2. test-review:coverage-inventoried 3. test-review:scenario-quality 4. test-review:invariant-preservation 5. test-review:gap-remediation 6. test-review:evidence-logged 7. test-review:findings-verified

Progressive Loading

Load modules as needed based on review depth:

• Basic review: Core workflow (this file)
• Framework detection: Load modules/framework-detection.md
• Coverage analysis: Load modules/coverage-analysis.md
• Quality assessment: Load modules/scenario-quality.md
• Remediation planning: Load modules/remediation-planning.md

Workflow

Step 1: Detect Languages (test-review:languages-detected)

Identify testing frameworks and version constraints. → See: modules/framework-detection.md

Quick check:

find . -maxdepth 2 -name "Cargo.toml" -o -name "pyproject.toml" -o -name "package.json" -o -name "go.mod"

Verification: Run the command with --help flag to verify availability.

Step 2: Inventory Coverage (test-review:coverage-inventoried)

Run coverage tools and identify gaps. → See: modules/coverage-analysis.md

Quick check:

git diff --name-only | rg 'tests|spec|feature'

Verification: Run pytest -v to verify tests pass.

Step 3: Assess Scenario Quality (test-review:scenario-quality)

Evaluate test quality using BDD patterns and assertion checks. → See: modules/scenario-quality.md

Focus on:

• Given/When/Then clarity
• Assertion specificity
• Anti-patterns (dead waits, mocking internals, repeated boilerplate)

Step 4: Plan Remediation (test-review:gap-remediation)

Create concrete improvement plan with owners and dates. → See: modules/remediation-planning.md

Step 5: Log Evidence (test-review:evidence-logged)

Record executed commands, outputs, and recommendations. → See: imbue:proof-of-work

Test Quality Checklist (Condensed)

• [ ] Clear test structure (Arrange-Act-Assert)
• [ ] Critical paths covered (auth, validation, errors)
• [ ] Specific assertions with context
• [ ] No flaky tests (dead waits, order dependencies)
• [ ] Reusable fixtures/factories
• [ ] Invariant-encoding tests intact (see below)

Invariant-Encoding Tests

Tests encode design invariants as well as verifying behavior. A test that asserts "module A never imports from module B" encodes a layer boundary. A test that asserts "this function is pure" encodes a concurrency model. These tests are load-bearing in ways that coverage metrics cannot capture.

During review, check:

1. Were invariant-encoding tests removed or weakened? A test that enforced an architectural boundary, data structure constraint, or API contract should not be deleted without naming the invariant being abandoned and escalating to human judgment.

2. Were test expectations changed to match a broken implementation? If an assertion value changed, ask: did the requirement change, or did the agent change the test to make its code pass? The latter is the single most dangerous form of test tampering.

3. Are new invariants encoded as tests? When a design decision is made (choice of data structure, module boundary, error strategy), there should be at least one test whose failure would signal that the invariant was violated.

Red flag patterns:

When invariant erosion is detected:

Do NOT approve. Flag as a BLOCKING quality issue and present the three options to the human:

1. Preserve: Revert the test change, fix the implementation to satisfy the invariant 2. Layer: Keep the invariant test, add the new behavior alongside it (accepting inelegance) 3. Revise: The invariant is genuinely wrong; remove the old test AND write a new test encoding the replacement invariant

This is a judgment call that models get wrong far too often. Default to option 1 (preserve) when no human is available.

Output Format

## Summary
[Brief assessment]

## Framework Detection
- Languages: [list] | Frameworks: [list] | Versions: [constraints]

## Coverage Analysis
- Overall: X% | Critical: X% | Gaps: [list]

## Quality Issues
[Q1] [Issue] - Location - Anchor: `verbatim source text at file:line` - Fix

## Remediation Plan
1. [Action] - Owner - Date

## Recommendation
Approve / Approve with actions / Block

Verification: Run the command with --help flag to verify availability.

Integration Notes

• Use imbue:proof-of-work for reproducible evidence capture
• Reference imbue:diff-analysis for risk assessment
• Format output using imbue:structured-output patterns

Verify Findings Are Grounded (test-review:findings-verified)

Every finding must cite a real location and a verbatim anchor. Write findings to .review/findings.json and confirm each citation resolves:

python plugins/imbue/scripts/citation_verifier.py \
  --findings .review/findings.json --repo-root .

Drop or label UNVERIFIED any finding the verifier fails (exit 1); only verified findings enter the report. See Skill(imbue:review-core) Step 5 and Skill(imbue:structured-output) for the schema.

Exit Criteria

• Frameworks detected and documented
• Coverage analyzed and gaps identified
• Scenario quality assessed
• Remediation plan created with owners and dates
• Evidence logged with citations
• Every reported finding carries a Location + verbatim Anchor confirmed

by citation_verifier.py (exit 0), or unverified findings were dropped or labeled UNVERIFIED

Troubleshooting

Common Issues

Tests not discovered Ensure test files match pattern test_*.py or *_test.py. Run pytest --collect-only to verify.

Import errors Check that the module being tested is in PYTHONPATH or install with pip install -e .

Async tests failing Install pytest-asyncio and decorate test functions with @pytest.mark.asyncio

Content Assertion Quality

Scoring criteria for evaluating content assertion tests during test review. Extends the scenario quality assessment with a Content Depth dimension.

Reference: leyline:testing-quality-standards/modules/content-assertion-levels.md

Content Depth Scoring

Rate content assertion depth on a 1-5 scale:

When to Flag Missing Content Assertions

During test review, flag as a content test gap when:

• A skill has tests but all are L1 (keyword-only) and the skill contains JSON or YAML code blocks
• A skill has version-gated features but no cross-reference validation
• A skill defines behavioral guidance (decision trees, strategies) but no anti-pattern or completeness tests
• A module documents forbidden behaviors but no test asserts their absence

Content Assertion Anti-Patterns

Avoid these when reviewing content tests:

Review Checklist Addition

Add this item to the existing Test Quality Checklist when reviewing a plugin that has execution markdown:

- [ ] Content assertion depth matches content complexity
      (L1 for simple skills, L2+ for code examples, L3 for behavioral guidance)

Remediation Guidance

When content tests are missing or insufficient:

1. No content tests at all: Generate L1 scaffolding using sanctum:test-updates/modules/generation/content-test-templates.md 2. L1 only, has code blocks: Upgrade to L2 (add JSON/YAML parsing tests) 3. L2 only, has version gates: Upgrade to L3 (add cross-reference validation) 4. L2 only, has behavioral guidance: Upgrade to L3 (add anti-pattern and completeness tests)

Coverage Analysis

Measure test coverage and identify gaps.

Coverage Tools by Language

Rust

# Using tarpaulin
cargo install cargo-tarpaulin
cargo tarpaulin --out Html --output-dir coverage/

# Using llvm-cov
cargo install cargo-llvm-cov
cargo llvm-cov --html

Python

# Using pytest-cov
pytest --cov=src --cov-report=html --cov-report=term-missing

# Using coverage.py
coverage run -m pytest
coverage html
coverage report --show-missing

JavaScript/TypeScript

# Jest
npm test -- --coverage --coverageReporters=html text

# Vitest
vitest --coverage

# Cypress (code coverage plugin)
cypress run --env coverage=true

Go

# Built-in coverage
go test -cover ./...
go test -coverprofile=coverage.out ./...
go tool cover -html=coverage.out

# Detailed coverage
go test -covermode=count -coverprofile=coverage.out ./...

Coverage Thresholds

Gap Identification

Find impacted test files

# Tests affected by changes
git diff --name-only main...HEAD | rg 'tests|spec|feature'

# Find related tests
git diff --name-only main...HEAD | while read file; do
  basename "$file" .py | xargs -I {} find . \
    -not -path "*/.venv/*" -not -path "*/__pycache__/*" \
    -not -path "*/node_modules/*" -not -path "*/.git/*" \
    -name "*test*{}*"
done

Identify uncovered code

1. Run coverage tool with --show-missing flag 2. Cross-reference with critical paths:

• Authentication/authorization
• Data validation
• Error handling
• API endpoints
• Database operations

3. Map to requirements:

• Feature specifications
• User stories
• Bug reports
• Security requirements

Coverage Patterns

Critical paths (should be 100%):

• Security boundaries (auth, validation)
• Data integrity operations
• Error recovery logic
• Public API surface

Lower priority (can be <80%):

• Internal helpers
• Logging/debugging code
• Trivial getters/setters
• Deprecated code paths

Output Format

## Coverage Analysis
- **Overall**: 78%
- **Critical paths**: 92%
- **Changed files**: 85%

### Gaps Identified
1. **src/auth.py:45-60** - Token validation edge cases
2. **src/api/routes.py:120-135** - Error handling for 400/500 codes
3. **src/db/migrations.py** - Rollback scenarios untested

### Test-to-Feature Mapping
- Feature: User registration → `tests/test_registration.py` (95%)
- Feature: Password reset → `tests/test_auth.py` (60%) [WARN]
- Feature: Email validation → Missing tests [FAIL]

Best Practices

1. Branch coverage over line coverage when available 2. Mutation testing for critical code (e.g., cargo mutants, mutmut) 3. Coverage trends: Track over time, not just absolute values 4. Exclude generated code: Focus on hand-written logic 5. Integration coverage: Don't just unit test in isolation

Framework Detection

Identify testing frameworks and tooling constraints.

Language Detection Patterns

Rust

• Framework: cargo test (built-in)
• Commands: cargo test, cargo nextest run
• Config files: Cargo.toml, Cargo.lock
• Test patterns: #[test], #[cfg(test)]
• MSRV: Check rust-version in Cargo.toml

Python

• Frameworks: pytest, unittest, behave
• Commands: pytest, python -m pytest, behave
• Config files: pytest.ini, pyproject.toml, tox.ini
• Test patterns: test_*.py, *_test.py, tests/
• Version: Check requires-python in pyproject.toml

JavaScript/TypeScript

• Frameworks: Jest, Mocha, Cypress, Vitest
• Commands: npm test, yarn test, cypress run
• Config files: jest.config.js, vitest.config.ts, cypress.config.js
• Test patterns: *.test.js, *.spec.ts, __tests__/
• Version: Check engines.node in package.json

Go

• Framework: go test (built-in)
• Commands: go test ./..., go test -v
• Config files: go.mod, go.sum
• Test patterns: *_test.go
• Version: Check go directive in go.mod

Detection Workflow

1. Scan for config files:

find . -maxdepth 2 -name "Cargo.toml" -o -name "pyproject.toml" -o -name "package.json" -o -name "go.mod"

2. Check test directories:

find . -type d -name "tests" -o -name "__tests__" -o -name "test"

3. Identify test files:

find . -not -path "*/.venv/*" -not -path "*/__pycache__/*" \
  -not -path "*/node_modules/*" -not -path "*/.git/*" \
  \( -name "*test*" -o -name "*spec*" \) \
  | grep -E '\.(rs|py|js|ts|go)$'

4. Version constraints:

• Extract MSRV, Python version, Node version
• Note if constraints affect tooling (e.g., async/await)
• Document CI/CD version requirements

Output Format

## Framework Detection
- **Languages**: Rust, Python
- **Frameworks**: cargo test, pytest
- **Versions**:
  - Rust MSRV: 1.70
  - Python: >=3.8
- **Config files**: Cargo.toml, pyproject.toml

Remediation Planning

Concrete strategies for test improvement.

Test Improvement Patterns

1. Add Missing Coverage

Tie tests to specific behaviors using Given/When/Then:

### Gap: Authentication edge cases
**Behavior**: Given missing auth token, When hitting /v1/resource, Then HTTP 401 returned
**Test**: `tests/test_auth.py::test_missing_token_returns_401`
**Priority**: High (security boundary)

2. Refactor Test Helpers

Before (repeated setup):

def test_user_creation():
    db = setup_database()
    config = load_test_config()
    user_data = {"email": "alice@example.com", "role": "user"}
    ...

def test_user_deletion():
    db = setup_database()
    config = load_test_config()
    user_data = {"email": "bob@example.com", "role": "admin"}
    ...

After (fixtures):

@pytest.fixture
def test_db():
    db = setup_database()
    yield db
    db.teardown()

@pytest.fixture
def test_config():
    return load_test_config()

def test_user_creation(test_db, test_config):
    user_data = user_factory(email="alice@example.com")
    ...

3. Data Builders and Factories

Factory pattern:

# conftest.py
def user_factory(**overrides):
    defaults = {
        "email": "user@example.com",
        "role": "user",
        "verified": True,
        "created_at": datetime.now()
    }
    return User(**{**defaults, **overrides})

# test file
def test_admin_access():
    admin = user_factory(role="admin")
    assert admin.can_access_dashboard()

Builder pattern (Rust):

struct UserBuilder {
    email: String,
    role: Role,
    verified: bool,
}

impl UserBuilder {
    fn new() -> Self {
        Self {
            email: "user@example.com".to_string(),
            role: Role::User,
            verified: true,
        }
    }

    fn with_role(mut self, role: Role) -> Self {
        self.role = role;
        self
    }

    fn build(self) -> User {
        User { /* ... */ }
    }
}

#[test]
fn test_admin_permissions() {
    let admin = UserBuilder::new().with_role(Role::Admin).build();
    assert!(admin.can_delete_users());
}

4. Improve Assertions

Replace magic values:

# Before
assert response.status == 200

# After
from http import HTTPStatus
assert response.status == HTTPStatus.OK

Add context:

# Before
assert result

# After
assert result.success, f"Expected success, got error: {result.error}"

5. Remove Brittle Patterns

Dead waits → Explicit conditions:

# Before
time.sleep(3)
assert element.visible

# After
wait_for(element.to_be_visible, timeout=5)

Mocking internals → Mock boundaries:

# Before: mocking private implementation
@patch('service._internal_helper')
def test_service(mock):
    ...

# After: mock external dependency
@patch('requests.post')
def test_service(mock_requests):
    ...

Phased Remediation

For major test suite rewrites:

Phase 1: Stabilize (Week 1-2)

1. Fix flaky tests (eliminate dead waits, order dependencies) 2. Remove duplicate tests 3. Add missing critical path tests 4. Metric: Flaky test rate < 1%, critical paths 100%

Phase 2: Acceptance Specs (Week 3-4)

1. Add BDD scenarios for user-facing features 2. Create feature-to-test mapping 3. Document test strategy per component 4. Metric: All features have acceptance tests

Phase 3: Enforce Quality (Week 5+)

1. Set coverage budgets (80% standard, 100% critical) 2. Add pre-commit hooks for coverage checks 3. Integrate mutation testing for critical code 4. Metric: Coverage trends upward, no regressions

Recommendation Template

## Remediation Plan

### Immediate Actions (This Sprint)
1. **Fix flaky test**: `test_user_login_retries` - Replace sleep with explicit wait
   - Owner: @alice
   - Due: 2025-12-10

2. **Add missing coverage**: Password reset flow (currently 0%)
   - Tests needed: valid token, expired token, invalid token
   - Owner: @bob
   - Due: 2025-12-12

### Short-term (Next Sprint)
3. **Refactor fixtures**: Extract common setup in `tests/test_api.py`
   - Pattern: Use pytest fixtures for DB, config
   - Owner: @charlie
   - Due: 2025-12-20

### Long-term (Next Month)
4. **BDD acceptance tests**: User registration feature
   - Tool: Behave/Gherkin
   - Owner: @diana
   - Due: 2025-01-15

Exit Criteria

• [ ] All critical gaps have assigned owners and due dates
• [ ] Recommendations tied to specific behaviors
• [ ] Phased approach for large refactorings
• [ ] Success metrics defined (coverage %, flaky rate, etc.)

Scenario Quality Assessment

Evaluate test quality using BDD principles and assertion patterns.

Given/When/Then Clarity

Good Examples

Rust:

#[test]
fn test_authenticated_user_can_access_profile() {
    // Given: authenticated user
    let user = create_authenticated_user("alice@example.com");
    let token = generate_token(&user);

    // When: accessing profile endpoint
    let response = get("/profile", &token);

    // Then: profile data returned
    assert_eq!(response.status, 200);
    assert_eq!(response.body["email"], "alice@example.com");
}

Python:

def test_invalid_credentials_rejected():
    # Given: user with wrong password
    user = User(email="bob@example.com")
    wrong_password = "incorrect"

    # When: attempting authentication
    result = authenticate(user.email, wrong_password)

    # Then: authentication fails with 401
    assert result.status_code == 401
    assert "invalid credentials" in result.error_message

Gherkin (BDD):

Scenario: Registered user logs in successfully
  Given a registered user with email "alice@example.com"
  When they submit valid credentials
  Then they receive an authentication token
  And the token expires in 24 hours

Assertion Quality

Bad Assertions (vague, brittle)

# Too vague
assert result

# Multiple unrelated assertions
assert len(users) > 0 and users[0].active and config.debug

# Magic numbers without context
assert response.status == 200

Good Assertions (specific, meaningful)

# Specific outcome
assert result.status_code == 200, "Expected successful login"

# Named constants
assert response.status == HTTP_OK
assert user.role == UserRole.ADMIN

# Structured assertions
assert response.json() == {
    "user": {"email": expected_email, "verified": True},
    "token": {"expires_at": ANY_DATETIME}
}

Anti-Patterns to Flag

1. Dead Waits

# BAD: arbitrary sleep
time.sleep(5)
assert element.is_visible()

# GOOD: explicit wait with condition
wait_until(lambda: element.is_visible(), timeout=5)

2. Mocking Internals

# BAD: mocking implementation details
@patch('module.internal._private_helper')
def test_feature(mock_helper):
    ...

# GOOD: mock external dependencies only
@patch('requests.get')
def test_api_call(mock_get):
    ...

3. Repeated Boilerplate

# BAD: copy-pasted setup
def test_user_creation():
    db = Database("test.db")
    db.connect()
    user = User("alice")
    ...

def test_user_deletion():
    db = Database("test.db")
    db.connect()
    user = User("bob")
    ...

# GOOD: fixture/helper
@pytest.fixture
def db_session():
    db = Database("test.db")
    db.connect()
    yield db
    db.close()

4. Order Dependencies

# BAD: tests depend on execution order
def test_01_create_user():
    global user_id
    user_id = create_user()

def test_02_delete_user():
    delete_user(user_id)  # Depends on test_01!

# GOOD: isolated tests
def test_delete_user():
    user_id = create_user()  # Self-contained
    delete_user(user_id)
    assert not user_exists(user_id)

5. Multiple Assertions Without Context

# BAD: unclear which assertion failed
assert user.active
assert user.verified
assert user.role == "admin"

# GOOD: grouped with context or separate tests
assert user.active, "User should be active"
assert user.verified, "User should be verified"
assert user.role == "admin", "User should have admin role"

BDD Suite Quality

Reusable Step Definitions

# Good: parameterized, reusable
@given('a user with email "{email}"')
def create_user(context, email):
    context.user = User(email=email)

@when('they submit credentials with password "{password}"')
def submit_credentials(context, password):
    context.response = authenticate(context.user.email, password)

Background Context Sharing

Feature: User authentication

  Background:
    Given a clean database
    And the authentication service is running

  Scenario: Valid login
    Given a registered user
    ...

Scenario Outlines for Edge Cases

Scenario Outline: Password validation
  Given a user registering with password "<password>"
  When they submit the registration form
  Then they receive response "<outcome>"

  Examples:
    | password    | outcome           |
    | abc         | too_short         |
    | password123 | no_special_chars  |
    | P@ssw0rd!   | success           |

Quality Scoring

Score each test file 1-5 on:

• Clarity: Given/When/Then structure evident
• Assertions: Specific, meaningful checks
• Isolation: No shared state or order dependencies
• Maintainability: DRY, uses fixtures/helpers
• Coverage: Tests behavior, not implementation

Overall quality:

• 4-5: Excellent, minimal changes needed
• 3: Good, some improvements recommended
• 1-2: Poor, significant refactoring required