Skip to content

fix(ci): parallelize nightly E2E across browsers to prevent timeout#1870

Merged
luans-qa merged 4 commits into
mainfrom
fix/nightly-e2e-browser-parallelism
May 29, 2026
Merged

fix(ci): parallelize nightly E2E across browsers to prevent timeout#1870
luans-qa merged 4 commits into
mainfrom
fix/nightly-e2e-browser-parallelism

Conversation

@luans-qa
Copy link
Copy Markdown
Contributor

Problem

The nightly E2E job has been hitting the 60-minute job limit every day and cancelling before any tests complete.

Root cause: 60 tests (20 per browser × 3 browsers) run serially on workers: 1. With retries: 2 and transfer tests that individually time out at 2 minutes, the full suite needs ~3 hours — 3× the job limit.

Timeline from today's cancelled run (26626516698):

  • 08:26:19 — "Running 60 tests using 1 worker"
  • 08:26:55 — first test fails, triggers retry cycle
  • 08:33:51 — solana transfer timeout (2 min) consumes retry budget
  • 09:21:32 — job cancelled after exactly 60 min with ~17/60 tests completed

Fix

Split the single job into a matrix of 3 parallel browser jobs (chromium, firefox, webkit). Each job runs only its 20 tests and has a 90-minute limit, which it comfortably fits within.

Before After
1 job, 60 serial tests, 60 min limit 3 parallel jobs, 20 tests each, 90 min limit
Cancelled every day before finishing Each browser completes independently
One timed-out Slack notification One combined Slack message with per-browser breakdown

Other improvements

  • Install only the needed browser per job (playwright install --with-deps ${{ matrix.browser }}) instead of all three — saves ~2 min of setup per job
  • fail-fast: false so a webkit failure doesn't cancel the chromium and firefox jobs
  • Separate notify job aggregates results from all 3 artifacts into a single Slack message

Test plan

  • Trigger via workflow_dispatch and confirm 3 parallel jobs appear
  • Confirm all 3 jobs complete within their 90-minute limit
  • Confirm a single combined Slack notification fires after all 3 finish

The regression run was consistently hitting the 60-minute job limit
because 60 tests (20 per browser × 3 browsers) were executing serially
on a single worker. With retries=2 and transfer tests timing out at
2 minutes each, the full suite needs ~3 hours to complete.

Fix: split into a matrix of 3 parallel jobs (chromium, firefox, webkit).
Each job runs only its 20 tests and completes well within the 90-minute
limit. A separate notify job downloads all three artifacts and sends a
single combined Slack message with per-browser results.

Also installs only the required browser per job
(playwright install --with-deps ${{ matrix.browser }}) instead of all
browsers, saving ~2 minutes of setup time per job.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 29, 2026

⚠️ No Changeset found

Latest commit: 93d4139

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@luans-qa luans-qa self-assigned this May 29, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Reviews (1): Last reviewed commit: "fix(ci): parallelize nightly E2E across ..." | Re-trigger Greptile

…ults

- Replace cramped fields 2-column layout with mrkdwn text block so all
  three browser summaries render with clear spacing
- Fix test name resolution: use spec.title (the actual test name) instead
  of test.title which is empty in Playwright's JSON output
- Build suite path from ancestor describe blocks using processSuite recursion
  so failures show full context (e.g. "auth › login › when email is valid")
- Strip ANSI escape codes from error messages before posting to Slack
- Show first non-empty error line per failure, truncated at 250 chars
- Add divider between summary and run/commit metadata blocks

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/e2e-regression-tests.yml:212-217
When a browser artifact download fails (silently, due to `continue-on-error: true`) but the matrix job itself succeeded, `result` will be `null` for that browser. The loop only sets `overallFailed = true` when `result && result.failedTests > 0`, so a missing artifact from an otherwise-healthy matrix run would leave `overallFailed` as `false`. The Slack message would then show "✅ Passed" alongside a "No results" line — a misleading green notification.

```suggestion
                  for (const browser of browsers) {
                      const filePath = path.join('test-results', browser, 'playwright-results.json');
                      const result = parseResults(filePath);
                      browserResults[browser] = result;
                      if (!result) overallFailed = true;
                      if (result && result.failedTests > 0) overallFailed = true;
                  }
```

Reviews (2): Last reviewed commit: "fix(ci): improve Slack notification form..." | Re-trigger Greptile

Comment thread .github/workflows/e2e-regression-tests.yml Outdated
Move the inline Node.js script from e2e-regression-tests.yml into a
standalone file at apps/wallets/quickstart-devkit/scripts/notify-slack.js,
alongside the existing generate-test-report.js script. The workflow now
calls it with a single `node` invocation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@luans-qa
Copy link
Copy Markdown
Contributor Author

Done — extracted to apps/wallets/quickstart-devkit/scripts/notify-slack.js, alongside the existing generate-test-report.js. The workflow step is now a single node apps/wallets/quickstart-devkit/scripts/notify-slack.js call (c62c5d2).

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/wallets/quickstart-devkit/scripts/notify-slack.js:78-82
When tests are stored directly on `suite.tests` (rather than in `suite.specs`), the failure record uses `suite: parentPath || ""` — which is the path *before* the current suite, so the current suite's own title is dropped from the failure context shown in Slack. For any test nested directly under a describe block, the suite field will be truncated or empty. Using `currentPath` here aligns with how specs are handled.

```suggestion
                        failures.push({
                            title: test.title || suite.title || "Unknown Test",
                            suite: currentPath,
                            error: result?.error?.message || "No error message",
                        });
```

Reviews (3): Last reviewed commit: "refactor(ci): extract Slack notify scrip..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test Results

Status: Failed

Statistics

  • Total Tests: 5
  • Passed: 0 ✅
  • Failed: 1 ❌
  • Skipped: 4 ⚠️
  • Duration: 1.70 min

Test Details


This is a non-blocking smoke test. Full regression tests run separately.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Reviews (4): Last reviewed commit: "fix(ci): apply Biome formatting to notif..." | Re-trigger Greptile

@luans-qa luans-qa merged commit 09fe4b0 into main May 29, 2026
3 checks passed
@luans-qa luans-qa deleted the fix/nightly-e2e-browser-parallelism branch May 29, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants