Skip to content

fix(app): deflake per-series number format e2e test - #3028

Open
wrn14897 wants to merge 1 commit into
mainfrom
warren/flaky-e2e
Open

fix(app): deflake per-series number format e2e test#3028
wrn14897 wants to merge 1 commit into
mainfrom
warren/flaky-e2e

Conversation

@wrn14897

Copy link
Copy Markdown
Member

Why

The dashboard e2e test per-series format overrides chart-wide format and falls back when reset to inherit fails intermittently on CI shards (example: failed run, all 3 attempts timed out at the final assertion).

Root cause is a razor-thin time budget:

  • test.setTimeout(15000) — the only sub-30s timeout in the file; every sibling full-stack dashboard test uses 30–90s and the project default is 60s. The test performs ~10 UI steps, 3 query round-trips, 2 tile saves, and two expect.poll calls that can consume 10s each.
  • The 10s poll after saveTile() can also genuinely expire on a loaded shard, since saveTile() returns before the tile refetches and re-renders.

What

  • Bump the test timeout 15s → 60s (file convention / project default)
  • Bump the tile re-render poll 10s → 15s

Test-only change, no changeset. Verified locally with make dev-e2e FILE=dashboard GREP="per-series format" — passes in 12.4s, confirming how tight the 15s budget was.

The test had a 15s total timeout — the only one in the file below 30s —
while containing two 10s expect.poll calls, three query round-trips, and
two tile saves. On loaded CI shards the final poll after the second save
ran out of wall clock on all retries.

Bump the test timeout to the file convention (60s, the project default)
and give the tile re-render poll 15s, since saveTile() returns before
the tile refetches.
@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 84ab848

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

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 28, 2026 9:15pm
hyperdx-storybook Ready Ready Preview Aug 28, 2026 9:15pm

Request Review

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 0
  • Production lines changed: 0 (+ 4 in test files, excluded from tier calculation)
  • Branch: warren/flaky-e2e
  • Author: wrn14897

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reduces flakiness in the per-series number-format dashboard E2E test by aligning its overall timeout with the project default and allowing more time for the asynchronous tile re-render poll.

  • Raises the test timeout from 15 seconds to 60 seconds.
  • Raises the post-save polling timeout from 10 seconds to 15 seconds.

Confidence Score: 5/5

The PR appears safe to merge because it only increases bounded E2E wait budgets to values consistent with existing project conventions.

The changed timeouts accommodate the documented asynchronous dashboard tile re-render without altering application behavior or exceeding the surrounding CI budget.

Important Files Changed

Filename Overview
packages/app/tests/e2e/features/dashboard.spec.ts The timeout increases are consistent with project defaults and neighboring dashboard E2E patterns, with no correctness issue identified.

Reviews (1): Last reviewed commit: "fix(app): deflake per-series number form..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a two-line, test-only change to a Playwright e2e spec (packages/app/tests/e2e/features/dashboard.spec.ts): the per-test timeout is raised 15000 → 60000 to match the file convention and project default, and the tile-re-render poll after saveTile() is raised 10000 → 15000. No production code, contracts, or assertions change.

The reviewers confirmed the change is safe:

  • The bumped expect.poll still asserts .toBe(true) against cells.length > 0 && cells.every(c => c.includes(substring)), so a longer timeout extends the wait budget without weakening the invariant or allowing a vacuous pass on an unrendered tile.
  • The poll cannot false-pass on the "briefly shows previously-rendered values" window it documents: at each call site the stale and expected format markers are mutually exclusive (% vs $), so a stale cell never satisfies the predicate.
  • Polling (rather than a fixed sleep) remains the correct handling for the async saveTile() → refetch → re-render latency that is the flake's root cause.

No P0/P1, P2, or P3 findings.


Reviewers (3): testing, correctness, julik-frontend-races.

Testing gaps: none identified — the change tunes timing budgets on an existing test and adds no new behavior requiring coverage.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 323 passed • 1 skipped • 1426s

Status Count
✅ Passed 323
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant