Repository navigation
Conversation
The Warden environment already brings up nginx, Varnish and Traefik, but nothing installed into it is reachable from the runner itself. Integration tests never noticed, because they drive Magento's own bootstrap rather than making HTTP requests. Three things have to be true before a browser can reach the storefront, and each is added here behind a default that preserves current behaviour: - setup:install needs --base-url, or Magento redirects to its default base URL regardless of where the environment is served - the Traefik hostname needs resolving on the runner, which has no dnsmasq - static content needs deploying, or the storefront returns 200 and renders nothing Also exposes the served URL as an output, so a caller does not have to reconstruct it, and makes the admin frontName and the Warden environment naming configurable rather than hardcoded -- a predictable admin URL is a prerequisite for testing the admin at all. Existing callers pass none of the new inputs and are unaffected.
Adds the canary for the browser-serving inputs on warden/setup-environment: it installs Mage-OS, serves it, deploys Luma's static content and drives a browser at the result, following the _internal-* convention the other actions in this repository use. The suite lives with the demo package, as the dummy e2e test already did. It stays deliberately small: its job is to prove a storefront is being served, not to cover a store's behaviour. The assertions are chosen to fail against things that should not pass. A 200 is not evidence of a storefront -- the admin login page, a CMS no-route page and a Magento error page all answer 200 with a header, a footer and a title -- so the suite checks the resolved route in the body class, Luma's minicart block and whether a stylesheet actually parsed into rules. Without the last one it would pass against a store whose pub/static was never deployed. The suite now refuses to start without PLAYWRIGHT_BASE_URL rather than falling back to a public demo store, since a suite that reports green about a store nobody asked about is worse than one that will not run.
The first run failed in the post phase of the Warden step: the local action could no longer be found. warden/setup-environment checks the Warden CLI out into warden/ at the workspace root, which is exactly where this repository keeps that action, so the checkout landed on top of it. Checking the repository out into actions/ leaves the workspace root free for the Warden CLI, which the steps reach as $(dirname $(pwd))/warden/bin/warden.
Contributor
Author
|
Green on run 42044685090 — 4m13s for the whole job:
Cheaper than I expected, which matters for the next step: gating a release on this costs minutes, not a budget conversation. The first run failed, usefully
Verified against a real store, not just in CIThe suite passes against a Mage-OS 4.0.0 Luma store, and the negative control confirms the discriminator bites. Pointed at the admin login page, which answers 200 with stylesheets attached: That is the vacuous green this suite is built to refuse. Staying in draft until #395 merges, since its commits are in this diff until then. |
The README told the reader to point PLAYWRIGHT_BASE_URL at the admin URL
and watch the test go red. It does not go red: page.goto("/") resolves
against the origin and drops the path, so the run hits the homepage again
and passes -- the exact vacuous green this suite exists to refuse.
Replacing it with a throwaway spec that navigates to the admin path
directly, and the output it produces against a real store.
trace: on-first-retry only produces a trace once a test has failed and been retried. A gate that fails is read by someone deciding whether to ship, and "re-run it and watch" is not an answer at that point, so the first failure now carries its own trace. Report artifacts are kept for 30 days rather than 7, long enough to outlive a release weekend.
The homepage check proves a store is served. It does not prove a store can take money. This walks the deepest path a storefront check can: category listing, product page, cart, shipping address, rate selection, payment, order. It touches the price index, the quote, the shipping and payment method pools, order placement, and the private-content sections Varnish has to punch holes for. That path needs a salable product, which a bare setup:install does not have. fixtures/mageos-e2e.xml seeds the least data it takes: 20 simple products with deterministic SKUs, one configurable, two categories in the main navigation. 12s wall clock on Mage-OS 4.0.0 under Warden, including the reindex the command runs itself, and idempotent. Core ships no profile for functional seeding -- the smallest of the five performance profiles generates 800 products, 100 images and 80 orders, because those profiles exist to benchmark a store under catalog weight. Sample data was the other option and is worse for a gate: its content changes between releases, so an assertion on a product name is an assertion about sample data rather than about Mage-OS. Verified both directions against a Warden store: green with three persisted guest orders carrying Flat Rate and a $6.00 grand total, and red at the rate-selection step with carriers/flatrate/active set to 0.
The checkout spec needs something to buy, and the install the workflow stands up has nothing. The profile is copied into the project directory because the command runs inside the container, which sees only that. It reindexes itself, so no indexer:reindex follows it.
The runner cannot write into the project directories the container has written to, because those are owned by the container's user: cp: cannot create regular file 'var/mageos-e2e.xml': Permission denied Piping the profile through exec avoids host permissions entirely and leaves nothing behind in the project. Keeping the profile outside setup/performance-toolkit means its di.xml is not loaded, which only mocks the mail transport -- noted in the profile, and nothing here generates customers or orders.
Which product a category lists first is not ours to decide, and on a clean install it is the configurable. That will not go into the cart until every attribute has a value, so the first CI run failed waiting for a success message that was never coming -- it had a validation error instead. It passed locally only because a leftover simple product happened to sort ahead of it. Selecting the first available value for whatever the page asks for makes the step indifferent to catalog order, and lets the spec run against a simple product, a configurable, or a merchant's own catalog. Verified against a configurable locally: one dropdown, three values, first selection adds to the cart.
6 of 12 tasks
_internal-e2e.yaml proves the actions work, but it cannot be reused: it references them through "./", and a relative path in a reusable workflow resolves against the caller's checkout, which is not this repository when the caller is the release pipeline. So own-repo actions are referenced by full ref here, as integration.yaml and full-integration-tests.yaml already do. test_command is an input rather than a hardcoded "npx playwright test". The workflow's job is to stand a store up and hand over a URL; it should not know what runner points at it, and a caller wanting a grep filter, a shard or a different reporter should not need a new input for each. It arrives through the environment rather than interpolated into the script, so quoting is bash's problem and not the expression engine's. Evidence policy stays in the suite's config, so the default command carries no flags: a CLI flag overrides the config, and --trace on records a trace for every passing test. fixture_profile seeds a catalog before the run, because a bare setup:install has nothing to buy. Empty skips it, which is right for a suite that asserts only on what a bare install provides. Verified green end to end on a throwaway branch, with the Warden action pointed at the stacked serving branch: store served, catalog seeded in 9s, 2 specs passed. At @main it needs the serving inputs, so this cannot go green until mage-os#395 is on main.
marcelmtz
changed the title
An expression interpolated into run: is substituted before bash sees the script, so its content has to be trusted as shell. This one is our own action's output rather than anything a caller supplies, so nothing was exploitable, but it is the pattern issue mage-os#314 asks this repository to stop using, and there is no reason for the gate to be the exception. No expression is interpolated into a run: block in either workflow now.
Probed from the running store rather than echoed back from the inputs. A version input can be a constraint, the repository can resolve it differently than expected, and a service tag does not carry its patch level: the local store asked for opensearch 3.3 and runs 3.3.2. A failing release gate has to be able to say which. MAGEOS_VERSION and MAGENTO_VERSION reach the suite, because a spec may reasonably branch on either -- Luma markup tracks upstream Magento while feature availability tracks Mage-OS. The rest goes to the job summary; no spec should branch on a Redis patch level. Nothing here can fail the job. A probe that comes back empty reports unknown, because a disabled service is not a broken store. Two probes needed care: varnishd -V writes its version to stderr, and OpenSearch reports its own several lines into a JSON body. The READMEs state the preference for test.skip with a reason over a --grep exclusion, so that a version condition which stops applying is visible in the report instead of silently selecting nothing.
The storefront is a thin skin over the catalog; the admin is where the framework lives, and where a release is most likely to break. This adds the enablement the admin needs and one spec that proves it works, so the specs that exercise UI components and the EAV save path have somewhere to start. Nothing in Magento switches two-factor authentication off. There is no enable flag, and leaving twofactorauth/general/force_providers empty does not help: ControllerActionPredispatch still routes a fresh admin user to tfa/tfa/index, which redirects to tfa/tfa/requestconfig. Disabling the module is the only route in, and MageOS_PasskeyAuth requires it and registers an admin 2FA provider, so both go in one command or dependency validation refuses it. The cost is stated in the workflow and the README: this gate does not cover admin 2FA, and doing that properly means driving a TOTP provider. The admin login CAPTCHA is on by default -- admin/captcha/enable 1, after_fail mode, three failed attempts per login name, counted in captcha_log. A suite that asserts a bad credential is refused manufactures exactly those failures, so leaving it on lets the negative control poison the positive one, reported as an "Incorrect CAPTCHA" on a correct password. Disabled, and the specs limit their own exposure: the refusal test uses a name that is not an account, and the three sign-in assertions are separate tests, so each gets its own context. No blanket modal dismissal. Both candidates were checked instead of assumed: Magento_AdminAnalytics' prompt is gated by admin/usage/enabled, which defaults to 0 and is now pinned, and Magento_ReleaseNotification builds its URL from system/release_notification/content_url, which Mage-OS ships empty. Neither renders. A helper that clicked away whatever covered the page would be dead code today and would hide a real blocking dialog later. Assertions are on page identity, not URLs: Magento serves the dashboard at whichever admin path was requested, so a URL assertion passes or fails depending on how the session arrived. #login-form is ambiguous -- the storefront authentication popup uses the same id -- so the body class is asserted first, which a wrong-frontname negative control found. The sign-in is a project dependency rather than a globalSetup hook so it is its own reported result: a broken session fails once and the admin specs report "did not run", instead of a wall of identical reds. Verified against a live store: three sign-in tests and the storefront smoke green in 14s, and both negative controls red -- a wrong password and a wrong frontname each fail the session setup on the assertion that should catch them.
Leaving 2FA in place means the admin specs cannot sign in, so the flag only makes sense together with a test_command that does not select them. Without that, the run fails on the session setup rather than on anything it meant to test.
Two admin specs, in this order on purpose. The grid spec is the whole Magento_Ui stack in one page: requirejs resolves the component, Knockout renders the table, a separate mui/index/render request answers with the data and the collection applies the filter. If that is broken, every admin screen is broken -- and a create-product spec fails too, with no way to tell "the form never rendered" from "the product could not be saved". The create spec then crosses the seams no storefront check can reach: form validation, the EAV write path, attribute set defaults, the stock item, url rewrite generation, the indexers that make a product salable, and a storefront quote. Fixtures are written straight to the database, so a seeded catalog passing says nothing about whether a merchant can add a product. The Luma selector map moves to luma.ts, shared with guest-checkout, so the two specs agree on what a product page looks like. Four traps, each found by a red run rather than by reading markup: - Every grid toolbar and table exists twice, duplicated inside .sticky-header, so both match two elements. The real table carries data-role="grid"; the real toolbar is a direct child of the outer wrap. - The notification bell is a second grid on every admin page, and its loading mask is otherwise indistinguishable from the one under test, so the mask is selected by data-component. - Grid filter state is saved per admin user in ui_bookmark, not per session, so a filter one run leaves applied is still applied for the next. Both specs start unfiltered and the filter is cleared as an asserted step. - The grid caches pages client-side, so clearing a filter can re-render with no request at all. Counts that settle asynchronously are polled. catalog/product/new is a 404 without an attribute set, so the spec clicks Add Product rather than hardcoding the default set id, and the url key is set explicitly so the storefront step is about url rewrites rather than about how a name becomes a url. The spec writes one product per run, with the sku generated inside the test so a retry does not collide with what the failed attempt saved. Both READMEs say so: an admin suite belongs against a throwaway store.
The reachability check reported a 404 and passed. It was curl --silent --show-error --fail --insecure --head "$BASE_URL" | head -1 and a GitHub `run:` without `shell: bash` runs under `bash -e` with no `-o pipefail`, so the pipeline exited with head's status and discarded curl's. The one step whose entire job is to say "nothing was served" said the opposite, and nine specs then failed against a store that was never there -- which reads as a broken suite rather than a broken environment. The status is now captured and compared, so the step does not depend on pipefail being set to mean what it says. A failure diagnostics step comes with it. When this goes wrong the containers leave with the runner, and nothing in the log says whether nginx was up, whether Varnish had a backend, or whether Traefik had a route at all -- a Traefik 404 and a Magento 404 are indistinguishable from outside, and only one of them is about the store.
Every request to the store answered 404, with every container in the
environment reporting healthy and bin/magento working fine inside them.
The diagnostics step added alongside this says why:
traefik | level=error msg="Failed to retrieve information of the
docker client and server host: Error response from daemon: client
version 1.24 is too old. Minimum supported API version is 1.40"
Warden 0.14.2 pins traefik:2.2, whose Docker provider speaks API 1.24,
and the Docker daemon on GitHub's current runner image refuses anything
below 1.40. Traefik therefore reads no container labels, builds no
routers, and returns its own 404 for every host -- which from outside is
indistinguishable from a Magento 404, and from the install's point of
view nothing is wrong at all.
The image tag is a variable in the compose file Warden ships
(traefik:${TRAEFIK_VERSION:-2.2}), so this sets it to 2.11, the tag
Warden itself moved to, rather than bumping the pinned Warden version:
0.16 also changes where the certificates are mounted, which is a larger
change than a broken proxy needs.
The traefik_version input is only legible with the failure beside it: a proxy that cannot reach the Docker daemon reads, from outside, exactly like a Magento that cannot find a route.
Two findings from the first CI run of this spec, both in the save path. The sku was wrong in the store. Magento_Catalog/js/components/import-handler mirrors the name into sku, url_key and the meta fields through the masks in catalog/fields_masks, and stops only once a field has been edited by hand. The name field is configured valueUpdate: 'keyup', which Playwright's fill() does not emit, so the import runs on the name's change event -- which fires as focus leaves it, i.e. while the sku is being filled. The two interleaved and the store ended up with a sku of "<name><sku>": Mage-OS E2E Product 1794623536178mageos-e2e-1794623536178 Every assertion passed anyway, because the grid's sku filter is a LIKE and a contains assertion is satisfied by a superstring. So the spec reported that it had created a product it had not created. The name is now blurred before anything else is typed, the sku is filled last, and three assertions make the trap unable to return quietly: the field holds what was typed, the saved edit form reads it back, and the grid cell equals the sku rather than containing it. Verified against the database -- the row now reads mageos-e2e-<stamp> exactly. The save was also flaky in CI: it timed out at 20s on the first attempt and the retry passed in 10s end to end. The first product save on a freshly installed store in developer mode pays for code generation along the save path, then for the Update on Save indexers -- price, EAV, product categories, catalog search -- which is work this gate wants happening. So that one assertion gets 90s, and the test 180s. It waits for the success *or* the error message, which is what keeps a generous timeout honest: a save rejected by validation now fails in a second showing the message, instead of in ninety with "element(s) not found".
The ceiling of 90s was still not enough: in CI the first attempt ran 1.8 minutes and the retry passed in 14.9s. Three runs now put the first save a store ever performs at 12s, 17s and over 90s, with every retry at about 15s -- so the cost is the first save, not a slow runner. Four minutes, then, with the test at five. High enough that runner variance produces a slow pass rather than a red gate, and retries: 1 covers the rest. A failure on the retry as well would mean the save path itself regressed, which is what the gate is for. Waiting for the error message as well as the success one is what keeps a ceiling this generous honest: a validation failure still reports in about a second.
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.
Revives the stalled browser-based e2e work as something a release can be gated on. Stacked on #395, which adds the serving inputs this needs.
Three pieces:
_internal-e2e.yaml— a canary that stands a store up with this PR'swarden/setup-environmentand runs a browser against it. Uses./actions/...deliberately, so it tests the action changes in the PR rather than whatever is onmain._test/demo-package/Test/e2e/— a homepage smoke spec, a guest-checkout spec, and a fixture profile.e2e.yaml— a reusableworkflow_callworkflow the release pipeline can call, withe2e-README.md. The canary cannot be reused for that: a relative action path in a reusable workflow resolves against the caller's checkout, so own-repo actions are referenced by full ref here, asintegration.yamlandfull-integration-tests.yamlalready do.Verified
Canary run, green in 4m10s total:
The reusable workflow was verified separately on a throwaway branch with the Warden action pointed at #395's branch — store served, catalog seeded, 2 specs passed, report uploaded. At
@mainit needs #395's inputs, soe2e.yamlcannot go green until #395 is on main. The canary is green today regardless.Guest orders land for real:
customer_is_guest=1,Flat Rate - Fixed,$6.00grand total. And the suite bites — withcarriers/flatrate/activeset to0it fails at rate selection withFlat Rate should be offered for the shipping address, not somewhere downstream.What the specs assert, and why
A
200is not evidence of a storefront: the admin login page, a CMS no-route page and a Magento error page all answer200with a header, a footer and a title. So each assertion is one that fails on something a storefront check should not accept.hello-magento.spec.ts200;body.cms-index-indexand an attached minicart (not some other 200);document.styleSheetshas parsed rules (not an emptypub/static— a stylesheet that 404s is still a<link>)guest-checkout.spec.tsCheckout is the deepest path available: it touches the price index, the quote, the shipping and payment method pools, order placement, and the private-content sections Varnish has to punch holes for.
Fixtures
A bare
setup:installhas no salable product, so checkout has nothing to buy.fixtures/mageos-e2e.xmlseeds the least data it takes: 20 simple products with deterministic SKUs, one configurable, two categories in the main navigation. 12s wall clock, including the reindex the command runs itself, and idempotent.Core ships no profile for functional seeding — the smallest of the five performance profiles generates 800 products, 100 images and 80 orders, because those exist to benchmark a store under catalog weight. Sample data was the other option and is worse for a gate: it is mirrored into repo.mage-os.org so it needs no Adobe credentials, but its content changes between releases, so an assertion on a product name becomes an assertion about sample data.
Two things a reviewer should know about the profile:
small.xml'sadmin/security/use_form_key=0andadmin_account_sharing=1. A gate running against a store with admin CSRF protection disabled is not testing the store we ship.<configs>entries because one fatals:ConfigsApplyFixture.php:29iterates$configs['config']as a list, but a single<config>element collapses to a flat map, so it iterates that element's own values and dies withCannot access offset of type string on string.small.xmlhas four entries, so upstream never sees it. Worth a one-line fix inmage-os/mageos-magento2separately.Two runner details that cost a red run each
warden/at the workspace root, which is exactly where this repository keeps that action. Checking out at the root clobbers it, so the canary usespath: actionsande2e.yamlusespath: suite. Documented in feat(warden): serve the environment for browser-based tests #395's README.cp: cannot create regular file 'var/...': Permission denied), so the fixture profile is streamed in throughexec -Trather than copied.On
test_commande2e.yamltakestest_commandas an input rather than hardcodingnpx playwright test. The workflow's job is to stand a store up and hand over a URL; it should not know what runner points at it, and a caller wanting a grep filter, a shard or a different reporter should not need a new input for each. It arrives through the environment rather than interpolated into the script, so its quoting is bash's problem and not the expression engine's.The default carries no flags on purpose. Evidence policy belongs in the suite's config where it is reviewable — a CLI flag overrides the config, and
--trace onrecords a trace for every passing test.The
test_commandinput and the setup-phase idea are both @digitalrisedorset's, from #313. The README documents the setup phase as Playwright projectdependenciesrather than two--grepinvocations, because two invocations share one report directory and the second overwrites the first, so a failing setup phase loses its own evidence. The setup project itself is not here — nothing needs an authenticated session yet, and it should land with the first admin spec that does.Evidence on failure
trace: 'retain-on-failure'rather thanon-first-retry, plus video and screenshot, kept 30 days. A release gate that fails is read by someone deciding whether to ship, and "re-run it and watch" is not an answer at that point. That paid off during this PR: a failing CI run's page snapshot showed the URL wasconfigurable-product-1.html, which is how the catalog-order bug was found in about a minute.Replaces
test_commandinput and@setupmodel are carried forward.localhost:8080. That bypasses Traefik and Varnish and uses aHostheader that does not match the installed base URL, so secure URLs, FPC and private content all behave differently than on a real store. @digitalrisedorset flagged the concern in the PR body himself. feat(warden): serve the environment for browser-based tests #395'sconfigure_host_resolution+base_urlgoes through Traefik instead.Both can be closed once this lands.
Not in scope
supported-versionmatrix. A release gate has to install the version being released, which by definition is not insupported.jsonyet, somagento_versionandmagento_repositoryare passed directly.generate-mirror-repo-jsand follows separately, oncee2e.yamlis onmainto reference.integration.yamland the common parts should be extracted. Still true, still worth doing separately; this does not make it worse.PR Checklist
e2e-README.md, suiteREADME.md)PR Type
Does this PR introduce a breaking change?