[Test] Web acceptance stability#4506
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 |
dosubot
Bot
added
size:XS
![coderabbitai[bot]](https://avatars.githubusercontent.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
Railway Preview Environment
|
…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>
dosubot
Bot
added
size:L
…invocation results
… consolidate provider creation and deletion logic
…ndency installation steps
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: 222c839e25
ℹ️ 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".
| const {projectId} = getProjectValues() | ||
| if (projectId) { | ||
| await triggerMetricsRefresh({projectId, runId, scenarioId}) | ||
| triggerMetricsRefresh({projectId, runId, scenarioId}).catch(() => {}) |
There was a problem hiding this comment.
When an annotation changes metric values, this starts triggerMetricsRefresh and then immediately invalidates/refetches the metric queries below. Since triggerMetricsRefresh performs the backend refresh POSTs asynchronously, those refetches can complete before scenario/run metrics are recomputed, caching stale metrics while the success toast is already shown; the drawer path still awaits this refresh before invalidating. Await the refresh before clearing/refetching the metric caches.
Useful? React with 👍 / 👎.
3 participants
Summary
Testing
Verified locally
Added or updated tests
QA follow-up
Demo
Checklist
Contributor Resources