orchestrata_tools.reviewer.test_reviewer

Reference › orchestrata_tools › reviewer › test_reviewer

TestReviewer checks test code for naming and structure problems. It reads an ASTIndex built from the tests snapshot and returns ReviewReport objects (models). The test_review wrapper prints the result.

_is_test_module follows pytest discovery: a file named test_*.py or *_test.py, skipping conftest.py and __init__.py. review_test_module reports a module with no test methods as critical and a badly named test module as low. review_test_method and review_test_function flag a name that does not start with test_ (high) and more than five parameters other than self and cls, counted as fixtures (medium), via _fixture_count. review_codebase visits only test modules, then their classes' test methods and module-level test functions, and returns only reports that contain issues.

At a glance

Class diagram

Dependency diagram

API

Classes

TestReviewer

Line 43

Review test quality using AST metadata.

Functions

Source

Permalink: https://gitlab.com/MSamy/orchestrata-ultimate-architecture/-/blob/a4cbc2bdf/src/orchestrata_tools/reviewer/test_reviewer.py

"""Test quality review engine.

Checks tests for isolation, naming, structure, and anti-patterns.
"""

from __future__ import annotations

import os
from typing import TYPE_CHECKING

from orchestrata_tools.reviewer.models import Issue, ReviewReport, Severity

if TYPE_CHECKING:
    from orchestrata_tools.models.ast_models import FunctionInfo, MethodInfo, ModuleInfo
    from orchestrata_tools.parser.indexer import ASTIndex


# Modules that are never test files — skip entirely
_SKIP_MODULE_NAMES = {"conftest.py", "__init__.py"}


def _is_test_module(module: ModuleInfo) -> bool:
    """Return True if the module is an actual test file (not conftest/__init__).

    Matches pytest discovery: files named test_*.py or *_test.py.
    """
    basename = os.path.basename(module.file_path or module.path)
    if basename in _SKIP_MODULE_NAMES:
        return False
    return basename.startswith("test_") or basename.endswith("_test.py")


def _fixture_count(method: MethodInfo | FunctionInfo) -> int:
    """Count fixture parameters, excluding self/cls.

    In pytest, fixtures are passed as function parameters.
    We exclude 'self' and 'cls' since they're not fixtures.
    """
    excluded = {"self", "cls"}
    return sum(1 for p in method.parameters if p.name not in excluded)


class TestReviewer:
    """Review test quality using AST metadata."""

    def __init__(self, index: ASTIndex):
        self.index = index

    def review_test_module(self, module: ModuleInfo) -> ReviewReport:
        """Review a single test module."""
        report = ReviewReport(target=module.path)

        test_methods: list[MethodInfo | FunctionInfo] = [
            m
            for cls in module.classes
            for m in cls.methods
            if m.name.startswith("test_")
        ]
        test_methods += [f for f in module.functions if f.name.startswith("test_")]

        if not test_methods:
            report.add(
                Issue(
                    rule="no-tests",
                    message="Test module has no test methods",
                    severity=Severity.CRITICAL,
                )
            )

        basename = os.path.basename(module.file_path or module.path)
        if not (basename.startswith("test_") or basename.endswith("_test.py")):
            report.add(
                Issue(
                    rule="test-module-naming",
                    message=f"Test module name '{basename}' should start with 'test_' or end with '_test.py'",
                    severity=Severity.LOW,
                )
            )

        return report

    def review_test_method(self, method: MethodInfo) -> ReviewReport:
        """Review a single test method."""
        report = ReviewReport(
            target=f"{method.module_path}.{method.class_name}.{method.name}"
        )

        # Test doesn't start with test_ (should never happen since we filter,
        # but catches tests called via indirect means)
        if not method.name.startswith("test_"):
            report.add(
                Issue(
                    rule="test-naming",
                    message=f"Test name '{method.name}' doesn't start with 'test_'",
                    severity=Severity.HIGH,
                )
            )

        # Too many fixtures (>5 excluding self/cls)
        fixtures = _fixture_count(method)
        if fixtures > 5:
            report.add(
                Issue(
                    rule="test-too-many-fixtures",
                    message=f"Test uses {fixtures} fixtures/parameters (excluding self/cls)",
                    severity=Severity.MEDIUM,
                    suggestion="Consider grouping fixtures or using a setup method",
                )
            )

        return report

    def review_test_function(
        self, func: FunctionInfo, module: ModuleInfo
    ) -> ReviewReport:
        """Review a standalone test function."""
        report = ReviewReport(target=f"{module.path}.{func.name}")

        if not func.name.startswith("test_"):
            report.add(
                Issue(
                    rule="test-naming",
                    message=f"Test name '{func.name}' doesn't start with 'test_'",
                    severity=Severity.HIGH,
                )
            )

        fixtures = _fixture_count(func)
        if fixtures > 5:
            report.add(
                Issue(
                    rule="test-too-many-fixtures",
                    message=f"Test uses {fixtures} fixtures/parameters",
                    severity=Severity.MEDIUM,
                    suggestion="Consider grouping fixtures or using a setup method",
                )
            )

        return report

    def review_codebase(self) -> list[ReviewReport]:
        """Review all test modules."""
        reports: list[ReviewReport] = []
        for module in self.index.codebase.modules:
            if not _is_test_module(module):
                continue

            mod_report = self.review_test_module(module)
            if mod_report.issues:
                reports.append(mod_report)

            for cls in module.classes:
                for method in cls.methods:
                    if method.name.startswith("test_"):
                        m_report = self.review_test_method(method)
                        if m_report.issues:
                            reports.append(m_report)

            for func in module.functions:
                if func.name.startswith("test_"):
                    f_report = self.review_test_function(func, module)
                    if f_report.issues:
                        reports.append(f_report)

        return reports