TEST fix(tests): verify e2e tests - #5725
Closed
santgr11 wants to merge 12 commits into
Closed
Conversation
HappyPath.verify and UnhappyPath.verify-scam were tagged with getCanaryTagAndAnnotation() but never called afterEachCanary(), so the 'AppKit Verify Happy success < 1' Grafana alert has been evaluating a metric that was never emitted.
Confirmed in CI logs (runs 81070543249 shard 3, 81083348538 shard 7): the runner intermittently can't reach security.ubuntu.com to fetch gstreamer packages, failing the whole shard with 0 tests run. Retrying up to 3 times absorbs the blip without masking a real, persistent install failure (still fails after 3 tries).
…ob summaries Error: locator.click: Target page, context or browser has been closed recurred across every analyzed CI run, cross-browser, in config.spec.ts, multi-wallet-multichain.spec.ts, and wallet.spec.ts. Root cause is still under investigation (worker-count/resource-contention already ruled out), but blocking every PR on it trains people to ignore CI. These tests are tagged @quarantine, excluded from the blocking gate via --grep-invert, and run separately with retries in a non-blocking CI step so they stay visible instead of silently failing for days. Also wires the report summarizer into both the quarantine lane and the main gating run's job summary, so pass/fail is visible in the GitHub Actions UI instead of requiring a scroll through raw logs.
Strips ANSI escape codes and escapes literal pipe characters before embedding test titles/errors in markdown table cells (raw ANSI codes would otherwise render as garbled control characters in a GitHub job summary), and falls back to a clear message instead of throwing when the report file is missing or unparseable.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
9 Skipped Deployments
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const ANSI_PATTERN = /\u001b\[[0-9;]*m/g | ||
|
|
||
| function sanitizeForTable(value) { | ||
| return String(value).replace(ANSI_PATTERN, '').replace(/\|/g, '\\|') |
Contributor
Visual Regression Test Results ✅ PassedChromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=988 👉 Please review the visual changes in Chromatic and accept or reject them. |
Contributor
Contributor
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
Contributor
Coverage Report
File CoverageNo changed files found. |
Match canary.spec.ts's existing convention (and the equivalent, already-open fix in PR #5709 / svenvoskamp/verify-alerts-firing) - the canary container only ever runs Desktop Chrome/wagmi, so Firefox never reports these metrics anyway; skip explicitly rather than relying on that never being exercised.
…iably emit The "AppKit Receive Session Proposal Timing > 7000ms" Grafana alerts (all 4 regions) were firing on No-Data, not real latency. Their query targets HappyPath.sign.timing.pairingReceiveSessionProposal, which the canary only recorded when pairingCreatedTime was set - and that was captured by scraping a WalletConnect "set core/pairing/pairing" console log that no longer fires. Capture pairingCreatedTime deterministically when the wallet begins processing the connection URI instead of depending on SDK console output, and push the metric unconditionally. (Brought in from the already-open fix on svenvoskamp/verify-alerts-firing.)
…t only
Trace-level analysis already on record showed config.spec.ts's first two
tests reliably pass in under 20s - only the third ("all features disabled")
is a confirmed freeze. Quarantining all three was over-broad and would hide
a real regression in tests 1/2 if one ever showed up. Also renames the
cluster id from "cluster-4-context-closed" to "unresolved-playwright-freeze",
matching the actual conclusion on record: four independent rule-outs (worker
count, browser engine, Playwright version, actionability-check bypass) with
no reproduction outside Playwright automation, not a generic "browser
crashed" label.
Already prototyped and explicitly declined in an earlier pass at this same problem - not necessary for this PR.
Un-tags all @quarantine tests and removes the non-blocking CI lane and --grep-invert exclusion - decided against separating tests into blocking and non-blocking lanes for now. The canary metric fixes, install-deps retry hardening, and job-summary reporting for the main gating run are unaffected.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Description
Please include a brief summary of the change.
Type of change
Associated Issues
For Linear issues: Closes APKT-xxx
For GH issues: closes #...
Showcase (Optional)
If there is a UI change include the screenshots with before and after state.
If new feature is being introduced, include the link to demo recording.
Checklist