Skip to content

test: fix the thread-head and dependency-sampler flakes - #129

Merged
QuicksilverSlick merged 1 commit into
mainfrom
fix/two-flaky-tests
Nov 2, 2026
Merged

QuicksilverSlick merged 1 commit into
mainfrom
fix/two-flaky-tests

Conversation

@QuicksilverSlick

Copy link
Copy Markdown
Owner

Factory-Task: fix two listed flakes (desktop-thread-head-stale-edit-55, pg-boot-sampler-scrape-race)
Built-by: claude-code/claude-opus-5-5
Undo: git revert the merge commit. This PR changes test code only, so nothing in the product moves. Put the two flake entries back if they were already removed.

Fixes two tests that keep throwing out unrelated PRs. Both flake entries can go once this merges. The thread-head test also moves from :55 to :56, so that entry's signature stops matching anyway.

1. thread-head-stale-edit.spec.ts

Cause. The reply button is only clickable while its row is hovered, and the spec used click({ force: true }). A forced click doesn't wait, so it could land while the row wasn't a hit target:

  • Scroll window: virtua's VList sets pointer-events: none on the list while it scrolls. Sampling the click point every frame showed the hit target as the message-timeline scroller, with pointer-events: none computed on the row and its parent, for about 40 to 150 ms. The geometry didn't change during that window. A run that failed had the scroller at the click point just before the click.
  • Re-pin: in one sampled run the row moved from y=490 to y=528 about 285 ms later (scrollTop 296 to 258), after the click point had been read.

Either way the click misses and the panel never opens, which is the CI signature.

Fix. The spec calls openGeneral (waits for the head refresh and the bottom pin) and pressRowAction (focus + Enter on the same button). The action bar also shows on focus-within, so this needs no pointer.

What the test proves is unchanged. The thread-replies gate and every assertion stay as they were.

Not the normal click you might expect. A hover + normal click also passed 15/15 unforced, but it timed out 4/4 under the forcing below, because hover can't finish once the row has moved under the composer. The keyboard version passed. Both files also end up byte-identical to upstream block#8682, so they merge cleanly on the next sync.

2. startup_owns_the_dependency_sampler

Cause. metrics-exporter-prometheus reads counters, then gauges, then histograms. A scrape overlapping the first sample can hold the completion gauge but not the counter that was written before it.

Fix. After the gauge is seen, the test scrapes again through the same helper and asserts on that second scrape. Its render starts after the first one finished, so it holds everything sample wrote before the gauge.

What was exercised

Spec, on a frozen e2e build of main. Private port, --workers=1, no retries. Before is main's spec, after is this branch's.

Condition Before (failed) After (failed)
No added load 1/55 0/30
During a cargo build 2/12 0/12
24 CPU spinners on 24 cores 3/40 0/40
4x CDP CPU throttle 0/22 0/10
Total 6/129 0/92

All 6 failures were the CI signature: message-thread-panel never visible.

  • Row moved off the pointer on arrival (forced): before failed 4/4, after passed 18/18.
  • Product fix reverted (headAux dropped): the new spec fails 3/3 at the head assertion, on the stale "these two PRs?" text. It still catches the original bug.

Sampler test, real relay binary. Throwaway Postgres 17 and Redis 7 containers, 24 CPU spinners.

Check Result
Real test, before 40/40 passed
Real test, after 40/40 passed
Probe: 200,000 cold starts on exporter 0.18.3, same write order as sample First scrape missing the counter: 20. Second scrape missing the counter or histogram: 0

Other checks: rustfmt --check, clippy -D warnings on this test target, biome on both TS files, tsc, desktop file sizes, and a clean git merge-tree with current main.

Known limits

  • The torn scrape did not reproduce in the real test locally (40/40 before), so the sampler evidence for the race is the exporter probe plus the reasoning above, not a before/after failure rate.
  • The sampler test can't run unmodified on Windows. env_clear() removes SystemRoot and the relay exits at once. I added a local passthrough to run it and did not commit it.
  • The Postgres lane wasn't run as CI runs it. I loaded the schema with raw psql as schema/schema.sql describes, and ran one test binary on one database rather than through nextest and its per-test wrapper.
  • Spec runs used local Chrome on Windows, not CI's Linux Chromium. I ran only this spec, not the full smoke suite. Nothing else imports the new helpers.
  • The 4x throttle never reproduced the failure (0/22 before), so that row says little. The rows with real load are the useful ones.
  • The next upstream merge gains one conflict. boot_lifecycle.rs conflicts with upstream's own fix (fix(relay): revoke invitations and restrict claims block/buzz#8237), which waits for all three series in one scrape. Resolve it by taking upstream's block.
  • The same forced click remains in other specs. auxiliary-pane-close-visibility.spec.ts:89 and channel-shared-header-backdrop.spec.ts:82 force the reply button, and mention-clipboard.spec.ts:554 and :815, message-copy-link.spec.ts:24 and messaging.spec.ts:2611 force more-actions. None of them is on the task list, so I left them.

🤖 Generated with Claude Code

thread-head-stale-edit.spec.ts:55 used replyButton.click({ force: true }).
virtua sets pointer-events: none on the list while it scrolls, and the
timeline re-pins after opening #general, so the forced click could land on
the scroller or where the row used to be. The panel then never opened. The
spec now waits for the head refresh and presses the button by keyboard, using
the same lines as upstream block#8682. Its assertions are unchanged.

startup_owns_the_dependency_sampler asserted on the first scrape that showed
the completion gauge. The exporter renders counters before gauges, so that
scrape can overlap the first sample and hold the gauge without the counter.
The assertions now read a second scrape, whose render starts after the gauge
was seen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@QuicksilverSlick QuicksilverSlick added the lane:ready Builder is done: green, agent-reviewed, exercised. The steward puts it in the next train. label Nov 2, 2026
@QuicksilverSlick
QuicksilverSlick merged commit 08e361f into main Nov 2, 2026
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lane:ready Builder is done: green, agent-reviewed, exercised. The steward puts it in the next train.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant