Phpunit Integration Test Reviewing

---
name: phpunit-integration-test-reviewing
version: 5.2.1
description: Internal sub-skill. Do not auto-activate. Use only when explicitly invoked by name by another skill or agent.
user-invocable: false
allowed-tools: Glob, Grep, Read, mcp__plugin_test-writing_test-rules__get_rules
---

# PHPUnit Integration Test Review

Review a Shopware PHPUnit integration test for compliance with integration testing conventions.

## Overview

Review the test against the composed integration catalog — INTEGRATION-001 through INTEGRATION-008 together with every convention, design, isolation, and provider rule whose `test-types` declares `integration` — assuming it belongs in `tests/integration/`. Do NOT load the placement reasoning rules (`group: placement`) and do NOT decide whether the test should be migrated.

## Input

- `{test_path}` (required unless `{digest}` is set, which supplies the class shape instead) — Path to the integration test file.
- `{methods}` (optional) — List of test method names to scope the review to. When omitted, the full class is reviewed.
- `{review_unit}` (optional) — `method`, `class-structure`, `class-bodies`, or a list of these. When set, only rules whose minimal evaluation unit matches load. When omitted, all rules load. Orthogonal to `{methods}`.
- `{digest}` (optional) — a pre-extracted, body-free structural digest of the test class. When set, review this text and skip reading the test file. Forces `class-structure` rules only. See Digest Mode.
- `{rules}` (optional) — the pre-rendered rule catalog as text, provided in your prompt. When set, enter Inline-Rules Mode: select rules from this text instead of calling `get_rules`. When omitted, rules load via `get_rules`. See Inline-Rules Mode.
- `{baseline}` (optional) — `pass`, `fail`, or `unavailable`: this file's test state before the review, supplied by the caller. Defaults to `unavailable` when omitted. Record and report it; this skill never executes tests to obtain it.

## Workflow

```dot
digraph integration_review {
  "Review request" [shape=doublecircle];
  "digest set?" [shape=diamond];
  "Digest Mode: judge the digest text alone; force class-structure rules; read nothing from disk" [shape=box];
  "Phase 1: locate the file, verify tests/integration/, mirror the path onto src/ to identify the SUT, read the test file and its integration base" [shape=box];
  "in tests/integration/?" [shape=diamond];
  "STOP: FAILED - Not an integration test" [shape=octagon, style=filled, fillcolor=red];
  "named methods resolve?" [shape=diamond];
  "STOP: FAILED - No matching methods found" [shape=octagon, style=filled, fillcolor=red];
  "src/ directory resolved by mirroring?" [shape=diamond];
  "Phase 2: read the resolved src/ files, list their constructor dependencies, note the boundary interfaces INTEGRATION-002 allows mocking" [shape=box];
  "Source resolution failed: continue with INTEGRATION-002 degraded - flag every non-boundary mock without the SUT-collaborator cross-check, and state in the report that source resolution failed and which rule it affected" [shape=box];
  "Phase 3: build the rule filters - test_type=integration, no group, no test_category, scoped_review, review_unit" [shape=box];
  "rules set?" [shape=diamond];
  "Inline-Rules Mode: select from the inline composed catalog; never filter on Group or Categories; open no rule file" [shape=box];
  "Call get_rules with the filters this mode sets" [shape=box];
  "get_rules available?" [shape=diamond];
  "STOP: FAILED - test-rules MCP server not available; never fall back to hardcoded checks" [shape=octagon, style=filled, fillcolor=red];
  "Phase 4: apply each rule's detection algorithm against the scoped code, record violations, write suggested fixes; INTEGRATION-008 emits one informational hint per class and no further deliberation" [shape=box];
  "Phase 5: apply the baseline - fail forces ISSUES_FOUND" [shape=box];
  "Emit the report per references/output-format.md" [shape=doublecircle];

  "Review request" -> "digest set?";
  "digest set?" -> "Digest Mode: judge the digest text alone; force class-structure rules; read nothing from disk" [label="yes"];
  "digest set?" -> "Phase 1: locate the file, verify tests/integration/, mirror the path onto src/ to identify the SUT, read the test file and its integration base" [label="no"];
  "Digest Mode: judge the digest text alone; force class-structure rules; read nothing from disk" -> "rules set?";
  "Phase 1: locate the file, verify tests/integration/, mirror the path onto src/ to identify the SUT, read the test file and its integration base" -> "in tests/integration/?";
  "in tests/integration/?" -> "STOP: FAILED - Not an integration test" [label="no"];
  "in tests/integration/?" -> "named methods resolve?" [label="yes"];
  "named methods resolve?" -> "STOP: FAILED - No matching methods found" [label="none match"];
  "named methods resolve?" -> "src/ directory resolved by mirroring?" [label="some or all match, or methods unset - each unmatched name is a warning"];
  "src/ directory resolved by mirroring?" -> "Phase 2: read the resolved src/ files, list their constructor dependencies, note the boundary interfaces INTEGRATION-002 allows mocking" [label="yes"];
  "src/ directory resolved by mirroring?" -> "Source resolution failed: continue with INTEGRATION-002 degraded - flag every non-boundary mock without the SUT-collaborator cross-check, and state in the report that source resolution failed and which rule it affected" [label="no - walked up to the src/ root"];
  "Phase 2: read the resolved src/ files, list their constructor dependencies, note the boundary interfaces INTEGRATION-002 allows mocking" -> "Phase 3: build the rule filters - test_type=integration, no group, no test_category, scoped_review, review_unit";
  "Source resolution failed: continue with INTEGRATION-002 degraded - flag every non-boundary mock without the SUT-collaborator cross-check, and state in the report that source resolution failed and which rule it affected" -> "Phase 3: build the rule filters - test_type=integration, no group, no test_category, scoped_review, review_unit";
  "Phase 3: build the rule filters - test_type=integration, no group, no test_category, scoped_review, review_unit" -> "rules set?";
  "rules set?" -> "Inline-Rules Mode: select from the inline composed catalog; never filter on Group or Categories; open no rule file" [label="yes"];
  "rules set?" -> "Call get_rules with the filters this mode sets" [label="no"];
  "Inline-Rules Mode: select from the inline composed catalog; never filter on Group or Categories; open no rule file" -> "Phase 4: apply each rule's detection algorithm against the scoped code, record violations, write suggested fixes; INTEGRATION-008 emits one informational hint per class and no further deliberation";
  "Call get_rules with the filters this mode sets" -> "get_rules available?";
  "get_rules available?" -> "STOP: FAILED - test-rules MCP server not available; never fall back to hardcoded checks" [label="no"];
  "get_rules available?" -> "Phase 4: apply each rule's detection algorithm against the scoped code, record violations, write suggested fixes; INTEGRATION-008 emits one informational hint per class and no further deliberation";
  "Phase 4: apply each rule's detection algorithm against the scoped code, record violations, write suggested fixes; INTEGRATION-008 emits one informational hint per class and no further deliberation" -> "Phase 5: apply the baseline - fail forces ISSUES_FOUND";
  "Phase 5: apply the baseline - fail forces ISSUES_FOUND" -> "Emit the report per references/output-format.md";
}
```

### Phase 1: Identify & Validate

1. Locate test file (by path or `Glob("tests/integration/**/*Test.php")`)
2. Verify file is in `tests/integration/` (abort if `tests/unit/` or `tests/migration/`)
3. Identify the SUT by mirroring the test's directory onto `src/`: `tests/integration/X/Y/` maps to `src/X/Y/`, walking up one directory level at a time until a directory exists under `src/`. `#[CoversClass]` is absent from integration tests by convention.
4. Read the full test file content
5. Read any `use IntegrationTestBehaviour;` / base class to understand the lifecycle
6. If `{methods}` provided: verify each named method exists. A method that is not found is a warning, and the remaining methods are still reviewed.

### Phase 2: Source Analysis

1. Read every `.php` file directly inside the `src/` directory resolved by mirroring in Phase 1 step 3 — this is the resolved source set INTEGRATION-002 reads against.
2. List each source file's constructor dependencies. INTEGRATION-002 uses this list to distinguish primary collaborators from boundary collaborators.
3. Note whether the SUT has explicit boundary interfaces (HTTP client, mailer, clock, randomness) — these are the allowable mock targets.

### Phase 3: Rule Review Filters

All `mcp__plugin_test-writing_test-rules__get_rules` calls in Phase 4 carry `test_type=integration` and NO `group` — that composes the catalog for this test type. Adding `group=integration` narrows it back to the integration group alone and drops every shared rule.

When `{methods}` is provided, also add `scoped_review=true`. When `{review_unit}` is set, also add `review_unit={value}`; the filter is single-valued per call, so for a list (e.g. the fused whole-class track `[class-structure, class-bodies]`) issue one call per value and union the results. Never pass a `test_category` filter — A–E categories are a unit-review axis.

Do NOT call `get_rules(group=placement)`. Placement reasoning is the migrating skill's responsibility.

### Scoped Review Filtering (Phase 4)

When `{methods}` is provided, apply detection only to the named methods and their associated data providers (identified by `#[DataProvider]` attributes on scoped methods). The rest of the class is available for context, but violations outside the scoped methods are not reported.

### Digest Mode

When `{digest}` is set, the supplied text is the only artifact under review:

- Do NOT `Read` the test file or the source class. The digest is body-free (class declaration, `#[CoversClass]`, member order, method signatures, attribute lines, property declarations) and self-contained for class-structure rules.
- Force `review_unit=class-structure`. In Phase 4, call `get_rules(test_type=integration, review_unit=class-structure)` with NO `scoped_review`. Apply whatever rules the filter returns — the composed catalog's only class-structure rule is CONV-005. When `{rules}` is also set, instead select the class-structure rules from the inline text per Inline-Rules Mode (`Review unit` == `class-structure`).
- Report `location` as a member name or attribute from the digest (line numbers are unavailable without the file body).
- `{methods}` and `{review_unit}` inputs are subsumed: the digest defines the scope and the unit.

### Inline-Rules Mode

When `{rules}` is set, the catalog is provided as text in your prompt: **select** rules from that text instead of calling `get_rules` in Phase 4. Each rule in the text is a metadata header — `# {id} — {title}`, then `Group: … | Enforce: …`, then `Test types: … | Categories: … | Scope: … | Review unit: … | Scoped review: …` — followed by the rule body. The text is already the composed catalog for this test type, so every rule in it applies here: never filter on `Group`. Select a rule when ALL hold:

- if `{review_unit}` is set: its `Review unit` equals that value (for a list, take the union over the values), **and**
- if `{methods}` is set (scoped review): its `Scoped review` is not `exclude`.

Do not filter on `Categories` — A–E is a unit-review axis. Apply each selected rule's detection algorithm. While `{rules}` is set, the inline text is the complete rule set: **NEVER** read, open, search, or locate a rule file by any means — no `Read`/`Grep`/`Glob`, no `get_rules`. (Reading the test file and its source class is unaffected.) When `{rules}` is omitted, rules load via `get_rules` with the Phase 3 filters.

### Phase 4: Apply Rules

For each rule obtained (inline selection or `get_rules`):

1. Read the rule's Detection section
2. Apply the detection logic against the test code (and source class when needed by INTEGRATION-002)
3. Record violations with rule ID, title, enforce level, location, current code, and suggested fix
4. For INTEGRATION-008 specifically: the rule produces a HINT, not an error or warning. Apply once per class, emit at most once in the Informational section. Do not deliberate further — the deep audit lives in the migrating skill.

### Phase 5: Generate Report

Apply the pre-review baseline: when `{baseline}` is `fail`, the report opens with a line before `## Summary` stating that this file's tests were already failing before this review, independent of the rule catalog below, and `status` becomes `ISSUES_FOUND` regardless of what the rule catalog finds. When `{baseline}` is `unavailable`, record it in the Summary's `Baseline` field and change nothing else. When `{baseline}` is `pass`, record it in the Summary's `Baseline` field.

For output format and examples, see references/output-format.md.

Report each issue using the rule's ID and title from `mcp__plugin_test-writing_test-rules__get_rules`:

```
### [{rule_id}] {title}
```

Include for each issue:
- **Current Code** — copied verbatim from the file at the cited location, read at review time. Never reconstructed from memory of the rule or paraphrased.
- **Suggested Fix** — the complete method body after the change. Empty only where the remediation deletes the method entirely; `deleted_methods` then names that method.
- **Issue** — names every line present in Current Code and absent from Suggested Fix. Each such line is a removal, and an unnamed removal is a defect in the finding. (The team-review schema names this field `summary`.)

A suggested remediation never changes what an existing assertion pins, and never introduces an assertion as a means of satisfying a structural, layout, naming, or style constraint — a fix that must compensate that way is wrong by construction: do not emit it. Where the finding IS itself a missing test or a missing assertion (a coverage-gap rule such as INTEGRATION-007 or DESIGN-006), those new assertions are that finding's explicit deliverable: name them in the finding, and never smuggle them into a remediation for an unrelated rule. Deletions ride `removed_assertions`. Re-expressing the same pinned fact in a different call — CONV-012's `assertTrue($a === $b)` becoming `assertSame($b, $a)`, or an ISOLATION-004 literal swap — does not change what the assertion pins.

Include the placement hint as a single line in the Informational section when INTEGRATION-008 fires.

Include full passed checks list.

### Output Contract

```yaml
test_path: tests/integration/Path/To/SomeTest.php
status: PASS|NEEDS_ATTENTION|ISSUES_FOUND|FAILED
baseline: pass | fail | unavailable   # supplied by the caller; recorded and reported, never executed
errors:
  - rule_id: INTEGRATION-001
    title: "Integration test uses Shopware integration base"
    enforce: must-fix
    location: SomeTest.php:25
    method: class-level                          # the test method the finding is in; "class-level" for a whole-class or structural finding
    current: |
      # problematic code
    suggested: |
      # fixed code
    implies_src_change: false    # true ONLY when the fix cannot be made in the test alone
    deleted_methods: []          # test methods this fix removes ENTIRELY, by bare name; [] when it removes none
    removed_assertions: []       # [{assertion, covered_by_test}] per assertion the fix removes
warnings:
  - rule_id: INTEGRATION-007
    title: "Setup-to-assertion ratio is balanced"
    enforce: should-fix
    location: SomeTest.php:60
    method: testPersistsState
    current: |
      # code
    suggested: |
      # improved code
    implies_src_change: false
    deleted_methods: [testRedundantReadBack]
    removed_assertions:
      - assertion: "static::assertSame('active', $product->getState())"
        covered_by_test: testPersistsState      # the surviving test, or the literal "none — coverage lost"
informational:
  - rule_id: INTEGRATION-008
    title: "Placement smoke check"
    method: class-level
    hint: "Every assertion is unit-shape. Consider invoking phpunit-integration-to-unit-migrating on this file."
    implies_src_change: false
    deleted_methods: []          # an informational entry whose fix removes test code names it here too
    removed_assertions: []
reason: null                     # the failure text when status is FAILED; null otherwise
```

Every entry names its `method`: the test method the finding is in, or the literal `class-level` for a whole-class or structural finding. `method` is half of a finding's identity (`rule_id|method`), so an omitted or empty value collapses every finding under one rule into the class-level bucket and merges defects that are not the same defect. Never leave it absent, and never write it with a trailing `(...)`.

Set `implies_src_change: true` on a finding whose fix cannot be made in the test file alone — it requires a change under `src/`. Default it to `false`. It is informational: it never changes `status` and never turns a warning into an error.

## Status Values

| Status | Condition |
|--------|-----------|
| PASS | 0 errors, 0 warnings |
| NEEDS_ATTENTION | 0 errors, 1+ warnings |
| ISSUES_FOUND | 1+ errors |
| FAILED | Invalid input (file not found, not in tests/integration/), or a refusal from the deletion after-state check |

Informational entries never raise `status` — the INTEGRATION-008 placement hint and the guard's `UNRESOLVED` entry alike. `PASS` with a placement hint is still `PASS`. A `fail` `{baseline}` sets `ISSUES_FOUND` regardless of this table. A guard refusal sets `FAILED` regardless of this table and of the baseline — `FAILED` outranks every other status.

## Track-Scoped Invocations

The team review decomposes large files into per-track reviews. Each track also receives `{rules}` (the pre-rendered catalog text in its prompt), so rule loading is Inline-Rules Mode selection rather than `get_rules`. Each track sets the inputs below:

- **Method track** — `test_path=…SomeTest.php`, `methods=[testCreate, testList]`, `review_unit=method`. Selects the catalog's `method` rules and judges only the named methods.
- **Fused whole-class track** — `test_path=…`, `review_unit=[class-structure, class-bodies]`, plus `methods=[…]` when the review is scoped. Reads full bodies; selects the class-structure and class-bodies rules from the inline text and unions them.
- **Class-structure digest** — `digest="<class shape text>"`, no `test_path` read. Per Digest Mode: select the `class-structure` rules from the inline text and judge them against the digest.

## Troubleshooting

### MCP Tool Unavailability

If `mcp__plugin_test-writing_test-rules__get_rules` is unavailable:
- Report error: "test-rules MCP server not available — ensure the test-writing plugin is installed and Claude Code was restarted"
- Do not fall back to hardcoded checks

### Not an Integration Test

If the file is not in `tests/integration/`:
- Report FAILED: "Not an integration test — this skill reviews tests in tests/integration/ only"
- Suggest `phpunit-unit-test-reviewing` for `tests/unit/` and `phpunit-migration-test-reviewing` for `tests/migration/`

### Source Directory Not Found

If directory mirroring walks up to the `src/` root without finding an existing directory:
- Continue the review; INTEGRATION-002 falls back to flagging any non-boundary mock without the SUT-collaborator cross-check
- Note in the report that source resolution failed and which rule was affected

### Placement vs. Review Boundary

The placement reasoning rules (`group: placement`, PLACEMENT-001..008) are NOT loaded by this skill. If the smoke check fires (INTEGRATION-008), the report emits a single hint; users must invoke `phpunit-integration-to-unit-migrating` explicitly to run the deliberation. Do not deliberate inline.