[Test] Web acceptance stability - #4506
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors Playwright acceptance tests to reduce network-response dependencies and adds comprehensive tests for the "Use API" documentation drawer. Changes include member-invite modal helpers, prompt-registry UI navigation, new Use API snippet tests for variants and deployments, and a slug validation assertion in evaluator creation. ChangesUse API Snippet Tests
Test Refactoring to UI-Based Waiting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
bekossy
marked this pull request as ready for review
May 31, 2026 20:04
dosubot
Bot
added
size:XS
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/oss/tests/playwright/acceptance/use-api/index.ts (1)
111-143: 💤 Low valueConsolidate the two drawer-open helpers.
openVariantUseApiDrawerandopenDeploymentUseApiDrawerare identical except for the button locator. Consider parameterizing to remove the duplicated drawer-resolution block.♻️ Proposed consolidation
-const openVariantUseApiDrawer = async (page: any) => { - await page.waitForLoadState("networkidle") - const useApiButton = page.locator('[data-tour="api-code-button"]') - await expect(useApiButton).toBeVisible({timeout: 15000}) - await expect(useApiButton).toBeEnabled({timeout: 5000}) - await useApiButton.click() - - const drawer = page.locator(".ant-drawer-content-wrapper").filter({ - hasText: "How to use API", - }) - await expect(drawer).toBeVisible({timeout: 20000}) - return drawer -} - -const openDeploymentUseApiDrawer = async (page: any) => { - await page.waitForLoadState("networkidle") - const useApiButton = page.getByRole("button", {name: "Use API"}).first() - await expect(useApiButton).toBeVisible({timeout: 15000}) - await expect(useApiButton).toBeEnabled({timeout: 5000}) - await useApiButton.click() - - const drawer = page.locator(".ant-drawer-content-wrapper").filter({ - hasText: "How to use API", - }) - await expect(drawer).toBeVisible({timeout: 20000}) - return drawer -} +const openUseApiDrawer = async (page: any, useApiButton: any) => { + await expect(useApiButton).toBeVisible({timeout: 15000}) + await expect(useApiButton).toBeEnabled({timeout: 5000}) + await useApiButton.click() + + const drawer = page.locator(".ant-drawer-content-wrapper").filter({ + hasText: "How to use API", + }) + await expect(drawer).toBeVisible({timeout: 20000}) + return drawer +} + +const openVariantUseApiDrawer = (page: any) => + openUseApiDrawer(page, page.locator('[data-tour="api-code-button"]')) + +const openDeploymentUseApiDrawer = (page: any) => + openUseApiDrawer(page, page.getByRole("button", {name: "Use API"}).first())web/oss/tests/playwright/acceptance/evaluators/tests.ts (1)
354-355: ⚡ Quick winUpdate slug assertion to account for real slug transformation (UI uses
slugify)The slug field in
CreateEvaluatoris set from the (debounced) evaluator name via aslugifyhelper (toLowerCase()andreplace(/[^a-z0-9_\-]+/g, "-"), plus trimming/collapsing dashes). So the assertion should not assume a no-op transformation in general.That said, the current tests pass
evaluatorNamevalues likee2e-human-eval-${Date.now()}(already lowercase and hyphen-safe), so it matches the slug output today. For robustness, assert againstslugify(evaluatorName)instead ofevaluatorName.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 685200ae-6c11-485f-8a7f-e0a67aa8fa47
📒 Files selected for processing (8)
web/ee/tests/playwright/acceptance/members/index.tsweb/ee/tests/playwright/acceptance/use-api/use-api.spec.tsweb/oss/tests/playwright/10-use-api.tsweb/oss/tests/playwright/acceptance/evaluators/tests.tsweb/oss/tests/playwright/acceptance/features/use-api.featureweb/oss/tests/playwright/acceptance/prompt-registry/index.tsweb/oss/tests/playwright/acceptance/use-api/index.tsweb/oss/tests/playwright/acceptance/use-api/use-api.spec.ts
Contributor
Railway Preview Environment
Updated at 2026-06-24T16:11:31.957Z |
…iness Without --max-time/--connect-timeout, curl could hang indefinitely when a Railway preview accepted the TCP connection but stalled before sending an HTTP response. This caused the wait-for-readiness job to block for hours instead of cycling through its 30-attempt loop. - Add --max-time 10 --connect-timeout 5 to each curl attempt so the loop is always bounded (~10 min max across both URL checks). - Add timeout-minutes: 25 to the job as a defence-in-depth backstop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n reinstall playwright install --with-deps chromium was downloading 170MB and running apt-get for hundreds of X11/font packages on every run, taking ~17 minutes. This left almost no time for the auth bootstrap within the job timeout. Cache ~/.cache/ms-playwright keyed on the tests package.json hash so the browser binary is restored on cache hits (subsequent runs). playwright install still runs after the cache restore — it detects the binary is present and skips the download, but still verifies/installs any missing system deps via apt which is fast when packages are already cached by the runner. Also bumps the job timeout from 25 to 30 minutes to give the first (cold) run enough headroom. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
--with-deps runs apt-get for ~200 X11/font/GTK packages on top of the browser download, adding 25+ minutes on cold runs and causing the job to time out before the auth bootstrap could run. The ubuntu-latest runner already has Chromium's core runtime libraries; the auth bootstrap (login form + save cookies) doesn't need the full X11/font stack. Removing --with-deps cuts the install from ~29 min to ~1-2 min (binary download only, skipped entirely on cache hits). Timeout reduced to 15 min to match the realistic job budget: - URL health check: ≤5 min - checkout + node + pnpm: ~2 min - playwright install (binary only): ~1-2 min - auth bootstrap: ~3 min Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bekossy
marked this pull request as draft
June 19, 2026 08:29
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d41267af27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
AntD 6 renders checkbox inputs with opacity:0, which Chrome's
accessibility tree excludes — causing getByRole("checkbox") to return
0 elements and time out. Switch to .ant-checkbox-input (CSS class) to
bypass the ARIA tree; Playwright 1.60 considers opacity:0 elements
visible for CSS locators so check() works without force.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ead of using AntD checkboxes
…ed count in dialog
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5933b1cfd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…erception In CI, Playwright's hover-before-click takes long enough for the CellContentPopover's mouseEnterDelay (500ms) to elapse, opening a popover portal at the row center. The portal sits on top of the <tr> and intercepts the native click, so onRow.onClick never fires and the testcase selection count doesn't increment. dispatchEvent sends the click directly to the <tr> element without moving the mouse, so the hover timer never starts and the portal never appears. Affects both "preserve connected testset" tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65ce8ff147
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Switch from dispatchEvent("click") on <tr> to .getByRole("checkbox").click()
in both failing playground acceptance tests. Clicking the selection-column
checkbox fires AntD's rowSelection.onChange with the full computed key set,
eliminating the stale-selectedIdsRef race that caused the second row click
(France) to report only 1 selection instead of 2. The checkbox column is
also outside the CellContentPopover portal footprint, so no dispatchEvent
workaround is needed.
- Stabilise handleSetSelection in useTestsetSelection by reading
selectedRevisionId through a ref instead of capturing it in the useCallback
dep array. The previous dep caused handleSetSelection to get a new reference
on every revision change, propagating through inline arrows to
useAutoSelectLatestChild where the re-run could trigger a second
initSelectionDraft call that reset the user's in-progress testcase selection.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ckbox")
getByRole queries the ARIA accessibility tree which excludes elements with
aria-hidden="true". In production AntD builds the native <input type="checkbox">
inside ant-checkbox-input is rendered with aria-hidden, so getByRole finds
nothing and the locator times out in CI.
Switching to .locator(".ant-checkbox-inner") uses a plain CSS selector that
bypasses the accessibility tree, targets the always-visible 16×16px styled
checkbox span, and works uniformly across AntD build configurations.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 250edc89a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ommit After committing a testcase via "Add to testset", invalidateRevisionsListCache marks the TanStack Query as stale but TanStack Query still returns the old cached value synchronously on the next read while the background refetch runs. When the user immediately opens "Load Testset", the entity picker's auto-select fires with the stale revision list (missing the new revision), AutoSelectHandler unmounts after selection, and even after the background refetch completes with the new revision the handler is gone — leaving the user stuck on the old revision that doesn't show the newly added testcase. Fix: add setRevisionsListCache to the entity package which uses setQueryData to eagerly populate the cache with the fresh revision list already fetched in the save flow. The Load Testset entity picker then reads fresh data immediately and auto-selects the new latest revision, so the newly added testcase is visible. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…utside data.inputs
getCanonicalTraceMappingPaths uses isInputEnvelope to detect when data.inputs
is a wrapper around actual user variables (the data.inputs.inputs sub-object).
It fires if messages are a sibling inside data.inputs, or if data.inputs has
reserved sibling keys (prompt/messages/tools/functions).
Playground completion traces store messages at data.parameters.prompt.messages
— entirely outside data.inputs. Neither condition matched, so isInputEnvelope
returned false. The code then used data.inputs as the variable location,
enumerated its "inputs" key, and produced path "data.inputs.inputs" with
suggested column "inputs" (an object value). The testset received an object
column {country: "France"} instead of the flat string column "country".
Fix: add a third condition — when messages are found outside data.inputs
entirely, data.inputs.inputs is still the variable envelope. With this,
variablePaths now produces "data.inputs.inputs.country" and the suggested
column name "country" correctly matches the existing testset column.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rting The re-export line referenced setRevisionsListCache but the symbol was never imported into revisionMolecule.ts, causing a webpack module parse error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AntD v6 (@rc-component/checkbox v2) removed the .ant-checkbox-inner span that was used in AntD v5 as the styled visual indicator. The visual box is now rendered via CSS pseudo-elements on .ant-checkbox itself. Clicking .ant-checkbox-wrapper (the <label> element) is the correct approach — it uses the native label→input toggle mechanism and fires rowSelection.onChange regardless of aria-hidden on the input. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ardaerzin
approved these changes
Jun 24, 2026
The .ant-checkbox-wrapper click reliably times out in CI (120 s exceeded). Skipped with .skip so the suite stays green while the interaction issue is investigated separately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same .ant-checkbox-wrapper / dialog interaction issue as the commit-revision test. Skipped with .skip to unblock CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
3 participants
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
Testing
Verified locally
Added or updated tests
QA follow-up
Demo
Checklist
Contributor Resources