amirbena / amirbena/code-review-skill

Consolidate duplicated test infrastructure without reducing regression coverage

Open
#395 0 comments 0 reactions 1 assignee Claimed by @amirbena View on GitHub
area:packaging-portability contributor-owned help wanted priority:P2 type:refactor
Dominant language
Python
Stars
7
Forks
2
Avg merge
26m
Merged PRs (30d)
200

Description

## Type

Refactor

## Area

Packaging / Portability

## Priority

P2 — Medium

Contributor-owned: a bounded, `tests/`-only maintainability pass — in the spirit of #217/#293 — that touches no packaged Skill resource, no production behavior, and no review/policy/benchmark semantics.

## Problem

A completed research pass over the ~3,950-test suite found two largely independent concerns: (1) test-count/maintenance growth caused by repeated test skeletons, and (2) runtime concentration in a small number of expensive integration/E2E tests. This epic addresses only the first. Roughly 350–400 test functions across five areas are the same generic test shape copy-pasted per policy doc, fixture corpus, or invocation option — not distinct coverage, but repeated scaffolding around genuinely per-target content.

## Goal

Reduce duplicated test definitions and source boilerplate toward an estimated ~3,560 collectible tests (from ~3,950), while every meaningful case, assertion, failure mode, and independent protection layer that exists today still exists after consolidation. This is test-suite consolidation, not test-coverage reduction; the ~3,560 figure is an estimate from research, not an acceptance target.

## Scope

Child issues:

- [ ] #396 — Consolidate specialist-depth/deepening policy-doc test skeletons
- [ ] #397 — Consolidate tests/policy/benchmark/ doc-pinning skeletons
- [ ] #398 — Build shared corpus-hygiene test infrastructure for benchmark and review fixture corpora
- [ ] #399 — Table-drive test_invocation_options.py's per-option contract tests
- [ ] #400 — Consolidate shared-policy wiring-suite tests in tests/policy/review/

## Non-Goals

- No production runtime, packaged Skill resource, or policy/benchmark semantics change.
- No CI, required-check, or workflow change (including `validate.yml`).
- No consolidation of the research's Cluster 7 (mutation-boundary, delegation, security-event, trusted-host-NL, publication-mode, and verdict-consistency malformed-fixture validator tests, or `validate_threat_model.py` hardening tests) — deliberately excluded, security-adjacent, left for a separate task.
- No runtime/CI optimization: no moving tests to a nightly tier, no removing or relocating sandbox adversarial tests, no parallelizing CI, no changing the production-adapter E2E test's behavior.

## Acceptance Criteria

- [ ] All child issues (#396–#400) are closed.
- [ ] Each child links back to this issue via native sub-issue and its `Parent:` reference.
- [ ] Aggregate collectible test count decreases meaningfully toward the ~3,560 estimate; the exact number is not a pass/fail gate.
- [ ] `python3 -m unittest discover -s tests -t .` passes after every child merges.
- [ ] No child touches Cluster 7 (security-fixture-validator hardening) or any CI/runtime-optimization surface.

## Dependencies

Depends on: none
Blocks: none
Parent: none
Related: #217, #293

## Validation

- Full suite (`python3 -m unittest discover -s tests -t .`) passes after each child and in aggregate.
- Collectible test count (`grep -rc "def test_" tests/`) recorded before and after each child.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.