Skip to content

Make two timing-dependent browser tests wait for what they assert - #413

Merged
demegire merged 2 commits into
mainfrom
fix/flaky-browser-tests
Oct 13, 2026
Merged

demegire merged 2 commits into
mainfrom
fix/flaky-browser-tests

Conversation

@demegire

Copy link
Copy Markdown
Contributor

What and why

With merges deploying automatically, a flaky test on main blocks the deploy. Two different browser tests failed on consecutive main commits today (3d1e746, 7ba43ed); a re-run passed both times.

  • lock page: the steps tick as the agent connects… The page renders its steps before its first poll answers. The test then switched the fake agent to "connected" right away. When the first poll reached the server after that switch, the page never saw the waiting state, and by design ("a step counts once per page view, and only when this page watched it happen") it didn't record agent_connected. The test now waits until the first poll has been answered, and resets the counter for each theme. The page itself is unchanged.
  • Extension eventually() gave up after 100 event-loop turns, not after a time limit, so real async work (credential hashing) on a loaded runner failed concurrent project connects serialize onto one browser credential. It now waits up to 5 s by time.
  • staging-check also exempts test files outside the top-level tests/ folder (any tests/ directory, *.test.js|cjs|mjs). Otherwise test-only changes like this one would need a staging deploy.

Verification

  • web/lock-page.browser.test.js: 6/6 passed. Under load (4 parallel copies × 3 rounds), all 12 runs passed.
  • Browser extension: npm test 50/50, test:collection 45/45.
  • The new exemption pattern still requires staging for web/app.js, src/tin_lite/static/app.js and browser-extension/src/background.js.

Tested on staging

Tests and a workflow pattern only; nothing that runs on the VM changes. staging-check runs from main's copy, so the new exemption doesn't apply to this PR yet, and I've set its staging status by hand.

🤖 Generated with Claude Code

The lock-page test moved the agent along before the page's first poll answered; when
that poll arrived late, the page never saw the waiting state and, by design, did not
count agent_connected. The extension harness gave up after 100 event-loop turns
instead of after a time limit. Both flaked on main and blocked automatic deploys.
staging-check now also treats test files outside tests/ as not reaching the service.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@demegire
demegire requested a review from a team as a code owner October 13, 2026 15:12
main requires the staging status from GitHub Actions, so a status set from a personal
account never counted. A maintainer now runs staging-check with the commit and how it
was tested (an infra/ change deployed with its own deploy.sh, for example).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@demegire

Copy link
Copy Markdown
Contributor Author

Added: a hand-set staging status from a personal account never satisfies main's rule, which requires the status from GitHub Actions (that's also why #410 needed the maintainer bypass). staging-check now has a Run workflow with the commit SHA and how it was tested, limited to PROD_DEPLOYERS. It posts the status as GitHub Actions, with the maintainer and reason in the description. docs/staging.md is updated. This PR can't use it until it's on main, so it needs the maintainer bypass this once.

@demegire
demegire merged commit 3743c86 into main Oct 13, 2026
5 of 6 checks passed
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.

1 participant