Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ its purpose, trigger phrases, and full instructions.
| Skill | Description |
|------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|
| **[pr-review](./skills/pr-review/)** | Pull request code review — reviews diffs for risk, security issues, API contract changes, dependency bumps, CI/CD and infrastructure changes. Produces concise Blocker / Important / Nit comments. |
| **[test-unit-standards](./skills/test-unit-standards/)** | Reference for unit test standards across isolation, scope, naming, assertions, coverage, and fixtures. Language-specific guidance (pytest, Jest, MUnit) with principles and conventions. |
| **[test-unit-write](./skills/test-unit-write/)** | Generate unit tests from scratch following language-specific standards. Analyzes source, selects mock strategies, and produces tests covering happy paths, failure conditions, and edge cases. |
| **[test-unit-review](./skills/test-unit-review/)** | Systematically audit unit test suites. Runs test runner, checks isolation/scope/naming/assertions/coverage standards, and reports findings by severity (Blocker / Important / Nit). |
| **[token-saving](./skills/token-saving/)** | Always-active response discipline — enforces brevity, no filler openers or closers, structured output, and a What/Why/How footer on code responses. Suspends on explicit "full detail" requests. |

## Finding More Skills
Expand Down
3 changes: 3 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ Navigation hub for all guides in this repository. Browse by category below.
|----|----|
| [PR Review](./pr-review.md) | How the PR review skill works, what sections it applies, and how to trigger it |
| [Token Saving](./token-saving.md) | Keeping AI responses concise — how the token-saving skill works and when it applies |
| [Unit Test Standards](./test-unit-standards.md) | Reference for unit test standards across isolation, scope, naming, assertions, coverage, fixtures |
| [Unit Test Writer](./test-unit-write.md) | Generate complete unit tests from scratch following language-specific standards |
| [Unit Test Reviewer](./test-unit-review.md) | Systematically audit unit tests and report findings by severity |

> **Keep this index up to date.** When you add a new guide, add a row to the appropriate table above.

Expand Down
101 changes: 101 additions & 0 deletions docs/test-unit-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# Unit Test Reviewer

This skill systematically audits unit tests against isolation, scope, naming, assertions, coverage, and fixture standards.

## Quick Start

Ask the agent to review tests:
- "Review these tests"
- "Audit this test file"
- "Check my unit tests"
- "Do these follow our standards?"
- "Give feedback on this test suite"

Provide the test file, and the agent will:

1. Run the test suite (pytest, Jest, MUnit, etc.)
2. Check each standard (isolation, scope, naming, assertions, coverage, fixtures)
3. Report findings grouped by severity: **Blocker / Important / Nit**

## What This Skill Does

**test-unit-review** performs a 5-step systematic audit:

1. **Load language reference** — Detect language/framework conventions
2. **Read test file completely** — Identify units under test, test count, dependencies
3. **Run tests if available** — Surface failing tests as blockers immediately
4. **Check against standards** — Walk through each standard category in order
5. **Report findings** — Group violations by severity with specific fixes

## Report Severity Levels

### Blocker

Tests that are broken or unreliable:
- **Failing tests** — any test that doesn't pass
- **Real I/O** — network calls, file reads/writes, real database connections
- **Shared mutable state** — tests that depend on execution order or share fixtures unsafely

### Important

Gaps in coverage or weak assertions:
- **Missing failure paths** — ≥1 failure path per behaviour not tested
- **Missing boundary coverage** — empty, zero, None/null, max/min values not tested
- **Weak assertions** — `assert result`, `assert x is not None` where exact value is known
- **Private member access** — tests reaching into private internals
- **Copy-pasted setup** — fixtures duplicated instead of reused
- **Unreadable naming** — test names that don't state condition and expected outcome

### Nit

Minor style issues:
- **Naming inconsistencies** — mixing naming conventions
- **Missing fixture docstrings** — fixtures without purpose documentation
- **Style issues** — formatting, unused imports, etc.

## Example Feedback

```
### Blocker
- ❌ test_payment_service_real_api (line 18): Makes real HTTP call to payment gateway. Mock the endpoint.

### Important
- ❌ test_order_with_zero_amount (line 45): Missing. Zero is a boundary value — test must verify rejection or special handling.
- ⚠️ test_order_succeeds (line 12): Assertion `assert result` is weak. Should be `assert result.status == "success"`.
- ⚠️ mock_payment_gateway (line 8): Fixture missing docstring — state purpose and side effects.

### Nit
- test_order_invalid_... and test_process_invalid_... mixed naming. Choose one prefix.

---
**Summary**: 1 blocker (remove real I/O), 2 important (add boundary test, strengthen assertion), fix naming. No other issues.
```

## Language Support

| Language | Command | Reference |
|---|---|---|
| Python + pytest | `pytest <file> -v` | `references/python-pytest.md` |
| JavaScript / TypeScript + Jest | `npx jest <file> --no-coverage` | `references/javascript-jest.md` |
| Scala + MUnit | `./mvnw test -pl <module> -Dtest=<Class>` | `references/scala-munit.md` |
| .NET | `dotnet test --filter "FullyQualifiedName~<TestClass>"` | (future) |

## When To Use

- Reviewing test files before PR merge
- Auditing test quality on an existing codebase
- Learning what standards look like in practice
- Validating test coverage and isolation

## When NOT To Use

- Writing new tests → use **[test-unit-write](./test-unit-write.md)**
- Abstract standards questions → use **[test-unit-standards](./test-unit-standards.md)**
- Reviewing PR code (non-tests) → use **[pr-review](./pr-review.md)**
- Debugging CI failures → not this skill (use debugging workflow)

## Related Skills

- **[test-unit-write](./test-unit-write.md)** — Generate tests following standards
- **[test-unit-standards](./test-unit-standards.md)** — Reference for standards definitions
- **[pr-review](./pr-review.md)** — Review full PR (including tests + source)
43 changes: 43 additions & 0 deletions docs/test-unit-standards.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Unit Test Standards

This skill defines comprehensive reference standards for unit testing across Python, JavaScript/TypeScript, and Scala.

## Quick Start

Ask the agent about unit test best practices:
- "What are our standards for unit test isolation?"
- "How should I name a test in pytest?"
- "What are the naming conventions for Jest tests?"
- "How do I structure fixtures properly?"

## What This Skill Does

**test-unit-standards** provides language-agnostic principles plus language-specific conventions for:

- **Isolation** — no real I/O, independent tests, no shared mutable state
- **Scope** — unit = one function/method/class, public interface only
- **Naming** — test names state scenario clearly
- **Assertions** — specific over generic, cover success + ≥1 failure path per behaviour
- **Coverage** — success paths, failure paths, boundary values (empty, zero, null, max)
- **Fixtures** — framework-idiomatic location, reusable, documented

## Language References

The skill loads language-specific guidance for:

| Language / Framework | Location |
|---|---|
| Python + pytest | `skills/test-unit-standards/references/python-pytest.md` |
| JavaScript / TypeScript + Jest | `skills/test-unit-standards/references/javascript-jest.md` |
| Scala + MUnit | `skills/test-unit-standards/references/scala-munit.md` |

## When To Use

- **Abstract questions**: "What are our standards?", "Best practices for X?", "Naming convention?"
- **Learning**: Understanding isolation rules, fixture reuse, coverage depth
- **Validation**: Confirming a test design before writing or reviewing

## Related Skills

- **[test-unit-write](./test-unit-write.md)** — Generate new tests following standards
- **[test-unit-review](./test-unit-review.md)** — Audit existing tests against standards
89 changes: 89 additions & 0 deletions docs/test-unit-write.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Unit Test Writer

This skill generates complete unit tests from scratch or extends coverage for functions, classes, and modules.

## Quick Start

Ask the agent to write tests:
- "Write unit tests for this function"
- "Add tests covering the failure paths"
- "Generate test cases for this class"
- "Help me test this service"

Provide the source code, and the agent will:

1. Analyze the public API and I/O boundaries
2. Select appropriate mock strategies (stub/patch/inject)
3. Generate test cases covering happy paths, failures, and edge cases
4. Output tests in language-idiomatic format (pytest, Jest, MUnit)

## What This Skill Does

**test-unit-write** follows a 7-step workflow:

1. **Load language reference** — Detect language/framework and load conventions
2. **Analyse source** — Identify public API, dependencies, I/O boundaries, failure conditions
3. **Choose mock strategy** — Decide how to isolate HTTP, DB, files, clock, randomness
4. **Scaffold test file** — Setup imports, fixtures, mocking infrastructure
5. **Write test cases** — Happy path, failure paths (≥1 per behaviour), edge/boundary cases
6. **Assert correctly** — Use framework-specific matchers, not generic truthy checks
7. **Run and validate** — Execute tests; fix any failures before returning

## Generated Test Structure

```python
# pytest example
import pytest
from unittest.mock import MagicMock
from order_service import OrderService

@pytest.fixture
def mock_payment_gateway():
"""Mock payment gateway for order processing."""
return MagicMock()

def test_order_succeeds_with_valid_input(mock_payment_gateway):
"""Happy path: order placed with valid amount and payment success."""
service = OrderService(mock_payment_gateway)
service.process_order(customer_id=123, amount=50.00)
assert mock_payment_gateway.charge.called

def test_order_fails_on_insufficient_funds(mock_payment_gateway):
"""Failure path: payment gateway returns insufficient funds."""
mock_payment_gateway.charge.side_effect = ValueError("Insufficient funds")
service = OrderService(mock_payment_gateway)
with pytest.raises(ValueError, match="Insufficient funds"):
service.process_order(customer_id=123, amount=50.00)
```

## Coverage Requirements

Each test addresses:
- **Success path** — main behaviour under ideal conditions
- **≥1 failure path per behaviour** — exceptions, error conditions, guards
- **Boundary values** — empty, zero, None/null, max/min values

## Language Support

- **Python** — pytest with fixtures, parametrization, monkeypatch
- **JavaScript / TypeScript** — Jest with mocks, spies, async/await
- **Scala** — MUnit with scopes, fixtures, assertions

## When To Use

- Writing tests for new functions or classes
- Extending coverage for existing code
- Scaffolding test structure and mocking strategy
- Learning what good test coverage looks like

## When NOT To Use

- Reviewing existing tests → use **[test-unit-review](./test-unit-review.md)**
- Abstract standards questions → use **[test-unit-standards](./test-unit-standards.md)**
- Test doubles / mocking patterns → separate skill (test-mocking-patterns, future)
- Integration tests → use integration test skill (future)

## Related Skills

- **[test-unit-standards](./test-unit-standards.md)** — Reference for naming, isolation, assertions
- **[test-unit-review](./test-unit-review.md)** — Audit tests after writing
98 changes: 98 additions & 0 deletions skills/test-unit-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
---
name: test-unit-review
description: >
Systematically review unit tests against isolation, scope, naming, assertions, coverage, fixtures.
Categorizes issues by severity (blocker/important/nit) with standards-grounded feedback.
Triggers on: review, audit, check, feedback requests ("review these tests", "audit this file",
"check my tests", "LGTM", "any issues", "give feedback", "do these follow standards").
NOT: writing (→test-unit-write), standards (→test-unit-standards), test doubles (→test-mocking-patterns),
test data (→test-data-management), PR review (→pr-review), debugging/CI.
license: Apache-2.0
compatibility: GitHub Copilot
---

# Unit Test Reviewer

**For writing new tests**, use **test-unit-write**. **For test doubles**, use **test-mocking-patterns**.

## Step 1 — Load language reference

Identify language/framework from imports or file extension. Load reference:

| Language / framework | Reference file |
|---|---|
| Python + pytest | `../test-unit-standards/references/python-pytest.md` |
| JavaScript / TypeScript + Jest | `../test-unit-standards/references/javascript-jest.md` |
| Scala + MUnit | `../test-unit-standards/references/scala-munit.md` |

## Step 2 — Read test file completely

Read entire file first. Note: unit under test, test count, imports, dependencies touched.

## Step 3 — Run tests if available

| Language | Command |
|---|---|
| Python | `pytest <test-file> -v` |
| TypeScript | `npx jest <test-file> --no-coverage` |
| Scala | `./mvnw test -pl <module> -Dtest=<TestClass>` |
| .NET | `dotnet test --filter "FullyQualifiedName~<TestClass>"` |

**Failing tests = Blocker** (regardless of style). Continue standards check even if all pass.

## Step 4 — Check against standards

Work through each standard from `../test-unit-standards/SKILL.md` in order.
For each category, list every violation found before moving to the next.

### 4.1 Isolation

- Real external I/O? (network calls, file reads/writes, real DB connections)
- Shared mutable state between tests?
- Tests that depend on execution order?

### 4.2 Scope

- Private members accessed (see language reference for the convention)?
- Test bypassing the public API to reach internal state?

### 4.3 Naming

- Does each test name clearly state unit, condition, and expected outcome?
- Apply the naming format from the language reference

### 4.4 Assertion quality

- Weak assertions (`assert result`, `assert result is not None`) where exact value known?
- Missing assertions (test body with no assert)?
- Side-effect assertions using manual truthy instead of framework matcher?
- **Note:** `pytest.approx` is **correct** for floats — do not flag. It is a specific assertion that verifies the value within a tolerance, stronger than truthy or `is not None` checks and kills arithmetic operator mutants. Only flag genuine weak assertions.

### 4.5 Coverage

- Success path per behaviour?
- Failure path per behaviour?
- Boundary values (zero, empty, None/null, max)?

### 4.6 Fixtures

- Setup code copy-pasted across tests?
- Shared fixtures lacking docstring?

## Step 5 — Report findings

Group by severity. Cite test name, line, rule, fix suggestion. Empty sections show `(none)`. End with a line confirming overall compliance or summarizing blockers/important issues.

### Blocker

Broken tests, real I/O calls, or shared state (suite unreliable/order-dependent).

### Important

Missing failure/boundary coverage, weak/absent assertions, private member access, copy-pasted setup, unreadable naming.

### Nit

Minor naming inconsistencies, missing fixture docstrings, style issues.

**Explicitly confirm compliance** for each category checked when no violations found.
Loading