Skip to content

[2.x] ci(core): overhaul reusable backend workflow#4693

Draft
DavideIadeluca wants to merge 8 commits into
flarum:2.xfrom
glowingblue:di/reusable-backend-workflows-overhaul
Draft

[2.x] ci(core): overhaul reusable backend workflow#4693
DavideIadeluca wants to merge 8 commits into
flarum:2.xfrom
glowingblue:di/reusable-backend-workflows-overhaul

Conversation

@DavideIadeluca

@DavideIadeluca DavideIadeluca commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #0000

Changes proposed in this pull request:

  • Introduces a new db_prefixes input:
    Allows consuming workflows to pass in custom table prefix names or disable prefix testing altogether by passing '[""]'
  • Makes permutations of job executions predictable by dropping the include and exclude section in the matrix strategy:
    Calling workflows can now predictably customize which permutations across DB Versions, PHP Versions and now as well DB Prefixes are run. The previous implementation did intend to make it possible for DB versions and PHP Versions already, but it didn't actually work consistently and was hard to understand. E.g. Sqlite tests were still run even when overriding db_versions in a calling workflow.
  • Updates the used database versions for MySQL, MariaDB and PostgreSQL to the latest LTS versions:
    While right now probably just a tiny fraction of Flarum Communities are running on those DB versions, it future proofs the workflows and is a tradeoff between covering a wide blast radius whilst not exploding the amount of Jobs being run per push. I'm open though to add additional DB versions if requested
  • Upgrades actions/checkout from v4 to v6
  • Pins actions (actions/checkout and shivammathur/setup-php to immutable commit hashes):
    This allows consumer extensions to keep targeting 2.x for the reusable backend workflow whilst increasing protection against upstream supply chain attacks. Dependabot creates PR's to bump actions. This is the same approach Laravel does, see Bump the github-actions group with 4 updates laravel/framework#60364 for an example.
  • Sets fail-fast to false to continue run other jobs when a single fails:
    This allows to get a full picture of the backend flows in one go.
  • Renames the backend test job names to be consistent with the structure of PHPStan jobs
  • Simplifies terminology and removes some comments

Reviewers should focus on:

  • The GitHub Diff is pretty messed up. I recommend to use the Split View to review the changes
  • When this gets merged I intend to cleanup the frontend reusable workflow as well (mainly thinking of pinning external actions right now) and reviewing the other workflows as well

Screenshot

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

@DavideIadeluca DavideIadeluca marked this pull request as ready for review June 5, 2026 14:33
@DavideIadeluca DavideIadeluca requested a review from a team as a code owner June 5, 2026 14:33
Comment thread .github/workflows/REUSABLE_backend.yml Outdated
@imorland

Copy link
Copy Markdown
Member

I added this to the rc.4 milestone, even tho this does not need a tag or release as such. This is so it gets documented in the release announcement, for visibility purposes

@DavideIadeluca

Copy link
Copy Markdown
Contributor Author

Documentation PR: flarum/docs#534

@imorland

Copy link
Copy Markdown
Member

Thanks for taking this on — the matrix is genuinely hard to reason about today, and dropping the include/exclude sprawl for a clean cartesian is the right direction. The action SHA-pinning (and fixing the unpinned checkout@master in the PHPStan job) and fail-fast control are all welcome.

I'd like to discuss a couple of points before merging — let me know your thoughts on these:

1. Keep the floor — test minimum and latest

The new defaults move to latest-only DBs (mysql:9.7, mariadb:12.3, postgres:18). We can't drop the older versions: 2.0 needs to stay as widely installable as realistically possible, and a large share of shared-hosting installs are still on MySQL 5.7 / MariaDB 10.x. CI is what keeps that support honest — the floor job doesn't guard against the old DB changing (it won't), it guards against our code drifting away from minimum-DB compatibility (a migration using a function 5.7 lacks, an index name too long for old MariaDB, etc.). Drop it and the first signal of breakage is a user bug report.

So I'd test both ends of the supported window — the minimum we claim to support and the current latest — and treat both as gating. We don't need the floor on every PHP though: someone still on MySQL 5.7 is realistically on older PHP too, and the SQL we generate doesn't branch on PHP version, so floor-DB on the oldest PHP only is enough:

include:
  - php: ${{ fromJSON(inputs.php_versions)[0] }}
    db: mysql:5.7
    prefix: ''
  - php: ${{ fromJSON(inputs.php_versions)[0] }}
    db: mariadb:10.11
    prefix: ''
  - php: ${{ fromJSON(inputs.php_versions)[0] }}
    db: postgres:10
    prefix: ''

(Latest of each engine stays in the main cartesian; SQLite is 3 throughout — no separate floor.) Worth aligning the exact minimums with the supported versions listed in install.md so CI and the docs agree.

2. Prefix testing doesn't need to be a full matrix dimension

Right now db_prefixes: ['', 'flarum_'] multiplies the whole matrix (×2 across every PHP × DB). What prefix testing actually catches is identifier-length limits (index/constraint names exceeding 64 chars on MySQL/MariaDB) — that's driver-fixed, not PHP- or version-dependent, so we're running the same code path ~12 redundant times. A single representative job is enough:

include:
  - php: ${{ fromJSON(inputs.php_versions)[0] }}
    db: mariadb:12.3
    prefix: flarum_

MySQL/MariaDB specifically, since the 64-char limit is tightest there (SQLite has no real limit, Postgres is 63 and rarely the issue). That lets us drop the db_prefixes input and the prefix dimension entirely — prefix coverage becomes one deterministic include cell.

Net of both: roughly 15–16 jobs instead of 24, with minimum + latest DB coverage and prefix coverage all represented.

Minor: the old matrix used php_versions[3] to pin special cases to "latest PHP", which silently breaks if the array length ever changes (part of why the old excludes were confusing). For the include cells above, [0] is always safe; I'd avoid hardcoded indexes like [3].

None of this is far off — happy to look again once the floor/prefix shape is settled. You mentioned being open to adjusting DB versions, so hopefully this lands as expected.

@DavideIadeluca DavideIadeluca marked this pull request as draft June 10, 2026 13:13
@DavideIadeluca

Copy link
Copy Markdown
Contributor Author

TL;DR AI Summary:
Three proposals for the CI matrix reusable workflow:

  1. DB versions: broad DB floor/ceiling coverage only in the framework's own workflow; calling workflows default to latest versions only. Revive a manual pre-release workflow for exhaustive DB matrix runs.
  2. fail_fast: keep default true, but explicitly set false in the framework's backend.yml so all DB failures surface instead of cancelling early.
  3. Prefix testing: acknowledged it's redundant across PHP versions, but the job count reduction may not be worth the unpredictability; free minutes and high concurrency limits make it a non-issue for most extensions.

  1. Keep the floor — test minimum and latest

After our discussion I agree that we should also test the framework against older database versions to keep the confidence that the framework is compatible with the widley used versions, however expanding the matrix thru the include section like described in your comment would reintroduce one of the main issues with the previous implementation in that calling workflows cannot predictably narrow down the tested databases with the db_versions input. If for example a calling workflow inputs db_versions: '["postgres:18"]', tests are still run for MariaDB and MySQL.

One additional thing I think we should split apart here is:

  1. how flarum/core and first party extensions are tested
  2. how 2nd party (FoF) and third party extensions are tested.

Would you agree that it makes sense to have broader DB version coverage just in the framework but leave the latest versions of MySQL, MariaDB, PGSQL and Sqlite as the default for calling workflows? I did a small POC where I input db_versions: '["mysql:5.7", "mysql:9.7", "mariadb:10.11", "mariadb:12.3","postgres:10", "postgres:18", "sqlite:3"]' in backend.yml (effectively the calling workflow in the framework) which does the floor testing. But of course this doesn't address the number of jobs as also the floor DB versions are tested against all PHP versions. More on that further below.

What I would circle back here is what we also discussed in potentially reviving the pre release workflow or setting up a new one which can be called manually at any time (though usually before a new release) which runs the workflows for a greater set of DB versions. In such a scenarion job concurrency and with that time until all jobs are done is less of an issue while still having the confidence that the framework doesn't just support the newest, but also the oldest "floor" versions and potentially also some versions in between. In such a scenario it also wouldn't matter as much DB Prefix tests are run against the same DB version for multiple PHP versions.


About the feedback for the fail_fast change. For further debugging I set it to false (overriding the default of true) and actually now have a good example in this very PR where for some reason only mariadb:10.11 started to fail after roughly 3 minutes. No other DB's are effected. With fail_fast set to true (as is the default) it would never really surface that the issue is just contained to mariadb:10.11 and instead I would infer it as being an issue for all database versions. Knowing this a developer can make a better informed decision of how to debug and approach an issue considering it only manifests in this permutation. The big picture here helps a lot.

Here is my middle way proposal: We leave the default to true (all jobs are canceled if one fails) but actually set it to false in backend.yml which calls the reusable backend workflow in the framework?


  1. Prefix testing doesn't need to be a full matrix dimension

I agree that practically there are no cases where a prefix test would break on one PHP version but not the other. Having said that, I'm uncertain if it's worth it to keep some of the unexpected behaviour to decrease the job count as is currently by ~30% (though the impact would become more exponential if e.g. new PHP versions are introduced and we wanna keep the default PHP versions (i.e. not remove 8.3 even after PHP 9 was released).

For the vast majority of Flarum Extensions which are open source this isn't an issue and has no implications and costs as action minutes are free for public repositories. The concurrency of the default github hosted runner is 60 meaning there is still room for growth with adding more PHP versions until the concurrency is reached and jobs are stuck in the queue until they are picked up. For high pace vendors where multiple developers are creating pull requests at the same vendors can use the runner_type input and use a custom github hosted runner with a max concurrency of 1000. This if course would introduce extra costs for both public and private repos but from experience isn't really an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants