test(e2e): revive the order lifecycle tests on the beta pipeline gate - #699
Open
codyborn wants to merge 2 commits into
Open
test(e2e): revive the order lifecycle tests on the beta pipeline gate#699codyborn wants to merge 2 commits into
codyborn wants to merge 2 commits into
Conversation
The post-deploy e2e gate has been running 4 of 28 tests; order.test.ts contributed zero. The skips happened in two generations: - ff32a8f (Nov 2025): Goerli was deprecated, so every on-chain block (expiry, fill, orders sanity) was skipped during the mainnet migration. - 7be9bb9 (Apr 2026): the one remaining live test (quote-API post) was skipped inside a VERSION-bump chore commit with no recorded reason. Since then the gate has passed green while covering nothing about order tracking — which is how a deploy that silently stopped every status-tracking step function sailed through both beta and prod gates. Revived (beta gate only, via RUN_LIFECYCLE_TESTS set per stage in addIntegTests): - 'checking expiry' — posts real orders and tracks them to 'expired'. This is the e2e coverage of the SFN lifecycle. - direct-post '2xx' / '4xx' - 'orders endpoint sanity checks' (pure HTTP, runs on prod gate too) Reliability fixes that make revival viable: - Funded-but-unfillable orders. The old tests posted 5000-UNI-input orders; the test wallet holds ~1 UNI, and an underfunded order resolves to 'insufficient-funds' when the tracker quotes it on-chain — never 'expired'. Orders now use a dust UNI input the wallet actually covers (verified on-chain: 1.0001 UNI balance, standing max Permit2 allowance) and a 100k-ETH-scale output nobody will ever fill. CI posts orders but never sends a transaction. - Poll for the outcome instead of sleep-once-and-check. The old waitAndGetOrderStatus slept a fixed period and read the status exactly once, racing the SFN's ~12s jittered cadence. pollOrderStatusUntil polls to a budget and returns early on any terminal status, so failures show the status actually reached. - beforeAll approval check is now awaited (it was a floating promise) and read-only: it throws with instructions if the standing allowance is missing rather than sending a mainnet approval tx from CI — the old auto-approve is what caused the "timeouts in beforeAll on gas spikes" note. Lifecycle tests run only against the beta gate because a beta e2e failure already blocks prod promotion, and prod's OrderExpiryRate alarm is a ratio: deploy-time dust expiries could be a large share of a quiet 15-minute window on chain 1 and page for nothing. Still skipped, now with the reason documented in place: the fill block (filler wallet has 0 ETH / 0 UNI on mainnet — funding it is an ops decision; until then fill coverage lives in the Anvil-based suites) and the quote-API test (three services plus a cosigner-address agreement; revive deliberately, not as a side effect). Verified: tsc clean, eslint no new errors, file imports and collects under both RUN_LIFECYCLE_TESTS modes, unit suite untouched at its baseline. The revived tests themselves need the pipeline's secrets and can only be exercised by the next beta gate run.
The integ-test RPC secret (all/gouda-service/integ-test/rpc) pointed at an API Gateway whose DNS no longer resolves — dead since roughly 2026-04-25 (the secret's last-changed date, which is also the day the final live e2e test was skipped in 7be9bb9). It went unnoticed because the only tests that read it were already skipped, so the gate stayed green on 4 HTTP-only tests. The secret now holds the same entry gateway the service itself uses, which authenticates via the x-internal-service-secret header. This passes that header into the build (from gouda-service-rpc-urls-2, same account as the CodeBuild project) and writes it to .env; RPC_HEADERS in lib/util/constants.ts already attaches it when the env var is set, so the test code needs no change. Verified end to end against beta with the exact URL+header the pipeline will use: 22 passed / 6 skipped (documented: fill x4, quote-api, nonce), 0 failures, 177s — including all three SFN-tracked order expiries.
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.
The post-deploy e2e gate runs 4 of 28 tests;
order.test.tscontributes zero. This PR revives the order-lifecycle coverage — the tests that would have caught order tracking silently stopping — and fixes the reliability problems that got them skipped in the first place.How they died
Two generations, both under misleading titles:
checking expiry,+ attempt to fill,orders endpoint sanity checks) was skipped during the mainnet migration. The in-code comment says it plainly: "GOERLI chain is deprecated. 1. change RPC_1 2. Deploy contracts 3. fund wallets."chore: bump lambda VERSION env vars, a one-character diff with no explanation in the PR body.Why simply un-skipping would have failed
The old expiry tests posted 5000-UNI-input orders. The test wallet holds 1.0001 UNI (verified on-chain). The status tracker quotes orders on-chain, so an underfunded order resolves to
insufficient-funds— neveropen→expired. Any naive revival would have gone red for a reason unrelated to expiry, which is likely why nobody did it.Reliability fixes
waitAndGetOrderStatusslept a fixed period and checked exactly once, racing the SFN's ~12s jittered cadence — the classic flake.pollOrderStatusUntilpolls to a 90s budget past the deadline and returns early on any terminal status, so a failure's diff shows what the order actually became.beforeAllno longer transacts, and is actually awaited.checkApprovals(uni, alice)was a floating promise that auto-sent mainnet approval txs — the source of the old "timeouts in beforeAll when priority fee moves" note. It's now awaited and read-only: it throws with instructions if the standing allowance regresses.Where they run
Lifecycle tests gate beta only (
RUN_LIFECYCLE_TESTS, set per stage inaddIntegTests). Two reasons:OrderExpiryRatealarm is a ratio (SEV2 ≥ 20%). Deploy-time dust expiries could dominate a quiet 15-minute window on chain 1 and page for nothing.Pure-HTTP blocks (
orders endpoint sanity checks,4xx) run on both gates.Note: the beta gate's own expiry-rate alarms will see these dust expiries — same as the pre-Nov-2025 steady state, and beta alarms are visibility, not paging. Flagging rather than hiding it.
Still skipped, with reasons now written down
+ attempt to fill— the filler wallet has 0 ETH / 0 UNI on mainnet. Reviving it means spending real gas every pipeline run or repointing at a cheap chain with a reactor and funded wallets; that's an ops decision, not a code fix. Fill coverage lives in the Anvil-based suites meanwhile./hard-quoteagreeing, plusCOSIGNER_ADDRESSmatching GPA's signer. Skipped in chore: bump lambda VERSION env vars #649 without a recorded reason; deserves its own deliberate revival.Verification — run for real against beta
Executed locally with the pipeline's exact secrets and env (
RUN_LIFECYCLE_TESTS=true):22 passing vs the gate's previous 4, including all three SFN-tracked expiries against real mainnet orders, with zero retries burned. The 6 skips are exactly the documented ones (fill ×4, quote-API, nonce).
Found in the process: the integ-test RPC secret was dead
all/gouda-service/integ-test/rpcpointed atssv6cpgn9i.execute-api…, whose DNS no longer resolves — the gateway was deleted. Nobody noticed because the only tests reading it were skipped. Its last-changed date is 2026-04-25 — the same day the final live e2e test was skipped in 7be9bb9, which is almost certainly the causal story: RPC infra changed, the test broke the pipeline, and it was skipped rather than fixed.Remediation, included here / done alongside:
ssv6cpgn9i…, new version idd1b04cf9…).x-internal-service-secret, so the buildspec now passesRPC_HEADER_SECRET(fromgouda-service-rpc-urls-2, same account) into.env.RPC_HEADERSalready attaches it when set — no test-code change.Also verified:
tscclean, eslint no new errors, bothRUN_LIFECYCLE_TESTSmodes collect, unit suite at baseline.Related: #698 (alarms + synth tests for the same incident), #697 (the fix itself).
🤖 Generated with Claude Code