fix(ci): make composer check:strict able to fail on tests - #196
Merged
Conversation
test:unit and test:all ended in '|| echo Tests require Nextcloud environment, skipping...', discarding phpunit's exit status unconditionally. Every other tool in check:strict passes here, so the whole command was green with the test gate switched off. The excuse in that message is actually TRUE for this repo - with a freshly installed vendor/ the suite still dies on 'Class OC\Mail\EMailTemplate not found' - but the mask was not conditional on it, so a genuine failure inside CI looked identical to 'no Nextcloud here'. Guarded on the real precondition instead, plus --no-coverage. Positive control (PHP 8.3.32 container, app mounted at /nc/apps-extra/nldesign): no server tree -> exit 0, loud SKIPPED, test:all not named server tree present -> exit 1, check:strict NAMES test:all (code 255) old composer.json -> same phpunit failure, test:all never named Tooling only.
Contributor
Quality Report — ConductionNL/nldesign @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-08-02 22:18 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
test:unitandtest:allended in:phpunit's exit status was discarded unconditionally, so
composer check:strictcould never fail on a test failure — and this repo'scheck:strictpasses everything else, so the whole command was green with the test gate switched off.Here the excuse in the message turns out to be true: with a freshly installed
vendor/the suite still dies withClass "OC\Mail\EMailTemplate" not found(an internalOC\class that only exists inside a running Nextcloud). But the mask was never conditional on that, so a genuine failure inside CI — where the tree is present — looked identical to "no Nextcloud here".Change
composer.jsononly. Replace the blanket mask with the actual precondition, plus--no-coverage:The plain
if [ -f vendor/bin/phpunit ]guard used elsewhere in the fleet is deliberately not used here: it would makecheck:strictpermanently, falsely red on every machine outside a Nextcloud tree, which is the fastest way to get a gate ignored again.Positive control — measured on this tree
PHP 8.3.32 container,
vendor/freshly installed, app mounted at/nc/apps-extra/nldesignso../../lib/base.phpis under my control:composer check:stricttest:allprintsSKIPPED: the unit suite needs a Nextcloud server treecheck:strictnamingtest:all(error code 255)composer.json, same phpunit failuretest:allnever named — swallowed by|| echoHonest caveat: in the middle row the "server tree" is a minimal stand-in defining
OC_App/OC_Hook, which is enough for phpunit to start but not enough for this suite to pass — so what that row proves is that phpunit's exit status now propagates, not that a specific injected assertion failed. A real Nextcloud tree (which CI provides) is needed for the suite itself.Policy / state after this PR
Tooling only — no findings are fixed here.
check:strictexits 0 outside a Nextcloud tree (all oflint,phpcs,phpmd,psalm,phpstanpass,test:allskips loudly). The real follow-up this exposes is that the unit suite cannot run standalone —tests/Unit/Controller/SettingsControllerAuditTest.phpand others reach for internalOC\classes. That was completely invisible while the mask was in place.