Split the matrix computation into its own reusable workflow - #277
Draft
swissspidy wants to merge 3 commits into
Draft
Split the matrix computation into its own reusable workflow#277swissspidy wants to merge 3 commits into
swissspidy wants to merge 3 commits into
Conversation
Prototype. A matrix becomes a collapsible group in the Actions run view only when it sits on a job declared in the workflow the run belongs to. In the current shape the matrix is one level down, inside reusable-testing.yml, so it is not surfaced and all fifty legs appear as one flat list under a single "test" header. That is why the earlier attempt to group them by moving the PHP version into the calling job's name failed: there was no group to move it to. Move the `prepare` job into reusable-prepare-matrix.yml and expose the two matrices as workflow_call outputs. A package can now call that directly and run the legs from its own top-level jobs, which puts the matrix where the run view will group on it. reusable-testing.yml keeps its four inputs and now delegates to the new workflow, so every existing caller is unaffected and adoption is opt-in per package. Verified that both shapes pass identical `with:` blocks to the called workflows, that the public inputs are unchanged, and that the extracted matrix logic produces byte-identical output — 41 functional and 9 unit legs on a pull request, 51 and 12 on the schedule. This repository's own testing.yml adopts the fanned-out shape as the reference implementation, referring to the workflows by local path so that a pull request here exercises its own changes rather than whatever is on main. The cost is documented in the README rather than hidden: the fan-out is about thirty lines in a file that is not synced, so future changes to it have to be repeated per package. Leg names stay self-describing in both shapes, which means the grouped view repeats the PHP version inside the group. Worth revisiting once the grouped run can actually be seen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Confirmed against wp-cli/wp-cli-tests#352 that the fanned-out shape does group the legs in the run view. What it also showed is that the leg names then repeat their group: a group called "Behat | PHP 8.5" containing "Behat | PHP 8.5 | WP latest | SQLite". wordpress-develop handles this by suppressing the prefix in the called workflow whenever the calling job already states it — its leg name only includes "PHP {0} with" for the test-group and coverage callers, which are exactly the ones whose job name is not "PHP {0}". Do the same, with an explicit `grouped` input rather than inferring it from unrelated inputs. Wrapped callers leave it at its default of false and are unaffected; their names still have to stand on their own because their calling job is not surfaced. The expression is written as `!grouped && <prefix> || ''` rather than the more natural looking `grouped && '' || <prefix>`, because an empty string is falsy and the latter would fall through to the prefix in both cases. That is the same trap that made fetch-depth always evaluate to 1 earlier in this branch; zizmor's unsound-ternary audit is clean on the form used here. Noted in the README that a grouped leg name is only unique within its group, so the back-out is to set `grouped: false` for that caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
Verified against wp-cli/wp-cli-tests#352. Grouping Behat by PHP version works — two to eight legs per group — but the unit matrix has exactly one leg per PHP version, so the same treatment produced nine groups containing one job each. Give the unit fan-out a single "Unit" group and let the PHP version distinguish the legs inside it. The grouped form of the unit name therefore keeps the version, rather than collapsing to a bare constant the way the Behat one does; the version is the only thing that tells those legs apart. Behat is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
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.
Draft. Prototype for evaluation — the payoff is presentational and has not been observed working yet. See the caveat at the bottom before merging.
Why
A matrix becomes a collapsible group in the Actions run view only when it sits on a job declared in the workflow the run belongs to. Today the matrix is one level down, inside
reusable-testing.yml, so it is not surfaced: all fifty legs appear as one flat list under a singletestheader.That is also why the attempt in #274 to group the legs by moving the PHP version into the calling job's name failed, and had to be reverted in #276. There was no group to move it to — the version was not moved, it was hidden.
What this does
reusable-prepare-matrix.yml(new) holds thepreparejob, moved verbatim, and exposes the two matrices asworkflow_calloutputs. A package can call it directly and run the legs from its own top-level jobs, which puts the matrix where the run view will group on it.reusable-testing.ymlkeeps its four inputs and delegatesprepareto the new workflow. It drops from 546 lines to 87. Every existing caller is unaffected; adoption of the new shape is opt-in per package.This repository's own
testing.ymladopts the fanned-out shape as the reference implementation. It refers to the workflows by local path rather than@main, so a pull request here exercises its own changes instead of whatever is onmain— which closes a real gap, since until now a pull request against this repository never tested the reusable workflows it was changing.Verified
with:blocks to the called workflows (asserted, not eyeballed).reusable-testing.ymlare unchanged.The cost, documented in the README rather than hidden
The fan-out is about thirty lines in
testing.yml, which is not synced. Every future change to it has to be repeated in each package that adopts the shape. Leg names stay self-describing in both shapes, because the same called workflows serve both, so the grouped view repeats the PHP version inside the group. Worth revisiting once a grouped run can actually be seen.The version that removes this cost is larger: derive
minimum-phpfromcomposer.json'srequire.phpand move the remaining overrides to a per-package config file, at which pointtesting.ymlcarries no per-package configuration and can be synced. Not attempted here.Before merging
The grouping is unverified. It is reasoned from the observation that only the top-level
testjob produced a collapsible header in wp-cli/wp-cli-tests run 31163465254, not from seeing the grouped view work. The intended check is to merge this, convert one package'stesting.yml, and look at the run.If the grouping does not appear, this bought nothing and should be reverted. Nothing else depends on it — the split is otherwise behaviour-neutral.
Generated by Claude Code