test(stripe-connect): add Tier 1 e2e suite for the Stripe Connect revamp - #3387
Open
shohan0120 wants to merge 19 commits into
Open
test(stripe-connect): add Tier 1 e2e suite for the Stripe Connect revamp#3387shohan0120 wants to merge 19 commits into
shohan0120 wants to merge 19 commits into
Conversation
Stripe Connect had zero automated coverage after PR #3272 was closed, while Stripe Express has a live suite. This adds a Tier 1 suite covering checkout, 3D Secure, saved cards, the multi-vendor split, refunds, webhook replay and payment-integrity attacks. 18 Tier 1 cases across 25 tests. 16 pass, each proven able to fail before being accepted. Two are deliberately not passing: - SC-42 is a confirmed defect and carries test.fail() on the classic surface. - XREF-01's PayPal purchase leg is test.fixme: PayPal is not configured on the test site and the shared sandbox buyer is locked out. Stripe Express and Stripe Connect are mutually exclusive, so every spec activates Connect in beforeAll and hands the checkout back to Express in afterAll. That keeps an Express spec sharing a shard working. The suite needs --workers=1 because it mutates the active gateway, the connected-vendor meta and the seller-pays-fee setting. utils/stripeApi.ts becomes a makeStripeApi(secretFn) factory. Connect is a separate Stripe platform account from Express, so it cannot share the key. stripeApi keeps its existing behaviour; stripeConnectApi is new. Four defects reproduced by this suite are filed under getdokan/plugin-internal-tasks#2293, with local notes in tests/pw/tests/e2e/stripe-connect/bugs/. The CI secrets and a STRIPE_CONNECT_REQUIRED gate are wired, but nothing has run on CI yet: the workflow builds dokan-pro from develop, which does not carry the revamp. This is intended to merge alongside dokan-pro PR #5646.
The four markdown files under tests/e2e/stripe-connect/bugs/ duplicated bug reports that already exist as GitHub issues under getdokan/plugin-internal-tasks#2293. Nothing in the specs referenced them by path, so they were a second copy with no reader and every chance of drifting out of step with the issues. The specs already cite the issue numbers in their comments, which keeps one place to read a bug. The existing stripe-express bug files stay as they are; those are referenced from the specs that fail because of them.
… methods dokan-pro #5861 made the Address verification method undeletable: VerificationMethodsApi::delete_item() returns dokan_pro_rest_cannot_delete (403) for kind === 'address', because the seller-address auto-fill flows query by that kind and install seeding never recreates it. deleteAllVerificationMethods still deleted every method, and delete() asserts response.ok(), so the 403 threw inside beforeAll and took the whole vendorVerifications spec with it. On develop the spec aborts after 5 tests; with this change 34 of 36 pass. Filtering on kind here rather than tolerating a 403 inside delete(), so a genuine delete failure on a custom method is still loud. The two tests that now fail, "admin can edit" and "admin can delete", were previously unreachable because the run aborted before them. They are a separate pre-existing problem and are not addressed here.
8 tasks
…I change
dokan-pro #5861 ("make method actions discoverable") rewrote the verification
method row: the edit and delete icons are no longer hover-revealed, they lost the
`rounded-full bg-violet` / `rounded-full bg-red` classes, and they gained an
aria-label.
Both selectors still matched on those classes, so the locator never resolved and
every attempt died on a 30s dispatchEvent timeout:
waiting for locator('//p[...]/../../..//button[contains(@Class, 'rounded-full bg-violet')]')
Target the aria-label instead. It is semantic, it is what the component actually
exposes, and it survives restyling. The hover step and the dispatchEvent
workaround both go with it, since the buttons are permanently visible now.
The same two selectors were duplicated in settings/settingsPage.ts; both copies
are updated so they cannot drift apart.
vendorVerifications.spec.ts now passes 36/36, verified on WordPress 7.1 with
WooCommerce 11.0.1 as well as on the pinned 7.0.3 stack.
removeVendorSubscription() deleted the Stripe Express subscription id but never the Connect one, so `_stripe_subscription_id` survived the reset and leaked from one describe into the next. The lifetime-pack case read that stale value and reported a subscription the non-recurring purchase had not created, which looks like a product defect and is not one.
Vendor subscriptions and coupons had no automated coverage, so regressions in either could only be caught by hand. Both are now release-blocking, which takes Tier 1 from 18 cases to 41 and the suite from 25 tests to 49. Vendor subscription packs (SCSUB-03, SCSUB-07 to SCSUB-12): the Payment Element mounts on a pack checkout, a recurring pack activates the vendor behind a real Stripe subscription, the pack fee stays with the platform, a decline creates no paid order, a lifetime pack mints no subscription, a free trial reaches `trialing`, and a 3D Secure card settles after the challenge. Dashboard and guards (SCSUB-13 to SCSUB-22): the active pack renders, cancel schedules `cancel_at_period_end` in Stripe while the vendor keeps selling, reactivate clears it, re-opening the checkout reuses the pending subscription rather than creating a second, one pack per cart, a subscribed vendor cannot add another, and a guest is sent to log in. Lifecycle webhooks (SCSUB-16 to SCSUB-18): a renewal-cycle invoice records a renewal order, a final payment failure terminates the vendor, and a deleted subscription revokes the pack. These inject into the module's EventFactory, so they do not exercise the live `?wc-api=dokan_stripe` endpoint. WooCommerce Subscriptions (SCSUB-01, SCSUB-02, SCSUB-06): a customer buys a vendor's recurring product, a renewal charges once and pays the vendor once, and the renewal payout is booked as exactly one approved auto-withdraw so the vendor cannot withdraw it a second time. SCSUB-06 guards the RenewalProcessor ledger fix. Renewals are driven through the gateway's own `woocommerce_scheduled_subscription_payment` hook rather than a synthetic webhook, so the token is really charged and the transfers really made. Coupons (CPN-01, CPN-02): a coupon reduces the Stripe capture and the order together to the cent, and a marketplace coupon on a two-vendor cart still pays both vendors without the transfers exceeding the capture. SCTOK-02 is marked `test.fail()` against getdokan/plugin-internal-tasks#2321: a free-trial checkout draws no card field. It ships with a paid-subscription control so a broken fixture cannot be mistaken for the defect. Every case was red-checked: one load-bearing assertion inverted, the failure confirmed to print the real observed value, then reverted. Notes on state, both learned the hard way: - `dokan_product_subscription[enable_pricing]` is set once per spec file and cleared in a file-level teardown. Flipping it per describe leaves windows where the module empties the pack back out of the cart; leaving it on leaks site-wide and empties every other spec's cart. - Saved cards are cleared before each pack purchase. A stale token is pre-selected at checkout, and that path confirmed at Stripe without settling the order locally, since it returns no redirect and localhost receives no webhook. The folder teardown leaves the `product_subscription` module deactivated, which only holds when the whole folder runs. Requires `--workers=1`, as before.
SC-48 (getdokan/plugin-internal-tasks#2299, an unchallenged payment recorded as "3d secure") and the #2295 validation (block checkout replacing Stripe's decline reason) both reported "expected to fail, but passed" on dokan-pro 6b3f2999a. Each was then re-run on its own and passed 4 attempts out of 4, so the markers are removed and the assertions now guard the fixed behaviour. The repeat runs were not a formality. SC-42, marked for the same reason, reported "expected to fail, but passed" in the same session and then reproduced again on the next attempt, so a single passing observation settles nothing. SC-42 keeps its marker and #2294 stays open. The XREF-01 PayPal purchase leg is deleted rather than left as a `test.fixme`: PayPal is out of scope for this work. The Connect-side half, that other gateways still register while Stripe Connect is active, is kept.
…6 is open The e2e job built dokan-pro from develop, which does not carry the Stripe Connect gateway revamp. On that build modules/stripe has no src/ and no Payment Element markup, so the gateway configures and renders normally and then every Connect checkout case times out waiting for a mount the build cannot produce. Run 33358188560 was cancelled for this reason. Pinning build_pro at feat/stripe-connect-revemp lets the suite meet the module it was written against. The branch is 0 commits behind develop and changes 5 files outside modules/stripe, so the other suites are unaffected. merge-reports still checks out develop, since feature-map.yml is identical on both branches. Restore the ref to develop once #5646 merges.
SCTOK-01, save-card checkbox. Stripe keeps adding accordion rows to the Payment Element after the card fields are filled — the CI trace shows ACH bank search and Cash App arriving, which a US runner IP is offered and this location never sees. Each row pushes the checkbox down, so a click aimed at where it just was is delivered without changing anything, and check() reports "Clicking the checkbox did not change its state" and gives up without retrying. The tick is now retried against a re-measured position, and says so when it needed more than one attempt: a miss now and then is the element settling, a miss every run would be a late-bound handler and a product question. The closing assertion is unchanged, so a checkbox that never ticks still fails. payments.spec.ts, admin can add stripe payment method. The revamped gateway ships enable_3d_secure disabled, since Payment Elements keep SCA on permanently and the toggle survives only for legacy orders. Ticking it can never succeed. checkIfOperable skips a control the gateway itself renders read-only and logs what it skipped; every other field keeps using check(), so one going read-only unexpectedly still fails the run. Verified locally: payments failed with the identical error before the fix and passes after. SCTOK-01 was already green locally, so it was verified against the failure mode instead — a swallowed first click reproduces the CI error and the fix clears it, while swallowing every click still fails.
…ct-tier1-e2e # Conflicts: # tests/pw/tests/e2e/vendor-verifications/vendorVerificationsPage.ts # tests/pw/utils/apiUtils.ts
…ng reasons SCPE-05 was a shard-state leak, not a defect. It pinned every existing transfer to vendor1 and treated anything new as belonging to this attempt, but all twelve shards share one Stripe platform and the same STRIPE_VENDOR1_ACCT, so a sibling shard paying that vendor mid-test read as "the abandoned challenge paid the vendor". It failed in-shard on runs 33367338869 and 33482179826 and passed alone both times. The assertion now keys off this attempt's own charge, which no other shard can produce, and says which proof it used when there is no charge to check. SC-42 (classic) is skipped rather than marked. #2294 is real and still reproduces on dokan-pro 61869ae8f: measured today the vendor was short by exactly the gateway fee on 7 of 27 orders. But detection is irreducibly statistical, because on a single order a correct implementation and a bug-present-but-race-won order are identical. One order was wrong ~40% of the time, three ~1 run in 8, and six still missed the defect entirely in one of three local runs; a 2% miss rate would need ~13 live orders per attempt. There is no deterministic signal to assert instead. The body still asserts the CORRECT behaviour, so deleting the test.fixme line once #2294 is fixed turns it into the regression lock. Verified locally against dokan-pro 61869ae8f, rebuilt: SCPE-04, SCPE-05, SC-41 and SC-42 (block) pass, SC-42 (classic) skips.
…gate (SC-45) The suite had nothing here at all. Whether a customer may buy from a vendor who has never connected Stripe is a gating decision about money, and neither answer was tested: not that checkout is refused when the admin disallows it, nor that the sale completes when they allow it. SC-45a asserts the refusal and that it names the reason, and that no order was paid. The gate is Validation::check_vendor_configure_stripe on woocommerce_after_checkout_validation, so the notice is a WooCommerce validation error rather than the Stripe JS error container the declined-card helper watches, which is why this does not reuse placeClassicOrderExpectError. SC-45b asserts the sale settles and that no transfer was made against the charge. The setting's own description is the specification here: "The payment will send to admin Stripe account." The check is scoped to this order's charge rather than an account listing, because all twelve CI shards share one Stripe platform. The gate keys on an empty _stripe_connect_access_key, not on dokan_connected_vendor_id, so a vendor seeded with only the account id would still be refused. Classic checkout only; whether the block surface enforces the same rule is declared uncovered rather than assumed. Both cases red-checked. Flipping the setting in SC-45a removes the notice entirely and the case fails; inverting SC-45b's transfer count fails reporting the real observed 0. Teardown restores the setting to its default, verified in the database, since leaving it on would disable the gate suite-wide.
SC-45a failed on CI run 33590556960 and passed locally, which was the tell. Both cases drive the classic checkout, but the [woocommerce_checkout] shortcode page does not exist in a fresh environment and every CI shard provisions its own. The run died on "Oops! That page can't be found" waiting for #place_order, a 404 with nothing to do with connected sellers. Locally it passed only because an earlier spec had already created the page on a long-lived site. Every other classic-checkout spec here already calls ensureClassicCheckoutPage in beforeAll. This one did not. Reproduced locally before fixing rather than inferred: with the page deleted the case failed with the identical error, and passes with the page absent once the beforeAll recreates it. Confirmed in a real browser that /classic-checkout/ renders with no console errors, and that teardown still leaves allow_non_connected_sellers at its default.
The RMA suite failed on CI run 33590556960 with a 403 on its seeding call, in the same shard as this spec, and its isolation re-run failed too. That was our doing, not theirs. WooCommerce Subscriptions promotes a buyer to the `subscriber` role when a subscription is created, and that REPLACES `customer` rather than adding to it. Both the role and the actor are global, so this spec left customer1 as a subscriber for everything that ran after it. Dokan's RMA REST create checks dokan_is_user_customer and refuses with 403 "Only customers can create warranty requests". The damage is persistent rather than transient, which is why running the victim alone did not clear it — worth remembering, since a failing isolation re-run normally argues against pollution. Found by bisecting the shard locally, running each spec in CI order and reading customer1's role back after each: sixteen specs clean, then this one flipped it to subscriber. Restored at file scope so it runs whichever describe executed. Verified as a chain rather than asserted: with the role forced to subscriber the RMA spec fails with the identical CI error (Expected 201, Received 403 at line 41); this spec then buys a subscription and still hands back `customer`; the RMA spec then passes 12/12.
SCSUB-06 failed once in a full-folder run and then passed on three consecutive attempts, so the fault is real but was not reproducible on demand. Rather than guess at it, the one un-polled read in the case is hardened. The auto-withdraw is written as a side effect of the renewal settling. The case polls for the order to reach a paid status, which is not the same instant the ledger row lands, and then took a single reading of the withdraw list. A read that beats the insert reports zero and fails on correct behaviour. Every other asynchronous check in this case already polls; this one did not. The assertion is unchanged: still exactly one new approved withdraw, still matched on amount and on a note carrying the renewal sub-order id. Only the reading waits. Red-checked by rigging the poll to observe no booking: it still fails after the full timeout with the intended message, so the poll cannot pass an unbooked payout. Rig removed, file green 5/5.
… SC-45a SC-45a failed on CI run 33604421784, three attempts out of three, and passed locally every time. The page snapshot at failure showed the checkout still on screen with the gateway selected, the product in the cart, no order placed and no error anywhere: the form had never been submitted at all. The case pressed #place_order once. The first press on a cold runner routinely does not register, which is why every other classic helper here re-presses up to three times guarded on whether the submit actually fired. Without that the case reports a missing error message, which reads as "the gate did not fire" and sends you looking at the gateway settings instead of at the click. placeClassicOrderExpectError now takes the error selector and message, so this case can reuse it: the refusal here comes from woocommerce_after_checkout_validation and renders as a WooCommerce notice, not in the gateway's inline error container that a declined card uses. Default behaviour is unchanged. Verified: SC-45a and SC-45b pass, and the default callers SCPE-06 and SCPE-07 still pass on the refactored helper.
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
1 participant
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.
All Submissions:
Changes proposed in this Pull Request:
Stripe Connect has had zero automated coverage since
getdokan/dokan#3272was closed, while Stripe Express has a live suite. This adds a Tier 1 suite for the Stripe Connect revamp.18 Tier 1 cases across 25 tests. Checkout on both surfaces, 3D Secure, saved cards, the multi-vendor split, refunds and reversal idempotency, webhook replay, and payment-integrity attacks.
New files:
tests/pw/tests/e2e/stripe-connect/helpers.ts,stripeConnectPage.tstests/pw/mu-plugins/dokan-stripe-connect-test-helpers.phpdokan-test-connect/v1Changed files:
tests/pw/utils/stripeApi.tsbecomes amakeStripeApi(secretFn)factory. Stripe Connect is a different Stripe platform account from Express, so it cannot share the key.stripeApikeeps its existing behaviour and env var;stripeConnectApiis new..github/workflows/e2e_api_tests.ymlgains the five Connect secrets and aSTRIPE_CONNECT_REQUIREDgate, matching the existing Express gate so fork PRs stay soft and internal runs fail loud.Two design points worth knowing before review:
beforeAlland hands the checkout back to Express inafterAll, reusing the Express helper's own configure route, so an Express spec sharing a shard still finds its gateway configured. Both directions of that flip are verified.--workers=1. It mutates the active gateway, the connected-vendor meta and the seller-pays-fee setting. Two workers would fight over all three.Related Pull Request(s)
feat/stripe-connect-revemp). These specs exercise the revamp and are intended to merge alongside it.getdokan/dokan#3272.Closes
How to test the changes in this Pull Request:
Expect 24 passed, 1 skipped. Four of the passes are
test.fail()cases that pass by failing.The client id is not optional:
Helper::is_ready()returns false without one and the gateway never reaches the checkout, so a key pair alone would leave every spec skipping to green.Without the keys the suite self-skips, and
stripeConnectPreflight.spec.tsturns that silent skip into a loud failure wheneverSTRIPE_CONNECT_REQUIREDis set.Changelog entry
Add Tier 1 end-to-end test coverage for the Stripe Connect revamp
Stripe Connect previously had no automated end-to-end coverage, so regressions in vendor payouts, refunds and payment integrity could only be caught by hand. This adds a Playwright suite covering card checkout on both the block and classic surfaces, 3D Secure completion and abandonment, saved cards, the multi-vendor split, refund reversal and its idempotency, webhook replay, and two payment-integrity attacks. Money assertions are made against the Stripe API and the Dokan REST API rather than the database, so a test can only pass when the money actually moved.
Verification
Three consecutive full-folder runs are green, two headless and one headed, 24 passed and 1 skipped each, roughly 14 minutes per run at
--workers=1.Every passing case was red-checked: one load-bearing assertion inverted, the run confirmed to fail with the real values in the message, the inversion reverted, and the case re-run green. Nothing is accepted from observation alone.
Stripe Express was re-run to prove it still works: 46 passed, 3 failed, 10 skipped. All three failures were browser and timeout shaped, all three pass when re-run alone, and every money test that exercises the refactored
stripeApi.tspassed in the full run.Not passing, deliberately
test.fail()getdokan/plugin-internal-tasks#2294test.fixmeEvery marker is called imperatively, one line before the failing assertion, never as a declaration modifier. A declaration-level
test.failmarks the whole body as expected-to-fail, so the positive controls above it would report green on the day they silently broke.Defects this suite reproduces
Filed under
getdokan/plugin-internal-tasks#2293. The issues are the single place each bug is described; the specs cite the issue numbers in their comments rather than carrying a second copy in the repo:Three further reported issues (#2296, #2298, and the transaction-id half of #2299) did not reproduce here. That is recorded rather than treated as closed, since two of them are reported as intermittent.
#2294 is worth a closer look. It is a race, not a constant: over five classic orders the vendor was over-transferred by exactly the gateway fee three times and matched twice. The root cause is deterministic —
_dokan_stripe_awaiting_disbursementis set on the block checkout and never on the classic one, so the classic transfer is made before the fee is known. Because the symptom is only about 60% reproducible, SC-42 (classic) places three orders and requires every one to match, which lifts detection to roughly 94%. A marker over a single order was wrong 40% of the time, and two runs proved it.CI
Wired but not yet exercised. The workflow builds dokan-pro from
develop, which does not carry the revamp, so until #5646 lands these specs would meet the legacy Connect module and fail for a reason unrelated to them. The dokan-pro checkout is deliberately not pinned at the revamp branch, so there is no temporary hack to unpick later.PR Self Review Checklist:
dokan_gateway_feeand similar order meta through the WC REST API, not the database, per the project's oracle rule.tests/pw/utils/stripeApi.tsshows a large diff because the body was indented one level into the factory.stripeApi's behaviour is unchanged.