t133: Add Playwright E2E tests for agent builder UI#612
Merged
superdav42 merged 11 commits intomainfrom Mar 24, 2026
Merged
Conversation
Covers the agent builder shipped in t082 (PR #437): - Create agent with custom name, system prompt, model, and tool profile - Validation: missing name or slug shows error notice - Agent list: cards render with edit/delete buttons and prompt preview - Edit agent: form pre-populated, success notice, updated name in card - Delete agent: confirm removes card; dismiss keeps it - Agent selector in chat: custom agent appears, is selectable, resets to default - Full lifecycle test: create → verify in chat selector → edit → delete All 24 tests discovered and parseable by Playwright. Closes #608
📝 WalkthroughWalkthroughAdds a new Playwright E2E test suite for the Agent Builder UI with deterministic REST mocks and helpers; also updates store thunks to apply optimistic updates when updating or deleting agents. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Playwright)
participant WP as WordPress Admin UI
participant MockAPI as Mock `/gratis-ai-agent` API
participant DB as Real WP REST (cleanup)
Browser->>WP: Navigate to agent settings page
WP->>MockAPI: GET /agents
MockAPI-->>WP: Return in-memory agents
WP-->>Browser: Render Agent Builder
Browser->>WP: Submit "Create Agent" form
WP->>MockAPI: POST /agents
MockAPI-->>MockAPI: Persist agent in-memory
MockAPI-->>WP: 201 Created
WP-->>Browser: Show success, render new agent card
Browser->>WP: Open Chat panel selector
WP->>MockAPI: GET /agents
MockAPI-->>WP: Return list with custom agent
WP-->>Browser: Selector shows custom agent
Browser->>WP: Edit agent
WP->>MockAPI: PUT/PATCH /agents/:id
MockAPI-->>MockAPI: Update in-memory agent
MockAPI-->>WP: 200 OK
WP-->>Browser: Show update success
Browser->>WP: Delete agent (confirm)
WP->>MockAPI: DELETE /agents/:id
MockAPI-->>MockAPI: Remove in-memory agent
MockAPI-->>WP: 200 OK
WP-->>Browser: Agent card removed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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
🧹 Nitpick comments (1)
tests/e2e/agent-builder.spec.js (1)
524-546: Consider usingpage.once()for one-time dialog handlers.Using
page.on('dialog', ...)registers a persistent listener for the page lifecycle. While this works correctly since each test gets an isolated page context,page.once()is more explicit for one-shot dialog handling and prevents accidental handler accumulation if test patterns are copied elsewhere.♻️ Suggested refinement
test( 'deleting an agent removes it from the list', async ( { page } ) => { // Accept the confirmation dialog automatically. - page.on( 'dialog', ( dialog ) => dialog.accept() ); + page.once( 'dialog', ( dialog ) => dialog.accept() ); const card = getAgentCards( page ).first();test( 'dismissing the delete confirmation keeps the agent', async ( { page, } ) => { // Dismiss the confirmation dialog. - page.on( 'dialog', ( dialog ) => dialog.dismiss() ); + page.once( 'dialog', ( dialog ) => dialog.dismiss() ); const card = getAgentCards( page ).first();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/agent-builder.spec.js` around lines 524 - 546, Replace the persistent dialog listeners with one-time handlers: in the two tests "deleting an agent removes it from the list" and "dismissing the delete confirmation keeps the agent", change page.on('dialog', ...) to page.once('dialog', ...) so the dialog handler is registered only for the single click action that triggers the dialog; update the calls around getDeleteButton(card).click() and the imports/uses of getAgentCards and getDeleteButton remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/agent-builder.spec.js`:
- Around line 121-127: The GET mock currently returns the full agents array for
every request via route.fulfill; change the handler that calls route.fulfill to
inspect the incoming request (route.request().url or route.request().uri/path)
and when the path matches /agents/:id extract the id, find the matching agent in
the agents array (e.g., by agents.find(a => a.id == id)) and fulfill with
JSON.stringify(singleAgent) and status 200; otherwise keep the existing behavior
of returning the full agents array. Ensure you reference the same route.fulfill
call and the agents array when implementing this conditional logic.
---
Nitpick comments:
In `@tests/e2e/agent-builder.spec.js`:
- Around line 524-546: Replace the persistent dialog listeners with one-time
handlers: in the two tests "deleting an agent removes it from the list" and
"dismissing the delete confirmation keeps the agent", change page.on('dialog',
...) to page.once('dialog', ...) so the dialog handler is registered only for
the single click action that triggers the dialog; update the calls around
getDeleteButton(card).click() and the imports/uses of getAgentCards and
getDeleteButton remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c06da40-8c95-4983-b5c0-c308f769c3ad
📒 Files selected for processing (1)
tests/e2e/agent-builder.spec.js
This was referenced Mar 19, 2026
Three root causes:
1. Route handler stacking: page.route() handlers accumulate across
beforeEach calls in the same browser context. Playwright matches the
first registered handler, so subsequent mocks never took effect.
Fix: call page.unrouteAll() at the start of mockAgentsApi() to clear
stale handlers before registering new ones.
2. Transient create notice: the component calls resetForm() immediately
after createAgent() resolves, which sets notice to null in the same
React render cycle as the success notice. The notice never renders.
Fix: assert the observable outcome (form closed, card count) instead
of the transient notice for create operations.
3. Tool profile options race: selectOption({ label: 'Read Only' }) ran
before the mocked tool-profiles API response populated the <select>.
Fix: wait for the option to be attached to the DOM before selecting.
4. Agent selector render timing: AgentSelector returns null until the
Redux store marks agents as loaded. Fix: add { timeout: 10000 } to
the initial toBeVisible() assertion so the component has time to
mount after the mocked API response is processed.
…akage
Two root causes for the remaining 13 failing tests:
1. Mock registration order: mockAgentsApi() was called AFTER
loginToWordPress(). The admin dashboard (loaded after login) renders
the floating widget which calls fetchAgents(). With the mock not yet
registered, this request hit the real server. On CI the real server
had a stale agent from a previous test (test 7 'creates an agent'
bypassed the mock for the same reason), causing subsequent tests to
see 1 card instead of 0, empty form fields, and a missing agent
selector. Fix: call mockAgentsApi() before loginToWordPress() in
every beforeEach block and in the Full Lifecycle test.
2. selectOption by label: selectOption({ label: 'Read Only' }) timed
out because @wordpress/components SelectControl may render option
text with extra whitespace or differ across WP versions. Fix: use
selectOption('read-only') by value, which is stable.
Additional hardening: add explicit toBeVisible({ timeout: 10000 })
waits before interacting with agent cards in Edit/Delete/List tests
so the mock response has time to populate the store before the test
proceeds.
The mock for /v1/tool-profiles was returning { slug: 'read-only' } which
does not exist in ToolProfiles::get_builtins(). The real built-in slugs
are 'wp-read-only', 'wp-full-management', etc. This caused selectOption()
to time out in CI because the option value it was looking for was never
present in the rendered select element.
Changes:
- Mock now returns { slug: 'wp-read-only', name: 'WP Read Only' } and
{ slug: 'wp-full-management', name: 'WP Full Management' } to match
the real implementation.
- All three selectOption() calls updated to use 'wp-read-only'.
- All three hasText wait conditions updated to 'WP Read Only'.
- Added timeout: 10000 to the .gratis-ai-agent-agent-prompt-preview
assertion to handle async card body rendering.
Root cause: tests were finding 2 agent cards instead of 1 because a previous test's POST leaked to the real WordPress DB when the route mock failed to intercept during a flaky retry. Subsequent tests then saw both the real DB agent and the mocked one. Changes: - Extract cleanupRealAgents() helper that deletes all real DB agents via a fresh browser page with no route mocks (bypasses page.route handlers) - Call cleanupRealAgents() in beforeEach for all describe blocks so each test starts with a clean DB state - Convert test.beforeAll cleanup to a reusable function (no longer needed as a global hook since per-test cleanup is more reliable) - Add spinner wait in goToAgentsTab() so tests don't assert before agentsLoaded=true (fixes timing-related card count failures) - Remove page.unrouteAll() from mockAgentsApi() — each test gets a fresh page context so stale handlers cannot accumulate - Add explicit visibility waits and longer timeouts as safety nets
Contributor
Author
|
Pulse fix (7178821): Committed the worker's accumulated changes after it thrashed for 40+ mins without pushing (74 msgs / 0 commits). Root cause of the 16 failing tests: test isolation failure — a previous test's POST leaked to the real WordPress DB when the route mock failed to intercept during a flaky retry. Subsequent tests found 2 agent cards (1 real + 1 mocked) instead of 1. Fix: per-test |
…?rest_route= format wp-env serves the REST API via index.php?rest_route= with URL-encoded slashes (%2F). Playwright's page.route() regex matchers compare against the raw encoded URL, so patterns like /gratis-ai-agent\/v1\/agents/ never match — the mock falls through to the real server which returns an empty array, causing 0 agent cards to be found. Replace all three regex route matchers in mockAgentsApi() with function matchers that call decodeURIComponent() before the includes() check. Also decode the request URL inside the handler body so that DELETE /agents/:id id extraction and POST guard work correctly with encoded paths.
…o update assertion The mock was returning the full agents array for both GET /agents (list) and GET /agents/:id (single). After a PUT, the component calls GET /agents/:id to refresh the card — receiving an array instead of a single object caused the component to show 0 cards. Fix: add a separate handler for GET /agents/:id that returns the single matching agent object. Also add a 10s timeout to the post-update card assertion to allow time for the component to re-fetch and re-render.
After update/delete the component re-fetches the agent list; the cards briefly disappear during the re-render. The default 5s timeout is not enough in CI. Increase to 10s for toContainText and toHaveCount assertions that follow a PUT/DELETE mutation.
… updates The three failing tests shared two root causes: 1. Edit Agent / Full Lifecycle — 'updated name in card' assertion: The AgentBuilder component renders cards only when showForm=false. After a successful PATCH the form stays open (resetForm is not called for updates), so getAgentCards() found 0 elements and timed out. Fix: wait for the success notice, then click Cancel to close the form before asserting on the card content. 2. Delete Agent — card count stays at 1: The deleteAgent thunk called dispatch.fetchAgents() (fire-and-forget) after the DELETE. If the mocked GET response was delayed or the real server returned stale data the store was never updated. Fix: add an optimistic setAgents() call immediately after the DELETE (and similarly after PATCH in updateAgent) so the UI reflects the mutation before the re-fetch completes. The re-fetch still runs to confirm server state.
WordPress's apiFetch http-v1 middleware converts PATCH/PUT/DELETE requests to POST with an X-HTTP-Method-Override header. The mock was checking route.request().method() which always returned 'POST', so the DELETE and PATCH handlers never fired. The re-fetch GET after each mutation returned the unmodified agents array, causing: - 'updated agent name is reflected in the card' to see the old name - 'deleting an agent removes it from the list' to see count=1 not 0 - 'full lifecycle' to fail on the edit step for the same reason Fix: read the X-HTTP-Method-Override header first and use it as the effective method. Guard the POST/create branch with !overrideHeader so that overridden DELETE/PATCH requests (sent as POST) don't accidentally match the create handler.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/store/index.js (2)
2215-2220: Remove the dead fallback from the optimistic merge.The PATCH endpoint already returns the full updated agent on success, so
updated || datashould never need thedatabranch. Falling back to the request payload masks contract/mock regressions and can briefly keep server-normalized fields stale until the re-fetch lands.♻️ Proposed cleanup
const current = select.getAgents(); const merged = current.map( ( a ) => - a.id === id ? { ...a, ...( updated || data ) } : a + a.id === id ? { ...a, ...updated } : a );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/index.js` around lines 2215 - 2220, The optimistic merge uses a dead fallback `updated || data`, which can mask server contract regressions; change the merge in the handler that builds `merged` to always use the server-returned object (use `updated` only) when `a.id === id` instead of falling back to `data`—locate the code that calls `select.getAgents()` and constructs `merged` and remove the `|| data` fallback so the optimistic update uses the full returned agent object from the PATCH response.
2221-2223: Await the re-fetch in both thunks to ensure awaiting these actions means "store settled."
src/settings-page/agent-builder.jsawaitsupdateAgent()anddeleteAgent(), but both currently resolve whilefetchAgents()is still in flight. Addingawaitto the re-fetch calls makes these thunks properly settle only after the server state is confirmed in the store, transforming the semantic meaning from "write request finished" to "store settled."♻️ Proposed change
dispatch.setAgents( merged ); // Re-fetch to confirm server state. - dispatch.fetchAgents(); + await dispatch.fetchAgents();dispatch.setAgents( current.filter( ( a ) => a.id !== id ) ); // Re-fetch to confirm server state. - dispatch.fetchAgents(); + await dispatch.fetchAgents();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/index.js` around lines 2221 - 2223, The re-fetch after setting agents should be awaited so the updateAgent and deleteAgent thunks only resolve once the store reflects server state; modify the thunks (updateAgent and deleteAgent) to await dispatch.fetchAgents() instead of calling it fire-and-forget (the code around dispatch.setAgents and dispatch.fetchAgents should await the fetchAgents promise) so callers like settings-page/agent-builder.js truly get a "store settled" result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/store/index.js`:
- Around line 2215-2220: The optimistic merge uses a dead fallback `updated ||
data`, which can mask server contract regressions; change the merge in the
handler that builds `merged` to always use the server-returned object (use
`updated` only) when `a.id === id` instead of falling back to `data`—locate the
code that calls `select.getAgents()` and constructs `merged` and remove the `||
data` fallback so the optimistic update uses the full returned agent object from
the PATCH response.
- Around line 2221-2223: The re-fetch after setting agents should be awaited so
the updateAgent and deleteAgent thunks only resolve once the store reflects
server state; modify the thunks (updateAgent and deleteAgent) to await
dispatch.fetchAgents() instead of calling it fire-and-forget (the code around
dispatch.setAgents and dispatch.fetchAgents should await the fetchAgents
promise) so callers like settings-page/agent-builder.js truly get a "store
settled" result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85e718b2-0721-41e8-804a-f439945c3ddf
📒 Files selected for processing (2)
src/store/index.jstests/e2e/agent-builder.spec.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/agent-builder.spec.js
superdav42
added a commit
that referenced
this pull request
Mar 24, 2026
This was referenced Apr 26, 2026
Merged
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
Adds
tests/e2e/agent-builder.spec.js— 24 Playwright E2E tests covering the agent builder UI shipped in t082 (PR #437).Test coverage
Create agent (9 tests)
Agent list (3 tests)
Edit agent (4 tests)
Delete agent (2 tests)
Agent selector in chat (5 tests)
Full lifecycle (1 test)
Implementation notes
/gratis-ai-agent/v1/agents,/v1/tool-profiles,/v1/providers) are intercepted viapage.route()— no live WordPress database requiredadmin-page.spec.js,chat-interactions.spec.js)--listverified)Closes #608
Summary by CodeRabbit