Skip to content

How to review Python test code

Paths in this document are relative to the Zolletta-MetaSkill project root.

The python-testing-style skill reviews Python test suites for isolation, naming, coverage gaps, mocking patterns, fixture design, and AAA structure. This guide walks through what the skill checks, how coverage gap detection works, and how to configure the rules — so you know what to expect when the skill runs as part of a full review or on its own.

Prerequisites

Requires a Python project set up via /zolletta-metaskill setup. Reads python.tools.* and python.testing from settings.json.

What the skill checks

The skill evaluates test code across six areas, combining always-on structural rules with configurable thresholds:

  • Test isolation — tests must be independent, with no shared mutable state between them. Each test should clean up after itself. Use fixtures with appropriate scopes (function, module, session) to manage shared resources without coupling tests to each other.
  • Test naming — test functions should follow the test_<unit>_<scenario>_<expected_outcome> pattern so that the name alone describes what is being tested. The skill enforces this with the deterministic src/zolletta_metaskill/testing_style/python/test_naming_scanner.py scanner, which counts underscore-separated segments after the test_ prefix and flags functions with fewer than the minimum (default: 3). Good names look like test_create_user_with_valid_data_returns_user; bad names look like test_1 or test_init.
  • Coverage gaps — the skill runs pytest --cov and analyses whether code is actually exercised by tests, regardless of whether a dedicated test_<module>.py file exists. This is a mandatory step: never flag a coverage gap based on grep alone.
  • Mocking patterns — when a class has no direct test file, the skill traces the call chain to determine whether callers instantiate the class for real (with mocked dependencies) or replace it entirely with a MagicMock or patch. A real instance means the class is indirectly covered; a full mock means it is not.
  • Fixture design — fixtures should use the narrowest scope that makes sense and avoid coupling tests through shared mutable state. The skill checks that fixtures are not leaking state between tests.
  • AAA structure — each test follows the Arrange-Act-Assert pattern: set up preconditions, execute the code under test, then verify the results. Tests that mix arrangement and assertion, or that assert before acting, are flagged.

Coverage gap detection procedure

Coverage gap detection is the most involved part of the review because a class with zero direct references in test files may still be well-covered through indirect calls. The skill follows a strict four-step procedure:

Step 1 — Run coverage and identify structurally missing files

The skill runs pytest --cov and then runs src/zolletta_metaskill/testing_style/general/test_structure_scanner.py to get the structural "Missing tests" table. Files that appear in this table are candidates — but structural absence does not mean zero coverage.

Step 2 — Check indirect coverage for each candidate

For each structurally missing file, the skill searches all test files for class name references. If any test file instantiates the class or calls its methods (even indirectly through a caller), the file has indirect coverage. The skill then checks whether callers use real instances or full mocks: my_class = MagicMock() means the class is NOT covered, while my_class = MyClass(mock_dependency) means the class IS covered because a real instance is created with only its dependencies mocked.

Step 3 — Only flag as a gap if coverage is genuinely low

Report a coverage gap only when all three conditions are true: coverage is below coverage_gap_threshold (default 50), there are no direct test references, and all callers are mocked in tests (no real instances). If any of these conditions is false, the class has adequate coverage and is not flagged.

Step 4 — For genuine gaps, check callers' test style

When a genuine gap is found, check whether the caller's tests mock the class or use a real instance, because that determines the recommended fix. If callers mock the class entirely, recommend creating a direct unit test file for the class itself. If callers use real instances but do not exercise all branches, recommend adding edge-case tests to the existing caller tests.

Scope boundary with the patterns skill

The patterns skill runs src/zolletta_metaskill/testing_style/general/test_structure_scanner.py, which produces a "Missing tests" table — a structural check that reports when no test_<module>.py file exists for a given source module. That structural finding is owned by patterns. The python-testing-style skill owns coverage analysis only: whether code is actually exercised by tests, not whether a matching test file exists. The structural check is not duplicated. If src/zolletta_metaskill/testing_style/general/test_structure_scanner.py already flagged a file as structurally missing a test, reference that finding but focus on whether the code is covered through indirect calls or integration tests.

Configurable rule toggles via settings.json

The skill reads its configurable rules from the python.testing object in settings.json. Three settings are available:

Key Type Default Description
coverage_gap_threshold integer (0–100) 50 Module coverage below this percentage is a candidate gap (combined with the other two conditions from Step 3)
coverage_well_covered_threshold integer (0–100) 80 Module coverage above this percentage is never flagged as a gap, even with no direct test references
check_test_naming boolean true When true, the skill runs src/zolletta_metaskill/testing_style/python/test_naming_scanner.py to enforce the test_<unit>_<scenario>_<expected> naming convention

The remaining rules — AAA pattern, test isolation, mandatory coverage gap detection, and the scope boundary with patterns — are always-on and cannot be disabled. Follows review mode — read-only, two-bucket classification, no fixes applied.

See also