Skip to content

fix(ci): make composer check:strict able to fail on tests - #101

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/check-strict-can-fail
Aug 2, 2026
Merged

fix(ci): make composer check:strict able to fail on tests#101
rubenvdlinde merged 1 commit into
developmentfrom
fix/check-strict-can-fail

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Problem

test:all and test:unit ended in:

./vendor/bin/phpunit --colors=always || echo 'Tests require Nextcloud environment, skipping...'

phpunit's exit status was discarded unconditionally, so composer check:strict could never fail on a test failure. The excuse in the message is not true for this repo — the suite runs standalone.

This matters because check:strict is about to be the thing an agent checks before deciding its job is done. A step that cannot fail satisfies that instruction with a no-op.

Positive control — measured on this tree

PHP 8.3.32 container, vendor/ freshly installed from composer.lock. One deliberately failing test added to tests/:

state composer check:strict
old composer.json + failing test exit 0 — ALL CHECKS PASSED, with the failure printed right above it — test:all never named, Tests require Nextcloud environment, skipping... printed instead
new composer.json + failing test fails naming test:all
new composer.json, test removed exit 0, clean — 754 tests, 2277 assertions

The control test is not in this PR — the diff is composer.json only.

Change

composer.json only, using the guard already shipped in shillinq and openregister, plus --no-coverage:

if [ -f vendor/bin/phpunit ]; then ./vendor/bin/phpunit --colors=always --no-coverage; else echo 'phpunit not installed, skipping...'; fi

--no-coverage is deliberate: this repo's phpunit.xml requests coverage, and without a coverage driver PHPUnit emits a runner warning and exits non-zero. Without the flag the newly-live gate would go red on any machine lacking pcov/xdebug — for an environment reason, not a code reason, which is the fastest way to get a gate ignored again. shillinq and decidesk already pass --no-coverage here.

Policy / state after this PR

composer check:strict now exits 0 on this branch and every tool really runs: lint, phpcs, phpmd, psalm, phpstan all pass and test:all runs 754 tests / 2277 assertions green. Nothing needed fixing — but until this PR none of that was being asserted for tests.

This gives the sharpest form of the control: exit 0 → exit 1 (injected failure) → exit 0 (reverted), on one tree.

test:all (and test:unit where present) ended in '|| echo Tests require
Nextcloud environment, skipping...', so phpunit's exit status was
discarded unconditionally and check:strict could never fail on a test
failure. This repo's suite runs standalone, so the message was untrue here.

Positive control, PHP 8.3.32 container, vendor/ freshly installed, adding
one deliberately failing test under tests/:
  old composer.json + failing test -> test:all never named, swallowed
  new composer.json + failing test -> check:strict fails NAMING test:all
  new composer.json, test removed  -> test:all passes

Also passes --no-coverage: this repo's phpunit.xml requests coverage, and
without a driver PHPUnit warns and exits non-zero, which would have made
the newly-live gate red for an environment reason rather than a code one.
shillinq and decidesk already do this.

Tooling only - no findings fixed here.
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ 7f60226

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 662/662
PHPUnit
Newman ⏭️
Playwright ⏭️

Coverage: 89.4% (17/19 statements)


Quality workflow — 2026-08-02 22:14 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 9f3f929 into development Aug 2, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant