Implement Skip Decorator Abuse Detection

[frankbria]

Summary

Create automated detection for @pytest.mark.skip decorators that AI agents add to circumvent failing tests.

Problem

AI agents sometimes add @Skip decorators to failing tests instead of fixing them, which:

• Hides real bugs
• Degrades test suite value
• Creates technical debt
• Violates TDD principles

Tasks

1. Create Detection Script

• Create tools/detect-skip-abuse.py
• Parse Python AST to find skip decorators
• Check for justification comments
• Report file, line, and function name

2. Validation Logic

• Detect @skip, @skipif, @pytest.mark.skip
• Check for skip reason strings
• Flag skips with weak justifications
• Handle false positives gracefully

3. Integration

• Add to pre-commit hooks
• Make script executable
• Test with various skip patterns
• Add to CI/CD pipeline

4. Documentation

• Document why skips are forbidden
• Explain approval process for legitimate skips
• Add examples of proper test fixing

Test Cases

# Should detect these:
@pytest.mark.skip  # No reason
@pytest.mark.skip("TODO")  # Weak reason
@skip  # Bare decorator

# Should allow (if policy changed):
@pytest.mark.skip(reason="External API unavailable in CI")

Success Criteria

• Detects all skip decorator variations
• Pre-commit hook blocks commits with skips
• Clear error messages explain violations
• No false positives on legitimate code

Technical Approach

Use Python's ast module to parse test files and walk the AST looking for skip decorators. This is more reliable than regex matching.

Dependencies

• Issue Add Claude Code GitHub Workflow #1 (enforcement foundation) must be complete
• Requires Python 3.8+ for ast.walk()

Estimated Effort: 3-4 hours

[traycerai]

Plan

Observations

The codebase currently has:

• No skip decorators in use - grep search found zero instances of @pytest.mark.skip, @skip, @pytest.mark.skipif, or xfail in test files
• No tools/ directory - needs to be created
• No pre-commit configuration - no .pre-commit-config.yaml exists
• Strict TDD philosophy - documented in TESTING.md and TDD_WORKFLOW.md with >85% coverage requirements and 100% pass rate
• Existing validation script pattern - scripts/verify_migration_001.py demonstrates the project's approach: boolean-returning check functions, main() with exit codes, human-readable output with emojis
• Python 3.11+ requirement - AST module fully available
• CI/CD design exists but not implemented - docs/plans/2025-10-21-github-workflows-implementation-design.md shows planned CI workflow with quality gates

Approach

Create a comprehensive skip decorator abuse detection system with three integration points:

1. Detection Script (tools/detect-skip-abuse.py) - AST-based Python script that walks test files, identifies skip decorators, validates justifications, and reports violations in a format suitable for both human reading and CI integration

2. Pre-commit Integration - Create .pre-commit-config.yaml with a local hook that runs the detection script on staged Python test files, blocking commits with unauthorized skips

3. Documentation - Add skip decorator policy to TESTING.md, reference it in CONTRIBUTING.md and TDD_WORKFLOW.md, with clear examples of proper test fixing instead of skipping

The script will follow the project's existing patterns (from verify_migration_001.py) while outputting in a format compatible with pre-commit and future CI integration.

Reasoning

I explored the repository structure, examined test file patterns and found no existing skip decorators, reviewed the existing validation script to understand output conventions, analyzed documentation to determine where policy should be documented, and researched pre-commit best practices for AST-based validation hooks.

Proposed File Changes

📁 tools ^(NEW) 🔗

Create a new tools/ directory to house enforcement and utility scripts. This directory will contain the skip decorator abuse detection script and can be extended with other code quality tools in the future.

📄 tools/detect-skip-abuse.py ^(NEW) 🔗

References

• 📄 scripts/verify_migration_001.py
• 📄 tests/conftest.py

Create the skip decorator abuse detection script with the following structure:

Shebang and Docstring:

• Add #!/usr/bin/env python3 shebang for direct execution
• Include comprehensive docstring explaining purpose, usage, and exit codes

Imports:

• Standard library: ast, sys, pathlib.Path, argparse, typing
• No third-party dependencies to keep it lightweight

Constants:

• SKIP_DECORATORS: Set of decorator names to detect (pytest.mark.skip, pytest.mark.skipif, skip, skipif)
• WEAK_JUSTIFICATIONS: Set of weak reason strings (TODO, fix later, broken, failing, empty string, etc.)
• EXIT_SUCCESS = 0, EXIT_FAILURE = 1

SkipDetectorVisitor Class (ast.NodeVisitor):

• Initialize with filename for error reporting
• Implement visit_FunctionDef and visit_AsyncFunctionDef to check decorators on test functions
• For each decorator, check if it matches skip patterns (handle both @skip and @pytest.mark.skip forms)
• Extract reason argument if present (handle both positional and keyword arguments)
• Check for nearby comments (within 2 lines above the decorator) as potential justifications
• Collect violations as tuples: (filename, line_number, function_name, decorator_name, reason_or_none)
• Store violations in self.violations list

Helper Functions:

• check_file(filepath: Path) -> list[tuple]: Read file, parse with ast.parse (catch SyntaxError), run visitor, return violations
• is_test_file(filepath: Path) -> bool: Check if file matches test patterns (test_*.py or *_test.py)
• format_violation(violation: tuple) -> str: Format violation as path:line: [function_name] Unauthorized skip decorator: @decorator_name (reason: "reason_text" or "no reason provided")
• print_summary(violations: list, total_files: int): Print summary with emoji indicators matching project style from scripts/verify_migration_001.py

main() Function:

• Parse command-line arguments (file paths from pre-commit or --all-files flag)
• If --all-files, recursively find all test files in tests/ directory
• Otherwise, filter provided paths to test files only
• For each test file, call check_file() and aggregate violations
• Print each violation in path:line: format for editor/CI integration
• Print summary with total files checked and violations found
• Return EXIT_SUCCESS if no violations, EXIT_FAILURE if any found

Script Guard:

• if __name__ == '__main__': sys.exit(main())

Make executable:

• File should be marked executable (chmod +x)

📄 .pre-commit-config.yaml ^(NEW) 🔗

References

• 📄 pyproject.toml

Create pre-commit configuration file with the following structure:

Top-level Configuration:

• Set default_language_version to python: python3.11 to match project requirements from pyproject.toml
• Add fail_fast: false to run all hooks even if one fails

Repos Section:

1. Standard pre-commit hooks (repo: https://github.com/pre-commit/pre-commit-hooks):

   • trailing-whitespace: Remove trailing whitespace
   • end-of-file-fixer: Ensure files end with newline
   • check-yaml: Validate YAML syntax
   • check-added-large-files: Prevent large files (args: ['--maxkb=500'])
   • check-merge-conflict: Detect merge conflict markers

2. Python code quality hooks (repo: https://github.com/psf/black):

   • black: Code formatter with args matching pyproject.toml (--line-length=100)

3. Python linting (repo: https://github.com/astral-sh/ruff-pre-commit):

   • ruff: Fast Python linter with args matching pyproject.toml configuration

4. Local custom hooks (repo: local):

   • Hook ID: detect-skip-abuse
   • Name: Detect Skip Decorator Abuse
   • Entry: python tools/detect-skip-abuse.py
   • Language: python
   • Types: [python]
   • Files: ^tests/.*\.py$ (only run on test files)
   • Pass_filenames: true
   • Require_serial: false (can run in parallel)
   • Description: Detects unauthorized @pytest.mark.skip decorators in test files

CI Block (optional for pre-commit.ci):

• autofix_prs: true for auto-fixable hooks
• autoupdate_schedule: weekly

This configuration ensures the skip detection runs automatically on every commit affecting test files, alongside existing code quality tools.

📄 TESTING.md ^(MODIFY) 🔗

References

• 📄 tests/conftest.py
• 📄 docs/process/TDD_WORKFLOW.md ^(MODIFY)

Add a new section titled "Test Skip Policy & Enforcement" after the "Troubleshooting Guide" section (around line 395) and before "Next Steps (Sprint 2)" section.

New Section Content:

Header: ## Test Skip Policy & Enforcement

Policy Statement:

• State that skip decorators (@pytest.mark.skip, @skip, @pytest.mark.skipif) are prohibited in the test suite
• Explain the rationale: skips hide bugs, degrade test suite value, create technical debt, violate TDD principles
• Reference the project's strict TDD philosophy from the existing document

Why Skips Are Forbidden:

• Skipped tests indicate incomplete work or known bugs that should be fixed, not hidden
• They reduce confidence in the test suite's ability to catch regressions
• They accumulate over time and become "permanent temporary" solutions
• They violate the "100% pass rate" requirement documented in this file

Automated Enforcement:

• Describe the tools/detect-skip-abuse.py script that runs automatically via pre-commit hooks
• Explain that commits containing skip decorators will be blocked
• Note that the script checks for weak justifications like "TODO" or "fix later"
• Mention that the same check runs in CI (once CI is implemented per the design in docs/plans/2025-10-21-github-workflows-implementation-design.md)

What to Do Instead of Skipping:

1. Fix the test immediately - If a test is failing, fix the underlying code or the test itself
2. Remove the test - If the test is no longer relevant, delete it rather than skip it
3. Mark as integration test - If the test requires external resources, use @pytest.mark.requires_api_key or similar markers defined in tests/conftest.py (these are handled programmatically, not via skip decorators)
4. Create an issue and fix it - If the fix is complex, create a GitHub issue, assign it, and fix it in the same sprint

Rare Exceptions (Approval Required):

• In extremely rare cases where a skip is legitimately needed (e.g., testing platform-specific behavior, external API temporarily unavailable), document the process:
  • Create a GitHub issue explaining why the skip is necessary
  • Add a comment above the skip with the issue number and expected resolution date
  • Add the file to an allowlist in the detection script (requires code review approval)
  • Set a deadline for removing the skip (maximum 1 sprint)

Examples:

Show a code block with Bad Examples (will be blocked):

• @pytest.mark.skip with no reason
• @pytest.mark.skip("TODO") with weak reason
• @skip bare decorator

Show a code block with Good Alternatives:

• Using @pytest.mark.requires_api_key for conditional skipping based on environment
• Fixing the test by correcting the assertion
• Removing obsolete tests

Running the Detection Manually:

• Provide command to run the script manually: python tools/detect-skip-abuse.py --all-files
• Explain that pre-commit runs it automatically on staged files

Cross-references:

• Link to docs/process/TDD_WORKFLOW.md for examples of fixing failing tests
• Link to CONTRIBUTING.md for the contribution workflow

📄 CONTRIBUTING.md ^(MODIFY) 🔗

References

• 📄 TESTING.md ^(MODIFY)
• 📄 docs/process/TDD_WORKFLOW.md ^(MODIFY)

Update the "Testing" section (around line 34-38) to add a reference to the skip decorator policy.

Add a new bullet point:

• After "Run pytest before submitting PRs", add: "Never use skip decorators (@pytest.mark.skip) - see TESTING.md for the skip policy and alternatives"

Update the "Pull Request Process" section (around line 40-48):

• Add a new step after "Run tests and linting": "Ensure no skip decorators are present (pre-commit hook will check automatically)"

Add a new subsection under "Testing":

Header: ### Fixing Failing Tests

Content:

• Brief statement: "If tests fail, fix them immediately - never skip them"
• Reference: "See docs/process/TDD_WORKFLOW.md for examples of diagnosing and fixing failing tests"
• Reference: "See TESTING.md for the complete skip decorator policy"

This keeps CONTRIBUTING.md concise while pointing contributors to the detailed policy and examples in other documents.

📄 docs/process/TDD_WORKFLOW.md ^(MODIFY) 🔗

References

• 📄 TESTING.md ^(MODIFY)
• 📄 tests/conftest.py

Add a new section titled "Fixing Failing Tests" in the "Troubleshooting" section (after line 358, before the "Coverage below 85%" subsection).

New Subsection:

Header: ### "Tests are failing"

Content:

Never Skip Tests - Emphasize that skip decorators are prohibited (reference TESTING.md for full policy)

Solution: Debug and Fix - Provide a step-by-step troubleshooting workflow:

1. Run the failing test in isolation with verbose output:

   • Command: pytest tests/test_module.py::test_function_name -vv
   • This shows the full traceback and assertion details

2. Identify the root cause:

   • Is the test wrong? (incorrect assertion, outdated expectation)
   • Is the implementation wrong? (bug in the code being tested)
   • Is there a missing setup/teardown? (fixture issue, state pollution)

3. Fix the issue:

   • If test is wrong: Update the test to match correct behavior
   • If implementation is wrong: Fix the code (this is TDD working as intended!)
   • If setup is wrong: Fix the fixture in tests/conftest.py or add proper cleanup

4. Verify the fix:

   • Run the test again: pytest tests/test_module.py::test_function_name -v
   • Run all tests to ensure no regressions: pytest tests/ -v
   • Check coverage: pytest tests/ --cov=codeframe --cov-report=term-missing

Example: Fixing a Failing Database Test

Provide a narrative example:

• Test test_update_project_status is failing with assertion error
• Run in isolation and see that status is not being persisted
• Identify that db.commit() is missing in the implementation
• Add db.commit() to the update_project() method in codeframe/persistence/database.py
• Re-run test - now passes
• Run full suite to verify no regressions
• Commit with message explaining the fix

Example: Fixing a Flaky Test

Provide a narrative example:

• Test test_async_operation sometimes passes, sometimes fails
• Identify that it's a race condition (missing await or improper timeout)
• Fix by adding proper await or using pytest-asyncio fixtures correctly
• Add a comment explaining the async behavior
• Verify test passes consistently (run 10 times: pytest tests/test_module.py::test_async_operation --count=10)

When to Remove a Test:

• If the test is testing behavior that no longer exists or is no longer relevant
• Delete the test rather than skip it
• Document the removal in the commit message

Cross-reference:

• Link to TESTING.md skip policy section for more details on why skips are forbidden

📁 tests/tools ^(NEW) 🔗

Create a tests/tools/ directory to house tests for the enforcement scripts. This follows the project's testing convention where all tests live under the tests/ directory and mirror the source structure.

📄 tests/tools/__init__.py ^(NEW) 🔗

References

• 📄 tests/init.py

Create an empty __init__.py file to make tests/tools/ a Python package, following the project's convention seen in tests/__init__.py.

📄 tests/tools/test_detect_skip_abuse.py ^(NEW) 🔗

References

• 📄 tests/test_config.py
• 📄 tests/conftest.py
• 📄 docs/process/TDD_WORKFLOW.md ^(MODIFY)

Create comprehensive tests for the skip detection script following the project's TDD approach and testing patterns from tests/test_config.py and tests/conftest.py.

Module Docstring:

• "Tests for skip decorator abuse detection script."

Imports:

• Standard library: tempfile, pathlib.Path, textwrap
• pytest for fixtures and assertions
• Import the detection script functions (add tools/ to sys.path or use relative imports)

Test Fixtures:

• temp_test_file(tmp_path): Fixture that creates a temporary Python test file with given content, returns Path
• sample_test_with_skip(temp_test_file): Fixture that creates a test file with various skip patterns

Test Classes:

TestSkipDetection:

• test_detects_pytest_mark_skip_no_reason: Create file with @pytest.mark.skip, verify violation detected
• test_detects_pytest_mark_skip_with_weak_reason: Create file with @pytest.mark.skip("TODO"), verify violation detected
• test_detects_bare_skip_decorator: Create file with @skip, verify violation detected
• test_detects_skipif_decorator: Create file with @pytest.mark.skipif, verify violation detected
• test_ignores_non_test_functions: Create file with skip on non-test function, verify no violation
• test_handles_async_test_functions: Create file with skip on async test, verify violation detected
• test_handles_syntax_errors_gracefully: Create file with syntax error, verify script doesn't crash
• test_reports_correct_line_numbers: Verify violations include correct line numbers
• test_reports_function_names: Verify violations include function names

TestFileFiltering:

• test_identifies_test_files: Verify is_test_file() correctly identifies test_*.py and *_test.py
• test_ignores_non_test_files: Verify non-test files are skipped
• test_handles_nested_test_directories: Verify test files in subdirectories are found

TestViolationFormatting:

• test_formats_violation_with_reason: Verify output format includes reason
• test_formats_violation_without_reason: Verify output format for no reason
• test_output_parseable_by_editors: Verify format matches path:line: convention

TestMainFunction:

• test_returns_zero_for_clean_files: Run main() on clean test files, verify exit code 0
• test_returns_one_for_violations: Run main() on files with skips, verify exit code 1
• test_processes_multiple_files: Verify main() handles multiple file arguments
• test_all_files_flag: Verify --all-files flag finds all test files

TestEdgeCases:

• test_handles_empty_test_file: Empty file should not crash
• test_handles_file_with_only_imports: File with no tests should not crash
• test_handles_nested_decorators: Multiple decorators on one function
• test_handles_decorator_with_multiple_args: Skip with multiple arguments

Integration Test:

• test_end_to_end_detection: Create a temporary test directory with multiple files (some clean, some with skips), run the script, verify correct violations reported and exit code

All tests should follow the AAA pattern (Arrange-Act-Assert) as documented in docs/process/TDD_WORKFLOW.md and use descriptive names as shown in existing test files.

📄 .github/workflows/ci-tests.yml ^(NEW) 🔗

References

• 📄 docs/plans/2025-10-21-github-workflows-implementation-design.md
• 📄 pyproject.toml
• 📄 docs/process/TDD_WORKFLOW.md ^(MODIFY)

Create a CI testing workflow that includes the skip detection check, following the design from docs/plans/2025-10-21-github-workflows-implementation-design.md.

Workflow Name: CI Tests

Triggers:

• push: on all branches
• pull_request: to main, staging, development branches

Jobs:

Job: test

• runs-on: ubuntu-latest
• Strategy matrix: python-version: ['3.11', '3.12']

Steps:

1. Checkout code - actions/checkout@v4

2. Set up Python - actions/setup-python@v5 with matrix python-version

3. Install dependencies:

   • Install pip, setuptools, wheel
   • Install project with dev dependencies: pip install -e ".[dev]"
   • Install pre-commit: pip install pre-commit

4. Run pre-commit hooks (includes skip detection):

   • pre-commit run --all-files --show-diff-on-failure
   • This runs all hooks including the skip detector

5. Run code quality checks:

   • Black: black --check codeframe tests
   • Ruff: ruff check codeframe tests
   • Mypy: mypy codeframe/

6. Run tests with coverage:

   • pytest tests/ --cov=codeframe --cov-report=xml --cov-report=term --cov-fail-under=85
   • This enforces the 85% coverage threshold from docs/process/TDD_WORKFLOW.md

7. Upload coverage report:

   • actions/upload-artifact@v4 to upload coverage.xml
   • Can be used for coverage badges or analysis

Environment Variables:

• ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} (for tests marked with requires_api_key)

Failure Handling:

• Any step failure should fail the entire job
• Pre-commit failures (including skip detection) will block the PR

This workflow ensures that skip decorator abuse is caught in CI, not just locally, providing a safety net if developers bypass pre-commit hooks.

📄 README.md ^(MODIFY) 🔗

References

• 📄 CONTRIBUTING.md ^(MODIFY)
• 📄 TESTING.md ^(MODIFY)

Add a brief mention of the skip detection enforcement in the README to make it visible to new contributors.

Location: In the "Development" or "Contributing" section (if one exists), or create a new "Code Quality" section.

Content to Add:

Header: ### Code Quality & Enforcement (if creating new section)

Bullet points:

• "Pre-commit hooks enforce code quality standards (black, ruff, mypy)"
• "Skip decorators (@pytest.mark.skip) are prohibited - see TESTING.md for policy"
• "Install pre-commit hooks: pre-commit install"
• "Run all checks manually: pre-commit run --all-files"

Cross-reference:

• Link to CONTRIBUTING.md for full contribution guidelines
• Link to TESTING.md for testing standards

This provides visibility of the enforcement mechanism without cluttering the README with excessive detail.

Diagram

sequenceDiagram
    participant Dev as Developer
    participant Git as Git Commit
    participant PreCommit as Pre-commit Hook
    participant Script as detect-skip-abuse.py
    participant CI as GitHub Actions
    
    Dev->>Git: git commit (with test changes)
    Git->>PreCommit: Trigger pre-commit hooks
    PreCommit->>Script: Run on staged test files
    Script->>Script: Parse AST, detect skip decorators
    
    alt Skip decorators found
        Script-->>PreCommit: Exit 1 (violations found)
        PreCommit-->>Git: Block commit
        Git-->>Dev: ❌ Commit rejected with violation details
        Dev->>Dev: Fix tests (remove skips)
        Dev->>Git: git commit (fixed)
    else No skip decorators
        Script-->>PreCommit: Exit 0 (clean)
        PreCommit-->>Git: Allow commit
        Git-->>Dev: ✅ Commit successful
    end
    
    Dev->>CI: git push
    CI->>CI: Run pre-commit --all-files
    CI->>Script: Check all test files
    
    alt Violations in CI
        Script-->>CI: Exit 1
        CI-->>Dev: ❌ CI failed - skip decorators detected
    else Clean in CI
        Script-->>CI: Exit 0
        CI->>CI: Run pytest with coverage
        CI-->>Dev: ✅ All checks passed
    end

Loading

Import In IDE

🤖 Prompt for AI Agents

I have created the following plan after thorough exploration and analysis of the codebase. Follow the below plan verbatim. Trust the files and references. Do not re-verify what's written in the plan. Explore only when absolutely necessary. First implement all the proposed file changes and then I'll review all the changes together at the end.

### Observations

The codebase currently has:
- **No skip decorators in use** - grep search found zero instances of `@pytest.mark.skip`, `@skip`, `@pytest.mark.skipif`, or `xfail` in test files
- **No tools/ directory** - needs to be created
- **No pre-commit configuration** - no `.pre-commit-config.yaml` exists
- **Strict TDD philosophy** - documented in `TESTING.md` and `TDD_WORKFLOW.md` with >85% coverage requirements and 100% pass rate
- **Existing validation script pattern** - `scripts/verify_migration_001.py` demonstrates the project's approach: boolean-returning check functions, main() with exit codes, human-readable output with emojis
- **Python 3.11+ requirement** - AST module fully available
- **CI/CD design exists but not implemented** - `docs/plans/2025-10-21-github-workflows-implementation-design.md` shows planned CI workflow with quality gates

### Approach

Create a comprehensive skip decorator abuse detection system with three integration points:

1. **Detection Script** (`tools/detect-skip-abuse.py`) - AST-based Python script that walks test files, identifies skip decorators, validates justifications, and reports violations in a format suitable for both human reading and CI integration

2. **Pre-commit Integration** - Create `.pre-commit-config.yaml` with a local hook that runs the detection script on staged Python test files, blocking commits with unauthorized skips

3. **Documentation** - Add skip decorator policy to `TESTING.md`, reference it in `CONTRIBUTING.md` and `TDD_WORKFLOW.md`, with clear examples of proper test fixing instead of skipping

The script will follow the project's existing patterns (from `verify_migration_001.py`) while outputting in a format compatible with pre-commit and future CI integration.

### Reasoning

I explored the repository structure, examined test file patterns and found no existing skip decorators, reviewed the existing validation script to understand output conventions, analyzed documentation to determine where policy should be documented, and researched pre-commit best practices for AST-based validation hooks.

## Mermaid Diagram

sequenceDiagram
    participant Dev as Developer
    participant Git as Git Commit
    participant PreCommit as Pre-commit Hook
    participant Script as detect-skip-abuse.py
    participant CI as GitHub Actions
    
    Dev->>Git: git commit (with test changes)
    Git->>PreCommit: Trigger pre-commit hooks
    PreCommit->>Script: Run on staged test files
    Script->>Script: Parse AST, detect skip decorators
    
    alt Skip decorators found
        Script-->>PreCommit: Exit 1 (violations found)
        PreCommit-->>Git: Block commit
        Git-->>Dev: ❌ Commit rejected with violation details
        Dev->>Dev: Fix tests (remove skips)
        Dev->>Git: git commit (fixed)
    else No skip decorators
        Script-->>PreCommit: Exit 0 (clean)
        PreCommit-->>Git: Allow commit
        Git-->>Dev: ✅ Commit successful
    end
    
    Dev->>CI: git push
    CI->>CI: Run pre-commit --all-files
    CI->>Script: Check all test files
    
    alt Violations in CI
        Script-->>CI: Exit 1
        CI-->>Dev: ❌ CI failed - skip decorators detected
    else Clean in CI
        Script-->>CI: Exit 0
        CI->>CI: Run pytest with coverage
        CI-->>Dev: ✅ All checks passed
    end

## Proposed File Changes

### tools(NEW)

Create a new `tools/` directory to house enforcement and utility scripts. This directory will contain the skip decorator abuse detection script and can be extended with other code quality tools in the future.

### tools/detect-skip-abuse.py(NEW)

References: 

- scripts/verify_migration_001.py
- tests/conftest.py

Create the skip decorator abuse detection script with the following structure:

**Shebang and Docstring:**
- Add `#!/usr/bin/env python3` shebang for direct execution
- Include comprehensive docstring explaining purpose, usage, and exit codes

**Imports:**
- Standard library: `ast`, `sys`, `pathlib.Path`, `argparse`, `typing`
- No third-party dependencies to keep it lightweight

**Constants:**
- `SKIP_DECORATORS`: Set of decorator names to detect (`pytest.mark.skip`, `pytest.mark.skipif`, `skip`, `skipif`)
- `WEAK_JUSTIFICATIONS`: Set of weak reason strings (`TODO`, `fix later`, `broken`, `failing`, empty string, etc.)
- `EXIT_SUCCESS = 0`, `EXIT_FAILURE = 1`

**SkipDetectorVisitor Class (ast.NodeVisitor):**
- Initialize with filename for error reporting
- Implement `visit_FunctionDef` and `visit_AsyncFunctionDef` to check decorators on test functions
- For each decorator, check if it matches skip patterns (handle both `@skip` and `@pytest.mark.skip` forms)
- Extract reason argument if present (handle both positional and keyword arguments)
- Check for nearby comments (within 2 lines above the decorator) as potential justifications
- Collect violations as tuples: (filename, line_number, function_name, decorator_name, reason_or_none)
- Store violations in `self.violations` list

**Helper Functions:**
- `check_file(filepath: Path) -> list[tuple]`: Read file, parse with `ast.parse` (catch SyntaxError), run visitor, return violations
- `is_test_file(filepath: Path) -> bool`: Check if file matches test patterns (`test_*.py` or `*_test.py`)
- `format_violation(violation: tuple) -> str`: Format violation as `path:line: [function_name] Unauthorized skip decorator: @decorator_name (reason: "reason_text" or "no reason provided")`
- `print_summary(violations: list, total_files: int)`: Print summary with emoji indicators matching project style from `scripts/verify_migration_001.py`

**main() Function:**
- Parse command-line arguments (file paths from pre-commit or `--all-files` flag)
- If `--all-files`, recursively find all test files in `tests/` directory
- Otherwise, filter provided paths to test files only
- For each test file, call `check_file()` and aggregate violations
- Print each violation in `path:line:` format for editor/CI integration
- Print summary with total files checked and violations found
- Return `EXIT_SUCCESS` if no violations, `EXIT_FAILURE` if any found

**Script Guard:**
- `if __name__ == '__main__': sys.exit(main())`

**Make executable:**
- File should be marked executable (`chmod +x`)

### .pre-commit-config.yaml(NEW)

References: 

- pyproject.toml

Create pre-commit configuration file with the following structure:

**Top-level Configuration:**
- Set `default_language_version` to `python: python3.11` to match project requirements from `pyproject.toml`
- Add `fail_fast: false` to run all hooks even if one fails

**Repos Section:**

1. **Standard pre-commit hooks** (repo: `https://github.com/pre-commit/pre-commit-hooks`):
   - `trailing-whitespace`: Remove trailing whitespace
   - `end-of-file-fixer`: Ensure files end with newline
   - `check-yaml`: Validate YAML syntax
   - `check-added-large-files`: Prevent large files (args: `['--maxkb=500']`)
   - `check-merge-conflict`: Detect merge conflict markers

2. **Python code quality hooks** (repo: `https://github.com/psf/black`):
   - `black`: Code formatter with args matching `pyproject.toml` (`--line-length=100`)

3. **Python linting** (repo: `https://github.com/astral-sh/ruff-pre-commit`):
   - `ruff`: Fast Python linter with args matching `pyproject.toml` configuration

4. **Local custom hooks** (repo: `local`):
   - Hook ID: `detect-skip-abuse`
   - Name: `Detect Skip Decorator Abuse`
   - Entry: `python tools/detect-skip-abuse.py`
   - Language: `python`
   - Types: `[python]`
   - Files: `^tests/.*\.py$` (only run on test files)
   - Pass_filenames: `true`
   - Require_serial: `false` (can run in parallel)
   - Description: Detects unauthorized @pytest.mark.skip decorators in test files

**CI Block (optional for pre-commit.ci):**
- `autofix_prs: true` for auto-fixable hooks
- `autoupdate_schedule: weekly`

This configuration ensures the skip detection runs automatically on every commit affecting test files, alongside existing code quality tools.

### TESTING.md(MODIFY)

References: 

- tests/conftest.py
- docs/process/TDD_WORKFLOW.md(MODIFY)

Add a new section titled **"Test Skip Policy & Enforcement"** after the "Troubleshooting Guide" section (around line 395) and before "Next Steps (Sprint 2)" section.

**New Section Content:**

**Header:** `## Test Skip Policy & Enforcement`

**Policy Statement:**
- State that skip decorators (`@pytest.mark.skip`, `@skip`, `@pytest.mark.skipif`) are **prohibited** in the test suite
- Explain the rationale: skips hide bugs, degrade test suite value, create technical debt, violate TDD principles
- Reference the project's strict TDD philosophy from the existing document

**Why Skips Are Forbidden:**
- Skipped tests indicate incomplete work or known bugs that should be fixed, not hidden
- They reduce confidence in the test suite's ability to catch regressions
- They accumulate over time and become "permanent temporary" solutions
- They violate the "100% pass rate" requirement documented in this file

**Automated Enforcement:**
- Describe the `tools/detect-skip-abuse.py` script that runs automatically via pre-commit hooks
- Explain that commits containing skip decorators will be blocked
- Note that the script checks for weak justifications like "TODO" or "fix later"
- Mention that the same check runs in CI (once CI is implemented per the design in `docs/plans/2025-10-21-github-workflows-implementation-design.md`)

**What to Do Instead of Skipping:**
1. **Fix the test immediately** - If a test is failing, fix the underlying code or the test itself
2. **Remove the test** - If the test is no longer relevant, delete it rather than skip it
3. **Mark as integration test** - If the test requires external resources, use `@pytest.mark.requires_api_key` or similar markers defined in `tests/conftest.py` (these are handled programmatically, not via skip decorators)
4. **Create an issue and fix it** - If the fix is complex, create a GitHub issue, assign it, and fix it in the same sprint

**Rare Exceptions (Approval Required):**
- In extremely rare cases where a skip is legitimately needed (e.g., testing platform-specific behavior, external API temporarily unavailable), document the process:
  - Create a GitHub issue explaining why the skip is necessary
  - Add a comment above the skip with the issue number and expected resolution date
  - Add the file to an allowlist in the detection script (requires code review approval)
  - Set a deadline for removing the skip (maximum 1 sprint)

**Examples:**

Show a code block with **Bad Examples** (will be blocked):
- `@pytest.mark.skip` with no reason
- `@pytest.mark.skip("TODO")` with weak reason
- `@skip` bare decorator

Show a code block with **Good Alternatives**:
- Using `@pytest.mark.requires_api_key` for conditional skipping based on environment
- Fixing the test by correcting the assertion
- Removing obsolete tests

**Running the Detection Manually:**
- Provide command to run the script manually: `python tools/detect-skip-abuse.py --all-files`
- Explain that pre-commit runs it automatically on staged files

**Cross-references:**
- Link to `docs/process/TDD_WORKFLOW.md` for examples of fixing failing tests
- Link to `CONTRIBUTING.md` for the contribution workflow

### CONTRIBUTING.md(MODIFY)

References: 

- TESTING.md(MODIFY)
- docs/process/TDD_WORKFLOW.md(MODIFY)

Update the **"Testing"** section (around line 34-38) to add a reference to the skip decorator policy.

**Add a new bullet point:**
- After "Run `pytest` before submitting PRs", add: "Never use skip decorators (@pytest.mark.skip) - see TESTING.md for the skip policy and alternatives"

**Update the "Pull Request Process" section** (around line 40-48):
- Add a new step after "Run tests and linting": "Ensure no skip decorators are present (pre-commit hook will check automatically)"

**Add a new subsection under "Testing":**

**Header:** `### Fixing Failing Tests`

**Content:**
- Brief statement: "If tests fail, fix them immediately - never skip them"
- Reference: "See `docs/process/TDD_WORKFLOW.md` for examples of diagnosing and fixing failing tests"
- Reference: "See `TESTING.md` for the complete skip decorator policy"

This keeps `CONTRIBUTING.md` concise while pointing contributors to the detailed policy and examples in other documents.

### docs/process/TDD_WORKFLOW.md(MODIFY)

References: 

- TESTING.md(MODIFY)
- tests/conftest.py

Add a new section titled **"Fixing Failing Tests"** in the "Troubleshooting" section (after line 358, before the "Coverage below 85%" subsection).

**New Subsection:**

**Header:** `### "Tests are failing"`

**Content:**

**Never Skip Tests** - Emphasize that skip decorators are prohibited (reference `TESTING.md` for full policy)

**Solution: Debug and Fix** - Provide a step-by-step troubleshooting workflow:

1. **Run the failing test in isolation** with verbose output:
   - Command: `pytest tests/test_module.py::test_function_name -vv`
   - This shows the full traceback and assertion details

2. **Identify the root cause:**
   - Is the test wrong? (incorrect assertion, outdated expectation)
   - Is the implementation wrong? (bug in the code being tested)
   - Is there a missing setup/teardown? (fixture issue, state pollution)

3. **Fix the issue:**
   - If test is wrong: Update the test to match correct behavior
   - If implementation is wrong: Fix the code (this is TDD working as intended!)
   - If setup is wrong: Fix the fixture in `tests/conftest.py` or add proper cleanup

4. **Verify the fix:**
   - Run the test again: `pytest tests/test_module.py::test_function_name -v`
   - Run all tests to ensure no regressions: `pytest tests/ -v`
   - Check coverage: `pytest tests/ --cov=codeframe --cov-report=term-missing`

**Example: Fixing a Failing Database Test**

Provide a narrative example:
- Test `test_update_project_status` is failing with assertion error
- Run in isolation and see that status is not being persisted
- Identify that `db.commit()` is missing in the implementation
- Add `db.commit()` to the `update_project()` method in `codeframe/persistence/database.py`
- Re-run test - now passes
- Run full suite to verify no regressions
- Commit with message explaining the fix

**Example: Fixing a Flaky Test**

Provide a narrative example:
- Test `test_async_operation` sometimes passes, sometimes fails
- Identify that it's a race condition (missing `await` or improper timeout)
- Fix by adding proper `await` or using `pytest-asyncio` fixtures correctly
- Add a comment explaining the async behavior
- Verify test passes consistently (run 10 times: `pytest tests/test_module.py::test_async_operation --count=10`)

**When to Remove a Test:**
- If the test is testing behavior that no longer exists or is no longer relevant
- Delete the test rather than skip it
- Document the removal in the commit message

**Cross-reference:**
- Link to `TESTING.md` skip policy section for more details on why skips are forbidden

### tests/tools(NEW)

Create a `tests/tools/` directory to house tests for the enforcement scripts. This follows the project's testing convention where all tests live under the `tests/` directory and mirror the source structure.

### tests/tools/__init__.py(NEW)

References: 

- tests/__init__.py

Create an empty `__init__.py` file to make `tests/tools/` a Python package, following the project's convention seen in `tests/__init__.py`.

### tests/tools/test_detect_skip_abuse.py(NEW)

References: 

- tests/test_config.py
- tests/conftest.py
- docs/process/TDD_WORKFLOW.md(MODIFY)

Create comprehensive tests for the skip detection script following the project's TDD approach and testing patterns from `tests/test_config.py` and `tests/conftest.py`.

**Module Docstring:**
- "Tests for skip decorator abuse detection script."

**Imports:**
- Standard library: `tempfile`, `pathlib.Path`, `textwrap`
- `pytest` for fixtures and assertions
- Import the detection script functions (add `tools/` to sys.path or use relative imports)

**Test Fixtures:**
- `temp_test_file(tmp_path)`: Fixture that creates a temporary Python test file with given content, returns Path
- `sample_test_with_skip(temp_test_file)`: Fixture that creates a test file with various skip patterns

**Test Classes:**

**TestSkipDetection:**
- `test_detects_pytest_mark_skip_no_reason`: Create file with `@pytest.mark.skip`, verify violation detected
- `test_detects_pytest_mark_skip_with_weak_reason`: Create file with `@pytest.mark.skip("TODO")`, verify violation detected
- `test_detects_bare_skip_decorator`: Create file with `@skip`, verify violation detected
- `test_detects_skipif_decorator`: Create file with `@pytest.mark.skipif`, verify violation detected
- `test_ignores_non_test_functions`: Create file with skip on non-test function, verify no violation
- `test_handles_async_test_functions`: Create file with skip on async test, verify violation detected
- `test_handles_syntax_errors_gracefully`: Create file with syntax error, verify script doesn't crash
- `test_reports_correct_line_numbers`: Verify violations include correct line numbers
- `test_reports_function_names`: Verify violations include function names

**TestFileFiltering:**
- `test_identifies_test_files`: Verify `is_test_file()` correctly identifies `test_*.py` and `*_test.py`
- `test_ignores_non_test_files`: Verify non-test files are skipped
- `test_handles_nested_test_directories`: Verify test files in subdirectories are found

**TestViolationFormatting:**
- `test_formats_violation_with_reason`: Verify output format includes reason
- `test_formats_violation_without_reason`: Verify output format for no reason
- `test_output_parseable_by_editors`: Verify format matches `path:line:` convention

**TestMainFunction:**
- `test_returns_zero_for_clean_files`: Run main() on clean test files, verify exit code 0
- `test_returns_one_for_violations`: Run main() on files with skips, verify exit code 1
- `test_processes_multiple_files`: Verify main() handles multiple file arguments
- `test_all_files_flag`: Verify `--all-files` flag finds all test files

**TestEdgeCases:**
- `test_handles_empty_test_file`: Empty file should not crash
- `test_handles_file_with_only_imports`: File with no tests should not crash
- `test_handles_nested_decorators`: Multiple decorators on one function
- `test_handles_decorator_with_multiple_args`: Skip with multiple arguments

**Integration Test:**
- `test_end_to_end_detection`: Create a temporary test directory with multiple files (some clean, some with skips), run the script, verify correct violations reported and exit code

All tests should follow the AAA pattern (Arrange-Act-Assert) as documented in `docs/process/TDD_WORKFLOW.md` and use descriptive names as shown in existing test files.

### .github/workflows/ci-tests.yml(NEW)

References: 

- docs/plans/2025-10-21-github-workflows-implementation-design.md
- pyproject.toml
- docs/process/TDD_WORKFLOW.md(MODIFY)

Create a CI testing workflow that includes the skip detection check, following the design from `docs/plans/2025-10-21-github-workflows-implementation-design.md`.

**Workflow Name:** `CI Tests`

**Triggers:**
- `push`: on all branches
- `pull_request`: to `main`, `staging`, `development` branches

**Jobs:**

**Job: test**
- `runs-on: ubuntu-latest`
- Strategy matrix: `python-version: ['3.11', '3.12']`

**Steps:**
1. **Checkout code** - `actions/checkout@v4`

2. **Set up Python** - `actions/setup-python@v5` with matrix python-version

3. **Install dependencies:**
   - Install pip, setuptools, wheel
   - Install project with dev dependencies: `pip install -e ".[dev]"`
   - Install pre-commit: `pip install pre-commit`

4. **Run pre-commit hooks** (includes skip detection):
   - `pre-commit run --all-files --show-diff-on-failure`
   - This runs all hooks including the skip detector

5. **Run code quality checks:**
   - Black: `black --check codeframe tests`
   - Ruff: `ruff check codeframe tests`
   - Mypy: `mypy codeframe/`

6. **Run tests with coverage:**
   - `pytest tests/ --cov=codeframe --cov-report=xml --cov-report=term --cov-fail-under=85`
   - This enforces the 85% coverage threshold from `docs/process/TDD_WORKFLOW.md`

7. **Upload coverage report:**
   - `actions/upload-artifact@v4` to upload coverage.xml
   - Can be used for coverage badges or analysis

**Environment Variables:**
- `ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}` (for tests marked with `requires_api_key`)

**Failure Handling:**
- Any step failure should fail the entire job
- Pre-commit failures (including skip detection) will block the PR

This workflow ensures that skip decorator abuse is caught in CI, not just locally, providing a safety net if developers bypass pre-commit hooks.

### README.md(MODIFY)

References: 

- CONTRIBUTING.md(MODIFY)
- TESTING.md(MODIFY)

Add a brief mention of the skip detection enforcement in the README to make it visible to new contributors.

**Location:** In the "Development" or "Contributing" section (if one exists), or create a new "Code Quality" section.

**Content to Add:**

**Header:** `### Code Quality & Enforcement` (if creating new section)

**Bullet points:**
- "Pre-commit hooks enforce code quality standards (black, ruff, mypy)"
- "Skip decorators (@pytest.mark.skip) are prohibited - see TESTING.md for policy"
- "Install pre-commit hooks: `pre-commit install`"
- "Run all checks manually: `pre-commit run --all-files`"

**Cross-reference:**
- Link to `CONTRIBUTING.md` for full contribution guidelines
- Link to `TESTING.md` for testing standards

This provides visibility of the enforcement mechanism without cluttering the README with excessive detail.

---

Developer Humor

🎭 A skip decorator walked into a bar,\n> The bouncer said \Sorry, you can't get far!\n> \But I'm just TODO,\ it cried with a plea,\n> \That's exactly why,\ said the AST.\n> 🚫 No skips allowed, fix your tests with care,\n> Or pre-commit will catch you—beware! ⚠️

---

Execution Information

Branch: main
Commit: d0e2480

---

💡 Tips

Supported Commands (Inside Comments)

• Use @traycerai generate to iterate on the previous version of the implementation plan.

Supported Commands (Inside Description)

• Add @traycerai ignore anywhere in the ticket description to prevent this ticket from being processed.
• Add @traycerai branch:<branch-name> anywhere in the ticket description to specify the target branch for the implementation plan.

Community

• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[frankbria]

Resolved in PR #20 - Skip decorator detection implemented with scripts/detect-skip-abuse.py. Supports @Skip, @pytest.mark.skip, @unittest.skip with AST parsing. 14/14 tests passing.