What happened today
Two repos had every PHPUnit cell red on development at the same time, for the same reason, and neither had a failing assertion.
dossiq: Tests: 3651, Assertions: 20819, Skipped: 1, Risky: 6. — EngineTaskInboxTest executes EngineInboxQuery without naming it (#2387).
pipelinq: Tests: 2886, Assertions: 11394, Warnings: 2, Skipped: 9, Incomplete: 3, Risky: 2. — ConnectorEgressTest executes ConnectorSourceRegister without naming it (ConductionNL/pipelinq#1910).
Both print OK, but there were issues! and exit 1.
Why every app will keep finding it this way
beStrictAboutCoverageMetadata makes a test risky when it executes a class its @covers block does not name, and failOnRisky turns that into a red suite. Both settings are inert without a coverage driver, and neither pcov nor xdebug is installed on the development machines. So the check cannot fire locally, no matter how carefully anyone runs the suite before pushing. composer check:strict passes. The first signal is a red cell in CI.
pipelinq's test file already carried a docblock note explaining this trap, written by whoever hit it on FleetAppId. It was accurate and it did not prevent the next occurrence, because the note lives in the file that already got fixed.
What makes it worse
Adding a collaborator to an existing test is a completely ordinary edit. Nothing about it looks like it touches coverage metadata. In dossiq the trigger was #2374, a feature change; in pipelinq it was a slug-resolution change. Neither author had any reason to think about @uses.
And on development the failure is invisible, because push runs there are cancelled: the standing "Release: merge development into beta" PR carries development as its head ref, so its run shares the concurrency group and cancel-in-progress kills the push run. The dashboard shows cancelled, which reads as noise. dossiq's development had been red for hours with no verdict published anywhere. It took gh workflow run code-quality.yml --ref development, which sits in its own lane, to see it.
Options, in rough order of how much I would trust them
- Install a coverage driver in the dev environment (pcov is fast enough for this) so
composer test reproduces CI. Highest fidelity, and it removes a whole class of "green locally, red in CI".
- Add a composer script that runs PHPUnit with coverage enabled purely to surface risky tests, and document it as the pre-push check for anyone touching tests.
- Reconsider whether
failOnRisky should gate the fleet at all when the condition it fires on is unobservable to the author. That is a real question, not a rhetorical one: the setting catches genuinely sloppy coverage metadata, but the cost is currently paid as a red default branch found hours later by someone else.
I am not picking one, because the trade-off belongs to whoever owns the fleet test configuration. Both today's instances are fixed.
Related
.github#740 — the PHPUnit job runs the suite twice under coverage inside a 45-minute cap- The concurrency behaviour is documented at length in each repo's
code-quality.yml
What happened today
Two repos had every PHPUnit cell red on
developmentat the same time, for the same reason, and neither had a failing assertion.dossiq:
Tests: 3651, Assertions: 20819, Skipped: 1, Risky: 6.—EngineTaskInboxTestexecutesEngineInboxQuerywithout naming it (#2387).pipelinq:
Tests: 2886, Assertions: 11394, Warnings: 2, Skipped: 9, Incomplete: 3, Risky: 2.—ConnectorEgressTestexecutesConnectorSourceRegisterwithout naming it (ConductionNL/pipelinq#1910).Both print
OK, but there were issues!and exit 1.Why every app will keep finding it this way
beStrictAboutCoverageMetadatamakes a test risky when it executes a class its@coversblock does not name, andfailOnRiskyturns that into a red suite. Both settings are inert without a coverage driver, and neither pcov nor xdebug is installed on the development machines. So the check cannot fire locally, no matter how carefully anyone runs the suite before pushing.composer check:strictpasses. The first signal is a red cell in CI.pipelinq's test file already carried a docblock note explaining this trap, written by whoever hit it on
FleetAppId. It was accurate and it did not prevent the next occurrence, because the note lives in the file that already got fixed.What makes it worse
Adding a collaborator to an existing test is a completely ordinary edit. Nothing about it looks like it touches coverage metadata. In dossiq the trigger was #2374, a feature change; in pipelinq it was a slug-resolution change. Neither author had any reason to think about
@uses.And on
developmentthe failure is invisible, because push runs there are cancelled: the standing "Release: merge development into beta" PR carriesdevelopmentas its head ref, so its run shares the concurrency group andcancel-in-progresskills the push run. The dashboard shows cancelled, which reads as noise. dossiq'sdevelopmenthad been red for hours with no verdict published anywhere. It tookgh workflow run code-quality.yml --ref development, which sits in its own lane, to see it.Options, in rough order of how much I would trust them
composer testreproduces CI. Highest fidelity, and it removes a whole class of "green locally, red in CI".failOnRiskyshould gate the fleet at all when the condition it fires on is unobservable to the author. That is a real question, not a rhetorical one: the setting catches genuinely sloppy coverage metadata, but the cost is currently paid as a red default branch found hours later by someone else.I am not picking one, because the trade-off belongs to whoever owns the fleet test configuration. Both today's instances are fixed.
Related
.github#740— the PHPUnit job runs the suite twice under coverage inside a 45-minute capcode-quality.yml