[mlapaglia]

The PHP test matrix jobs are never created when the tests job is skipped on a pull request without PHP changes, so requiring them blocks frontend-only PRs. The new job passes when the tests passed or were skipped because no PHP changed, and fails otherwise, including a skip after a failed prerequisite. Branch protection should require it instead of the matrix jobs.

Summary by CodeRabbit

• Chores

  • Pull requests now trigger CI even when they only change Markdown. PHP tests run when relevant PHP code, tests, or tooling changes, and an always-running check reports whether they passed, were appropriately skipped, or failed.
  • The PHP test result check fails if tests fail, are unexpectedly skipped, or change detection does not succeed.

• Documentation

  • Updated the quality-gate guide with the PHP test check’s behavior and the check to require for branch protection.

[coderabbitai]

Walkthrough

The workflow updates pull-request triggering and PHP change detection. It adds an always-running check that reports whether PHP tests passed or were correctly skipped. The quality-gate document describes the check outcomes and branch-protection use.

Changes: PHP Test Result Check

Layer / File(s)	Summary
PHP change detection .github/workflows/CICD.yml	The pull-request trigger no longer excludes Markdown-only changes. The PHP change filter now includes bootstrap, public, and tests PHP files, plus .github/workflows/php_tests.yml.
PHP test result check and guidance .github/workflows/CICD.yml, docs/specs/5-operations/quality-gate.md	The workflow adds an always-running result check. It passes when tests succeed or are skipped because a pull request has no PHP changes. It fails when change detection fails or tests fail, are cancelled, or are unexpectedly skipped. The document describes these rules, identifies the check for branch protection, and updates its displayed dates.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Moderate · up to 55ed6

The new 1️⃣ PHP tests result check can report success when a PHP syntax check failed on a language-file-only pull request. Make the result job depend on the syntax check before branch protection relies on it.

Pre-merge checks: 1

Check name	Status	Explanation
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{I’m a rabbit; tests now run,
Or skip when PHP changes none.
A steady check reports the state,
And guides the gate beside the date.
I nibble greens and hop away,
With clear results to start the day.}

[coderabbitai]

Actionable comments posted: 2

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4c73d947-f23b-4d85-b001-8e9299bcc887

Commits

Reviewing files that changed from the base of the PR and between 4e5d5c8 and 7413f44.

Files selected for processing: 2

• .github/workflows/CICD.yml
• docs/specs/5-operations/quality-gate.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Major · Reject skipped tests after a failed prerequisite. · CICD.yml:241-243

.github/workflows/CICD.yml:241-243
Major · Functional Correctness · Quick win

Reject skipped tests after a failed prerequisite.

On a pull request that changes only lang/**, php_syntax_errors still runs. If it fails, tests is skipped. TESTS_EXPECTED remains false, so this branch exits successfully and marks 1️⃣ PHP tests result as passed. This violates the documented rule that a skipped test after a failed prerequisite must fail. (raw.githubusercontent.com)

Add php_syntax_errors to tests_result.needs. Accept this skip only when that prerequisite is success or intentionally skipped.

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3989f2a0-a0a4-4519-aa8a-13e4542e10da

Commits

Reviewing files that changed from the base of the PR and between 7413f44 and 55ed686.

Files selected for processing: 2

• .github/workflows/CICD.yml
• docs/specs/5-operations/quality-gate.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

[ildyria]

The PHP test matrix jobs are never created when the tests job is skipped on a pull request without PHP changes, so requiring them blocks frontend-only PRs. The new job passes when the tests passed or were skipped because no PHP changed, and fails otherwise, including a skip after a failed prerequisite. Branch protection should require it instead of the matrix jobs.

That is not a problem because in those case we override the requirement. The php tests take about 1h20 to run. We do not want to wait for those when the changes are only front-end.

The issue is the inflexibility of the tests requirements in Github to be able to say which tests are required depending of which paths. :(

[mlapaglia]

Sorry, the description wasn't clear: this does not run the PHP tests on front-end-only PRs. The 1h20 matrix still only runs when the php path filter matches. When it doesn't, 1️⃣ PHP tests is skipped as today, and the new 1️⃣ PHP tests result job only reads that outcome and finishes in seconds. You can see it on this PR: 1️⃣ PHP tests was skipped and 1️⃣ PHP tests result passed in 9 s.

It is meant as a workaround for the path-dependent requirement you mention. GitHub can't require checks per path, but it can require one check that always reports and whose own logic is path-aware. If 1️⃣ PHP tests result replaces the six 1️⃣ PHP tests / 8.5 - postgresql -- … checks in branch protection:

• front-end-only PRs become mergeable without an override, and still run no PHP tests;
• PHP PRs still can't merge until the full matrix passes, because the job fails when any test job fails, is cancelled, or was skipped although PHP changed.

The second commit (from review) has two side effects worth knowing about:

• The pull_request trigger no longer ignores Markdown, so docs-only PRs get the required checks instead of none. This also runs the Dockerfile lint jobs on them.
• Changes to PHP files in tests/, bootstrap/ and public/, and to php_tests.yml, now run the PHP tests. Before, they did not.

I'm happy to drop either of these. And if you'd rather keep overriding, feel free to close this.

[ildyria]

Hooo nice!