amirbena / amirbena/code-review-skill

Table-drive test_invocation_options.py's per-option contract tests

Open
#399 0 comments 0 reactions 1 assignee Claimed by @amirbena View on GitHub
area:packaging-portability maintainer-led priority:P2 type:refactor
Dominant language
Python
Stars
7
Forks
2
Avg merge
27m
Merged PRs (30d)
188

Description

## Type

Refactor

## Area

Packaging / Portability

## Priority

P2 — Medium

Contributor-owned: single-file scope, no invocation-option semantics touched.

## Problem

`tests/unit/review/test_invocation_options.py` (42 tests) is ~35 repetitions of one 7-test template — natural-affirmative phrasing enables the option, explicit negatives force it off, ambiguous/vague language leaves it unset, canonical `false` beats a natural affirmative phrasing, conflicting natural values fall back to default, direct and mediated forms have parity, no leakage between invocations — applied once per invocation option (fix-prompt/fix-guidance, `human_review_output`, `human_inline_findings`, details, compact/structured-summary), differing only by option name, phrase list, and defaults dict.

## Goal

One table-driven/parametrized test covering the same 7-axis contract per option, preserving identical per-option, per-axis coverage.

## Scope

- Extract the shared 7-test template into a parametrized test over `(option_name, phrase_list, defaults)`.
- Keep the phrase-list `subTest` loops already used inside each axis (legitimate exhaustive-space coverage, not a duplication smell) — do not remove them.
- Preserve per-option, per-axis failure attribution.

## Non-Goals

- Does not change any invocation-option semantics, phrase vocabulary, or the `normalize()` reference model in `tests/reference/review/invocation_options.py`.

## Acceptance Criteria

- [ ] Baseline test count recorded (42).
- [ ] After-change test count recorded and reported.
- [ ] All 6 options' 7-axis coverage preserved; no axis silently dropped for any option.
- [ ] A failing case identifies both the option and the axis that broke.
- [ ] Scoped tests and the full suite (`python3 -m unittest discover -s tests -t .`) pass.

## Dependencies

Depends on: none
Blocks: none
Parent: #395

Independent of the other four children (single file, no shared-helper collision).

## Validation

- Before/after: `grep -c "def test_" tests/unit/review/test_invocation_options.py`.
- `python3 -m unittest discover -s tests -t .`

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.