feat: add Playwright e2e suite + on-demand CI for docs screenshots#235
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
brtkwr
force-pushed
the
two-e2e-screenshots
branch
from
ad6faeb to
6789d31
Compare
There was a problem hiding this comment.
Re-reviewed the full diff. The previously flagged script-injection risk in .github/workflows/e2e-screenshots.yml (unsanitized grep input interpolated into the shell) is now fixed — inputs are passed via env: and referenced as shell variables. No other critical issues (security, data loss, breaking changes, unsafe migrations) found in the e2e suite or workflow.
Ready for human approval ✅
brtkwr
force-pushed
the
two-e2e-screenshots
branch
from
6789d31 to
788a623
Compare
Two admin-config + storefront checkout journey specs with a skip-verification test buyer; e2e-screenshots.yml runs on demand and uploads screenshots as an artifact.
brtkwr
force-pushed
the
two-e2e-screenshots
branch
from
4f1c25c to
cd55282
Compare
Ports the minimum-order behavioural test in from the ABN overlay suite, where it was testing vanilla Two_Gateway logic from the wrong repo, and makes it deterministic: instead of relying on however the shared test merchant happens to be configured, the test pins the merchant minimum-order store config to a value strictly between the free-shipping and flat-rate totals (measured at runtime), then asserts the method appears and disappears live as the buyer's shipping choice moves the total across it — no page reload. The original config is restored in a finally block. The test needs an admin login, so the workflow gains the same WIF + Secret Manager password resolution as the ABN suite (all identifiers via repo vars; every credentialed step no-ops and the admin-gated specs skip until the infra lands). Concurrency collapses to a single non-cancelling global group so two runs can't interleave their config writes and a cancel can't skip the restore. Shared buyer helpers move from checkout-luma.spec.ts into _helpers.ts so both specs drive the checkout the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewed the full diff, including the minimum-order-gate test and the CI Secret Manager/WIF plumbing added since the last review. The previously flagged script-injection issue remains fixed (grep/store_url passed via env:, not interpolated into run:).
One thing I checked closely: playwright.yml now triggers on pull_request (paths: e2e/**) with id-token: write and a WIF auth step. In a public repo this could look like fork PRs obtaining cloud credentials by editing e2e/**, but the workflow's own comment correctly notes GitHub doesn't issue OIDC tokens to pull_request-triggered runs from forked repositories, and the auth/gcloud steps are continue-on-error, so this degrades safely today (no-op until the WIF infra lands) and isn't exploitable via a fork PR.
No other critical issues (security, data loss, breaking changes, unsafe migrations) found in the new commit.
Ready for human approval ✅
2 participants
Summary
Adds a self-contained Playwright e2e suite in
e2e/that drives the plugin on a staging store, captures the screenshots used in the docs, and carries the plugin's behavioural regression tests. Per the overlay architecture, ALL logic lives in this plugin — so behavioural tests live here; brand-overlay repos (e.g. magento-abn-plugin) only verify that their overlay assembles and renders.admin-config.spec.ts— logs into Magento admin and captures the Two config tabs (General / Payment / Search) plus the section overview. NeedsADMIN_PASS; skips cleanly without it.checkout-luma.spec.ts— drives the full storefront checkout: product → cart → company search → the "Buy Now Pay Later on Invoice Terms" method → payment term → places an order, using a skip-verification test buyer so the order-intent auto-approves. Order placement depends on the sandbox risk decision, so a persistent decline is logged rather than failed; the deterministic assertion is that the method is offered and its form renders.min-order.spec.ts— regression test for the minimum-order-value gate, ported in from the ABN suite and made deterministic. It pins the merchant minimum-order store config to a value strictly between the free-shipping and flat-rate totals (measured at runtime, so it never depends on how the shared test merchant happens to be configured), then asserts the method appears/disappears live as the buyer's shipping choice moves the total across the threshold — no page reload, which is the real regression risk in the availability recalc. Original config restored infinally. Admin-gated like admin-config: skips withoutADMIN_PASS._helpers.ts— shared buyer/checkout + admin helpers used by all specs..github/workflows/e2e-screenshots.yml— runs onworkflow_dispatchand on PRs touchinge2e/**, uploads screenshots as an artifact. ResolvesADMIN_PASSfrom Secret Manager via a dedicated WIF SA; all identifiers come from repo vars and every credentialed step no-ops (admin-gated specs skip) until the infra lands (two-inc/infra#3070). Single non-cancelling concurrency group so runs can't interleave their staging config writes and a cancel can't skip the config restore.Kept as an isolated Node package under
e2e/so it doesn't touch the PHP module.Test plan
cd e2e && npm ci && npx playwright install chromiumnpx playwright test→ checkout journey green (product page, filled checkout, order confirmation captured).ADMIN_PASS=… npx playwright test admin-config→ all config tab screenshots captured.ADMIN_PASS=… npx playwright test min-order→ gate test (pending thee2e_ciadmin user / secret value from two-inc/infra#3070; runnable today with any admin credentials viaADMIN_USER/ADMIN_PASS).