fix(tests): playground role-select selectors, apiBase pre-nav bug, mark 5 flaky tests retry-eligible - #5896
Merged
Conversation
…fix apiBase pre-navigation bug
- playground/tests.ts addNewPrompt(): replace the 5-hop ancestor::/locator('..') XPath
chain with a scoped, index-correlated lookup (data-testid="prompt-schema-control",
added to PromptSchemaControl.tsx in PR #5895, scopes the "Message" button and role
selectors; the Nth .message-user-select role button and Nth .editor-input[role="textbox"]
always belong to the same message row, since ChatMessageList renders one of each per
message in array order). Fixes "Should update the prompt and save the changes" and
"should save the current changes as a new variant", both confirmed passing live against
144.76.237.122:8180. Depends on PR #5895 (dropdown-menu.tsx animate-none fix) for the
data-testid and for the role dropdown to actually close after a selection.
- agent-chat/tests.ts apiBase(): seedAgentChatApp() runs before the test navigates
anywhere, so page.url() is still "about:blank" -- a non-empty string that defeats
`page.url() || fallback`, and whose .origin serializes to the literal string "null"
(a bogus /null/api/... request that 404s in ~300ms). Treat about:blank as "no real
page yet" and fall back to AGENTA_WEB_URL, matching the test config's own baseURL.
Confirmed live: the test now gets past seeding and into navigateToAgentPlayground()
(60s+ runtime instead of ~300ms), then fails downstream on a real, separate issue --
waitForPath times out because the seeded is_agent app's playground URL resolves to a
bare /playground path without the /apps/<id> segment, suggesting agent-type workflows
have moved under a different route in the current IA. Not chased; flagging for
a follow-up.
…ahmoud's call
Wraps each test in its own nested test.describe(...) with
test.describe.configure({retries: 2}) -- the narrowest scope that covers exactly one
test, leaving every sibling test in the same file at the suite's global default
(0 retries locally, 2 in CI; playwright.config.ts has no per-project override).
These five were identified (PR #5854) as low-confidence timeouts/races, not stale
selectors, and are read-only or use uniquely-named (Date.now()-suffixed) fixtures, so a
retry never collides with or destroys prior data:
- auto-evaluation/index.ts: "should run a single evaluation" -- openAutoEvaluationRunFromList's
search-input toHaveValue race.
- evaluators/index.ts: "should navigate to the evaluators page and display both automatic
and human evaluator tabs" -- waitForEvaluatorsQuery timeout.
- human-annotation/index.ts: "should create a new evaluator inline and annotate a scenario
from the annotate tab" -- annotateCurrentHumanScenario's annotation-form predicate timeout.
- observability/index.ts: "view traces" and "should open a span and drill into its
attributes" -- both can lag past the polling window on async trace indexing.
Verified live against 144.76.237.122:8180: `--list` collects all 80 tests with no
errors, "should navigate to..." passes cleanly under the new describe scope, and
"should run a single evaluation" genuinely retried twice (3 total attempts) before
failing, confirming the configure({retries: 2}) is wired up and does not break test
loading. Pass/fail of these five stays nondeterministic by design -- nothing here was
skipped.
dosubot
Bot
added
the
size:XS
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Playwright acceptance tests now resolve API URLs safely from blank pages, retry selected suites up to two times, and use scoped prompt selectors with dropdown readiness synchronization. ChangesPlaywright acceptance test resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a2ee04f9-ffc1-487d-9fab-2827b5b57bd8
📒 Files selected for processing (6)
web/oss/tests/playwright/acceptance/agent-chat/tests.tsweb/oss/tests/playwright/acceptance/auto-evaluation/index.tsweb/oss/tests/playwright/acceptance/evaluators/index.tsweb/oss/tests/playwright/acceptance/human-annotation/index.tsweb/oss/tests/playwright/acceptance/observability/index.tsweb/oss/tests/playwright/acceptance/playground/tests.ts
Contributor
Railway Preview Environment
Updated at 2026-08-10T10:20:53.522Z |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Three independent fixes to the web acceptance suite, continuing the repair from #5854/#5855.
1. Playground prompt-message XPath fragility (confirmed real)
playground/index.ts:178("Should update the prompt and save the changes") and:219("should save the current changes as a new variant") both failed at
scrollIntoViewIfNeededtiming out onaddNewPrompt()'s 5-hopxpath=ancestor::.../locator('..')chain, which no longer matches the current DOM (theprompt editor now renders through
PromptSchemaControl, shared with the evaluatorDrillInView, not the structure the chain assumed).
Fix: replaced the chain with a scoped, index-correlated lookup:
data-testid="prompt-schema-control"toPromptSchemaControl.tsx's root — thisships in fix(frontend): dropdown menus never unmount after close (scroll-fade vs Radix Presence) #5895, not here (see Dependency below).
addNewPrompt()now scopes the "Message" button and.message-user-selectrolebuttons to that testid, and pairs the Nth role button with the Nth
.editor-input[role="textbox"]— eachChatMessageItemrenders exactly one of each,in message order, so no ancestor traversal is needed.
A second, real bug surfaced during investigation (live-debugged against
144.76.237.122:8180, not a test artifact): selecting a role from the dropdown updatedthe value correctly but its Radix portal never unmounted —
data-stateflipped to"closed"but the node stayed mounted and interactive forever, permanentlyaria-hidingthe rest of the page (confirmed viagetComputedStyle+ manual repro).Root cause:
DropdownMenuContent'soverflow-y-autoclass collides withglobals.css'sscroll-fade animation, which Radix's
Presencewaits on before unmounting — ascroll-timeline animation never fires the
animationendit's waiting for. Fixed with aone-class
animate-noneoverride, verified via isolated CSS-specificity test. Thisfix also ships in #5895, and is a prerequisite for these two tests to pass — a selector
rewrite alone can't work around a permanently-open, aria-hiding overlay.
Verified live: both tests pass (16.0s and 14.0s) against
144.76.237.122:8180once#5895's changes were live on that stack.
2.
apiBase()about:blank bug (confirmed real, fixed)agent-chat/attach-send-render-reload.spec.tsfailed in ~300ms:seedAgentChatApp()callsapiBase(page)before the test navigates anywhere, sopage.url()is still"about:blank"— a non-empty string that defeatspage.url() || fallback, and whose.originserializes to the literal string"null"(a bogus
/null/api/...request).Fix: treat
about:blankexplicitly as "no real page yet" and fall back toAGENTA_WEB_URL, matching the test config's ownbaseURL.Verified live: the test now runs past seeding and into
navigateToAgentPlayground()(60s+ instead of ~300ms). It then fails on a separate, real, downstream issue —
expectPathtimes out waiting for/apps/<id>/playground; the seededis_agentapp'splayground URL instead resolves to a bare
/playgroundpath with no/apps/<id>segment, suggesting agent-type workflows have moved under a different route in the
current IA. Not chased, per scope — flagging as a follow-up.
3. Mark 5 nondeterministic tests retry-eligible (not skipped)
Per Mahmoud's call: three low-confidence timeouts (two evaluator-area: a
toHaveValuerace in
openAutoEvaluationRunFromList, and awaitForEvaluatorsQuerytimeout; onehuman-annotation predicate timeout) and two observability trace-indexing tests are
read-only or use uniquely-named fixtures, so a retry is safe — never skipped.
Implementation: each test wrapped in its own nested
test.describe(...)withtest.describe.configure({retries: 2})— the narrowest scope covering exactly one test,leaving every sibling test in the same file at the suite's global default (0 retries
locally, 2 in CI;
playwright.config.tshas no per-project override, confirmed noexisting retry pattern in the suite to conflict with).
auto-evaluation/index.ts: "should run a single evaluation"evaluators/index.ts: "should navigate to the evaluators page and display bothautomatic and human evaluator tabs"
human-annotation/index.ts: "should create a new evaluator inline and annotate ascenario from the annotate tab"
observability/index.ts: "view traces" and "should open a span and drill into itsattributes"
Verified live:
--listcollects all 80 tests with no errors; the evaluators testpasses cleanly under the new describe scope; the auto-evaluation test genuinely retried
twice (3 total attempts visible in the run log) before failing on unrelated flakiness,
confirming
configure({retries: 2})is wired up correctly and doesn't break testloading. Pass/fail of these five stays nondeterministic by design.
Dependency
Requires #5895 to merge first. This branch's playground selectors use the
data-testid="prompt-schema-control"that PR adds, and rely on itsanimate-nonefixfor the role dropdown to close at all. Verification above was run against the live dev
stack after #5895's changes were applied there directly (not yet merged) — CI here won't
go green until #5895 lands on
release/v0.112.0.Test plan
144.76.237.122:8180about:blank/nullbug; downstream/apps/<id>routing failure reported separately, not fixed here
--listclean (80 tests, 21 files); one test passed under the newscope, one genuinely retried twice as configured
pnpm turbo run lint --filter=@agenta/oss— clean (pre-existing warnings only,unrelated files)
prettier --writeon all touched files