fix(quality): stop a phpmd finding from silently deleting the PHPUnit signal

[rubenvdlinde]

The defect

phpunit was gated on needs.php-quality.result != 'failure'.

php-quality is a matrix job, so its result is failure if any leg fails — lint, phpcs, phpmd, psalm, phpstan, phpmetrics. A complexity or formatting finding therefore deleted the entire PHPUnit matrix.

This is a dead gate in the strict sense: its absence looks exactly like its success. A skipped matrix renders in the checks list as a single greyed-out skipping placeholder — not red, not missing, just quietly not there. Nothing in the PR view says "your tests did not run".

The evidence — a natural A/B, measured 2026-08-04

repo	phpmd job	total jobs	PHPUnit legs
ConductionNL/procest (run 30863839239)	failure (25 findings)	26	0
ConductionNL/decidesk (run 30862373575)	success	29	4, all green

Identical workflow inputs. The only difference is phpmd's colour.

Two things follow:

1. procest's 1684 tests had no CI signal at all for as long as phpmd was red.
2. decidesk's phpmd was green only because phpmd.baseline.xml was suppressing 73 real findings. So whether decidesk's suite ran at all was contingent on a suppression file. Delete the baseline honestly — which is the direction that repo is going — and its test signal would have gone dark too.

That second point is the one that makes this structural rather than cosmetic: the fix for one problem (remove the suppression) silently caused another (lose the tests).

The change

-    if: ${{ inputs.enable-php && inputs.enable-phpunit && !cancelled() && needs.php-quality.result != 'failure' && needs.security.result != 'failure' }}
+    if: ${{ inputs.enable-php && inputs.enable-phpunit && !cancelled() && needs.security.result != 'failure' }}

• security gate kept. A repo with a known-vulnerable dependency should not spin up servers.
• needs: retained for ordering; with !cancelled() and no result condition, PHPUnit runs regardless of static-analysis colour.
• This aligns phpunit with playwright, which in this same file already deliberately declines to gate on php-quality (its if: checks only needs.security.result). That inconsistency is itself evidence the php-quality gate on phpunit was an oversight rather than a decision.
• newman keeps its php-quality gate. It stands up a live stack and is a separate cost/risk decision; left unchanged and noted inline.

Verification

• YAML parses; jobs count unchanged at 18; phpunit.if and playwright.if are now equivalent in their php-quality treatment.
• A live positive control is running: ConductionNL/procest@ci/prove-phpunit-gate points code-quality.yml at this branch while procest's phpmd is still red. If the fix works, that run shows phpmd failing and the PHPUnit legs executing. Result posted below as a comment.

Blast radius

Fleet-wide (every Conduction app consumes quality.yml@main). The effect is strictly more signal: repos whose php-quality is green see no change; repos whose php-quality is red gain back their test results. CI cost rises only for repos that are currently red.

[rubenvdlinde]

Live positive control — the fix works, and it immediately found something

ConductionNL/procest@feature/prove-phpunit-gate is development with one line changed: code-quality.yml points at this branch instead of @main. procest's phpmd is still red (25 findings), so under the old gate the PHPUnit matrix could not run.

Run 30866791400:

	before (run 30863839239, @main)	after (run 30866791400, this branch)
PHP Quality (phpmd)	failure	failure (unchanged — still 25 findings)
PHPUnit legs	0 (single skipping placeholder)	4 — PHP 8.3/8.4 × NC stable31/stable32
total jobs	26	29

phpmd red, PHPUnit ran. That is exactly the behaviour the old if: made impossible.

What it surfaced — the gate was hiding a second, worse defect

All four PHPUnit legs failed:

Tests: 1684, Assertions: 5614, Skipped: 5, Risky: 223.
Process completed with exit code 1

Zero failures, zero errors — 223 risky tests. procest's phpunit.xml sets failOnRisky="true" and beStrictAboutCoverageMetadata="true", and CI runs phpunit --coverage-clover, so every test that touches a class not named in its #[CoversClass]/#[UsesClass] list is risky and the run exits 1:

1) ...AssistantControllerTest::testHermiqGuardrailBlockRelaysStatusAndErrorCode
This test executed code that is not listed as code to be covered or used:
- OCA\Procest\Service\Assistant\HermiqAssistantException

This is invisible locally: composer test:all runs without coverage, so it reports 1684 passing and exits 0. Only the CI invocation collects coverage, and that invocation had never once been allowed to run.

So procest's PHPUnit job was not merely dark — it was red on arrival. Two dead-gate causes stacked: the job was skipped, and had it not been skipped it could never have passed. The skip is what kept the second one invisible.

For contrast, decidesk carries identical phpunit.xml strictness (failOnRisky="true", beStrictAboutCoverageMetadata="true") and its 4 legs are green — so this is procest's annotations being incomplete, not a bad setting to be loosened.

Consequence for merging this PR

Merging turns procest's PHPUnit red where it is currently silent. That is the correct outcome and the entire point — but it should land together with, or just ahead of, the annotation fix, so procest is not left red on a newly-visible axis. Tracking that separately; this PR is what makes the defect visible at all.

The probe branch feature/prove-phpunit-gate will be deleted once this merges.