Skip to content

feat: Split ci into layer-scoped workflows - #299

Merged
Dumea Alexandru (alexdumea) merged 19 commits into
mainfrom
201/split-ci
Oct 27, 2026
Merged

Dumea Alexandru (alexdumea) merged 19 commits into
mainfrom
201/split-ci

Conversation

@alexdumea

@alexdumea Dumea Alexandru (alexdumea) commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Closes #201.
Closes #202.

Needs a repository admin before or right after the merge: in Settings → Branches (or Rules → Rulesets), the protection rule for main must require only the validation-gate check (source: GitHub Actions).
The split renames almost every job (for example php-quality (php-8.1) becomes php / php-quality (php-8.1), and future-compatibility becomes php / phpstan-trunk).
Required checks with the old names never report again, so every pull request would wait on them forever.
Remove every old per-job check, and do not require dispatch-gate, nightly-gate or the layer gates (php-check, admin-check, storefront-check, acceptance-check): they do not run on pull requests, or validation-gate already covers them.
validation-gate keeps its name, so it can be required before the merge already.

ci.yml had grown into one 700-line file that did everything.
It also had a shortcut that let a green check mean nothing:

  • Pushes to main could skip all tests.
    verify-main-validation reused the merged pull request's results, even though the merge commit can contain different code.
    It then set run_full_matrix=false, and validation-gate reported success without any job having run.
  • The release trusted that check blindly.
    store-release.yml accepted any check run named validation-gate on the commit.
    It could not tell "all passed" from "all skipped", and any workflow or app can create a check with that name.
  • The gate's job list was kept by hand, in two places.
    expected_checks and the needs of validation-gate had drifted apart: docs-lint was in the first but not in the second.
  • The acceptance suite never ran in CI.
    tests/acceptance was only run locally.

What changed

ci.yml is now an orchestrator.
It calls one reusable workflow per layer and aggregates them in validation-gate:

Workflow Covers
php.yml PHP quality, PHPStan against trunk (was future-compatibility), the Shopware lanes, BC check, MySQL 8.4 install, advisory checks
admin.yml Administration lint, Jest, and the per-lane webpack and Vite builds
storefront.yml The storefront lanes
acceptance.yml The tests/acceptance Playwright suite: lint, typecheck, Signer, the lane × shard matrix, and the changed specs repeated ten times
package-zip.yml Unchanged: builds and install-tests the release archive
nightly.yml New: all layers once a day on main, every acceptance lane repeated three times

Each layer has its own *-check gate, so a skipped job inside a layer stays visible to ci.yml.
Every layer also keeps a workflow_dispatch trigger and can run on its own.
Every check that ran before the split still runs, with the same lanes and steps; docs-lint is now part of the gate.
bc-check is new: an advisory job that runs the Roave backward-compatibility check (composer bc-check, HEAD^ against HEAD, which on a pull request is its base against its merge), so a break is reported without blocking the merge.

Every run validates its own commit

  • No fast path.
    verify-main-validation and run_full_matrix are gone.
    Every push, pull request and manual run runs the full validation on the commit it is for.
  • The gate cannot pass by skipping.
    validation-gate and every layer gate use re-actors/alls-green with if: always().
    A skipped job fails the gate unless it is explicitly allowed to skip.
    The only allowed skip in ci.yml is acceptance-full on a pull request without the label.
  • No hand-kept job list.
    expected_checks is replaced by the gate's needs.
    The new workflow-lint job runs bin/ci-assert-gate-coverage.sh, which fails if any job of a gated workflow is missing from its gate.
    A job that should not block is made advisory with continue-on-error, not left out of the gate.
    This is now how sdk-main-compatibility, bc-check and composer-security-report stay non-blocking.
  • Concurrency only cancels superseded pull request runs.
    A push, tag or manual run on main never cancels the run a release reads.

Stricter release gate

store-release.yml no longer looks for a check run by name.
It now requires:

  • the newest ci.yml push run on main for the release commit,
  • that run to be completed, so an older green run cannot outvote a newer one that is still running or failed,
  • a successful validation-gate job in that run's latest attempt.

Otherwise it refuses to publish and names the run it found, or says that none exists.
A manual CI run reports its gate as dispatch-gate, and the nightly reports nightly-gate, so neither can make a commit releasable.

Acceptance suite in CI

  • Pull requests run the trunk lane.
  • The 6.5 and 6.6 lanes run on every push and manual run.
    A pull request opts in with the run/acceptance-full label; adding the label starts a new run.
  • Changed specs are repeated ten times (acceptance-tests-changed), on trunk, without retries, on two workers, to catch flaky tests before they land:
    • only specs this pull request changes run; Setup and Signer run once as dependencies, and Setup, Signer and KnownBlocked specs are not repeated;
    • the spec paths reach Playwright as filters, without a -- separator, which would make Playwright drop them and run the whole suite;
    • a changed spec that selects no test (no project tag, or a name Playwright reads as a regular expression) fails the job instead of passing as "no tests";
    • on failure the job uploads the failures' traces and the shop's var/log as acceptance-tests-changed-failure, so a failed ATS request can be read with its response.
  • One report per run.
    acceptance-report merges the shard reports into one HTML report, tagged by lane, and writes failed and flaky tests to the step summary.
  • playwright.config.ts tags tests with their lane, and no longer builds the own-host mapping in signer-only mode.
    new URL('/') threw there, because test:signer runs without APP_URL.

Acceptance tests on 6.5, 6.6 and under repetition

The @UcpAdmin specs had only run on trunk.
On the 6.5 and 6.6 lanes, and under the ten-times repetition, they exposed test problems and plugin bugs.

Test problems, fixed in the suite:

  • Update notification. On older versions, core's "a new Shopware version is available" notification covered the Filters button for a freshly logged-in admin; the fresh-admin login now answers the update check with no version.
  • Stored list filters. 6.5 stores list filters through /api/user-config, not /api/_info/config-me; the filter tests wait for either.
  • Template modal. 6.5 renders the modal's Cancel button without a variant; the page object selects the footer button that is not primary.
  • Duplicate test hook. On 6.6, mt-switch copies ac-ucp__expose-switch to its container and its field; the page object takes the first match.
  • Reload after saving. Before 6.7.9 the save reloads the UCP state once more, and a reload right after it could read a response of the page being left; the tab now only accepts responses started after the navigation.

Stability under ten parallel repetitions of export-channel-attribution.spec.ts:

  • List search. The list tests now wait for the list's own search request before asserting rows; under load it answered after the 15-second assertion timeout.
  • User creation. Since 6.7.13.1 core revokes a new user's refresh tokens inside the user insert, and the unindexed delete on refresh_token fails with MariaDB error 1020 while other workers log in (HTTP 500).
    User creation retries only that error, with a jittered backoff, and reports status and body otherwise.
    UcpAclUsers no longer depends on the ATS AdminSession, whose user insert has no such retry.
  • Cleanup errors. The ATS cleanup error now lists each attempt's message; Playwright used to drop them.

Plugin bugs, registered as known blockers with KnownBlocked specs:

The feed channel's insights totals are checked from 6.7.10 only, where core's product export insights show them; the plugin's own statistics tab before that shows the charts only.

  • Tests cover the change, or it needs none, such as documentation or CI configuration.
  • A change merchants notice has an entry under # next version in both changelogs.
  • The change works on Shopware 6.5, 6.6 and 6.7 from one codebase.

@alexdumea Dumea Alexandru (alexdumea) linked an issue Oct 19, 2026 that may be closed by this pull request
5 tasks
@alexdumea Dumea Alexandru (alexdumea) added the run/acceptance-full Runs the 6.5.x and 6.6.x acceptance lanes on this PR, in addition to the default checks label Oct 20, 2026
@alexdumea Dumea Alexandru (alexdumea) linked an issue Oct 20, 2026 that may be closed by this pull request
4 tasks

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Changed-spec validation can pass while silently omitting an unmatched spec when another changed spec selects tests.

1 open issue
What changed in this PR

Splits CI into reusable layer workflows, strengthens commit-specific release validation, and brings the acceptance suite into CI, addressing #201 and #202.

Changes:

  • Adds layer gates, gate-coverage checks, and scheduled full-lane validation.
  • Requires release validation from the latest applicable CI push run on main.
  • Adds acceptance repetition and reporting, improves cross-version test stability, and records known blockers.
File Description
tests/​acceptance/​tests/​UcpContent/​knownBlockerRanges.ts Defines version ranges for new blockers.
tests/​acceptance/​tests/​UcpAdmin/​product-feed-export.spec.ts Adds save and template-cancellation checks.
tests/​acceptance/​tests/​UcpAdmin/​export-channel-attribution.spec.ts Stabilizes searches and adapts version-specific assertions.
tests/​acceptance/​tests/​KnownBlocked/​product-feed-export.spec.ts Adds reproductions for D12 and D13.
tests/​acceptance/​services/​UcpTestDataService.ts Narrows user-creation retries to token conflicts.
tests/​acceptance/​playwright.config.ts Tags lanes and fixes signer-only configuration.
tests/​acceptance/​page-objects/​UcpSalesChannelDetail.ts Filters stale responses and handles duplicate hooks.
tests/​acceptance/​page-objects/​AgenticCommerceFeedDetail.ts Adds creation helpers and compatible modal selectors.
tests/​acceptance/​known-blockers.ts Registers blockers and updates issue links.
tests/​acceptance/​fixtures/​UcpTestData.ts Includes cleanup attempt errors in diagnostics.
tests/​acceptance/​fixtures/​UcpAclUsers.ts Creates admins lazily and suppresses update notifications.
docs/​releasing.md Documents non-blocking SDK compatibility checks.
CONTRIBUTING.md Updates advisory-job gating guidance.
bin/​ci-assert-gate-coverage.sh Checks that workflow gates cover every job.
bin/​ci-acceptance-report.sh Merges acceptance reports and summarizes failures.
.github/​workflows/​storefront.yml Extracts storefront validation and its gate.
.github/​workflows/​store-release.yml Verifies CI run identity and gate attempt.
.github/​workflows/​php.yml Extracts PHP validation and advisory checks.
.github/​workflows/​nightly.yml Schedules full-layer validation and repeated acceptance tests.
.github/​workflows/​ci.yml Orchestrates layers without the validation fast path.
.github/​workflows/​admin.yml Extracts Administration checks and coverage upload.
.github/​workflows/​acceptance.yml Adds lane execution, changed-spec repetition, and gating.
.github/​actions/​acceptance-report/​action.yml Shares report download, merge, and upload steps.
.github/​actions/​acceptance-lane/​action.yml Shares Shopware provisioning and acceptance setup.

Review effort: Balanced

Comment thread .github/workflows/acceptance.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Release authorization and required-check migration need human verification of CI outcomes and branch protection before approval.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Avoid repeating dependency projects with --repeat-each

.github/​workflows/​acceptance.yml:172

--repeat-each=10 also applies to the Setup and Signer dependency projects in playwright.config.ts. Running all changed specs in one invocation avoids scheduling those projects separately for each spec, but does not exempt them from repetition. Every changed-spec job therefore repeats the dependency suites ten times, including Setup's database writes, browser logins and signing-key checks. Run those projects once first, then repeat only the changed specs with --no-deps.

1 resolved since last review

Review effort: Balanced

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UcpAdmin and UcpAcl projects pass with this branch on Shopware 6.5 and 6.6 The new D12 and D13 specs fail on both versions, so both blockers reproduce.

Copilot's remaining comment on --repeat-each doesn't hold. With --list --repeat-each=10 on tests/UcpAdmin/product-feed-export.spec.ts, Playwright lists the 23 Setup and 30 Signer tests once. Only the 7 UcpAdmin tests grow to 70. The comment in the changed-spec step is therefore right that Setup and Signer execute once.

The description says that every check that ran before the split still runs. bc-check is a new advisory job, as it didn't run in ci.yml before. Please mention it as new in the description.

The run/acceptance-full label, the nightly run and the ten repetitions of changed specs aren't documented for contributors yet. Please add them to Running it in the acceptance README or to CONTRIBUTING.md.

Comment thread tests/acceptance/known-blockers.ts Outdated

test('an OpenAI feed is not saved without a return policy URL', async ({ TestDataService, AdminPage }) => {
test('an OpenAI feed is not saved without a return policy URL', async ({ InstanceMeta, TestDataService, AdminPage }) => {
test.skip(satisfies(InstanceMeta.version, SAVE_SHOWS_NO_REQUIRED_SETTING_ERROR), 'known blocker D12, asserted in tests/KnownBlocked/product-feed-export.spec.ts');

@dgrothaus-sw Dominik Grothaus (dgrothaus-sw) Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The skip reason says that D12 is asserted in tests/KnownBlocked/product-feed-export.spec.ts below 6.6. That spec only covers a new channel, though. According to the D12 description, the first click on a saved channel marks nothing on 6.5. That case has no spec that fails while the bug exists.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new saved-channel spec in KnownBlocked/product-feed-export.spec.ts passes on 6.5, so it doesn't reproduce D12 there yet.

open() returns while the detail page still shows six .sw-loader elements. A click on Save right after that marked the field in one of two runs. On a fully loaded page, the first click marked nothing in both runs, as the D12 description says.

openNew() returns even earlier, as soon as the field is visible. That hides the first-click half of D12 for a new channel. The new-channel spec fails on 6.5 only at the second click.

Please wait in open() and openNew() until the page has finished loading, for example until no loader is left. Then both D12 specs click Save on the page a merchant sees. The saved-channel spec then fails on 6.5 while the bug exists.

Comment thread tests/acceptance/tests/KnownBlocked/product-feed-export.spec.ts Outdated
Comment thread .github/workflows/acceptance.yml
Comment thread .github/workflows/acceptance.yml Outdated
Comment thread .github/workflows/acceptance.yml Outdated
# Two workers, the default on the 4-vCPU runner, so a smaller runner keeps the parallel load.
# No `--` before the specs: Playwright drops every argument after it from the test filter and
# runs the whole suite. The plan only emits paths starting with tests/, never an option.
npx playwright test --grep-invert @UcpKnownBlocked --repeat-each=10 --retries=0 --workers=2 --reporter=list "${specs[@]}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core's job of the same name repeats the changed specs with --repeat-each="10" -x (acceptance.yml, added in shopware/shopware#9990). The -x stops the run at the first failure. One failure is enough to show that a spec is flaky, so the remaining repetitions only cost runner time.

In the latest run, the repetition took 11 of the job's 25 minutes. Please add -x here too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done , now it will stop at first failure

@BrocksiNet Björn Meyer (BrocksiNet) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement!

@alexdumea
Dumea Alexandru (alexdumea) merged commit e7350ed into main Oct 27, 2026
45 checks passed
@alexdumea
Dumea Alexandru (alexdumea) deleted the 201/split-ci branch October 27, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run/acceptance-full Runs the 6.5.x and 6.6.x acceptance lanes on this PR, in addition to the default checks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden verify-main-validation and the release gate Split ci.yml into layer-scoped workflows

4 participants