Add Playwright E2E tests with sandbox isolation#488
Open
Conversation
- Use MIX_ENV=test so e2e tests run against the test database (port 4002) instead of wiping the dev database, addressing reviewer feedback - Remove duplicate sandbox plug and debug logging from endpoint.ex - Revert dev.exs changes (sandbox pool, commented logger line) - Update mix alias test.e2e to seed the test DB instead of dropping dev - Use Playwright baseURL config instead of hardcoded localhost:4000 - Fix Campaigns tests to use per-test sandbox (was bypassing via beforeAll) - Only run Chromium (remove Firefox/Safari as PR author suggested) - Remove committed test-results/.last-run.json, add to gitignore - Add Riders and Navigation test suites - Clean up test code: proper types, remove unnecessary clicks, semicolons https://claude.ai/code/session_01L8gaYbunRb7cRLN13P8Xqf
📝 WalkthroughWalkthroughThis PR introduces end-to-end testing infrastructure using Playwright, adding test configuration files, a comprehensive test suite covering authentication and core workflows, sandbox helpers for database isolation, and supporting documentation and configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Playwright Test
participant Phoenix as Phoenix Server
participant Sandbox as Ecto Sandbox
participant Context as Request Context
Test->>Phoenix: POST /sandbox (setup)
Phoenix->>Sandbox: Create isolated session
Sandbox-->>Phoenix: Return sessionId
Phoenix-->>Test: sessionId in response
Test->>Context: Register route handler
Context->>Context: Store sessionId for header injection
Test->>Test: Run test with injected headers
Test->>Context: Make requests (headers include x-session-id)
Context->>Phoenix: Route with x-session-id header
Phoenix->>Sandbox: Execute in isolated context
Sandbox-->>Phoenix: Return results
Phoenix-->>Test: Response
Test->>Phoenix: DELETE /sandbox (teardown)
Phoenix->>Sandbox: Cleanup isolated session
Sandbox-->>Phoenix: Session destroyed
Phoenix-->>Test: Confirmed
sequenceDiagram
participant Test as E2E Test
participant UI as Browser/UI
participant Phoenix as Phoenix Server
participant DB as Database
Test->>UI: Navigate to /login
UI->>Phoenix: GET /login
Phoenix->>UI: Render login form
Test->>UI: Fill phone number & submit
UI->>Phoenix: POST login credentials
Phoenix->>DB: Verify credentials
DB-->>Phoenix: Success
Phoenix->>UI: Redirect with flash message
Test->>UI: Assert flash message present
Test->>UI: Verify page navigated
UI-->>Test: Assertion passes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/bike_brigade_web/live/campaign_live/form_component.ex (1)
75-79:⚠️ Potential issue | 🟠 MajorHeterogeneous options format will cause issues with
options_for_select/2.The code mixes a tuple
{"", nil}with keyword list entries[key: p.name, value: p.id, label: p.name]. According to the Phoenix HTML docs,options_for_select/2expects a homogeneous list. Additionally, the keyword list format here uses non-standard keys—:labelis not a valid option key. The standard formats are either 2-tuples{label, value}(as used in messaging_form_component.ex) or keyword lists with:keyand:valuekeys only.Use consistent tuple format instead:
Proposed fix
def mount(socket) do - programs = for p <- Delivery.list_programs(), do: [key: p.name, value: p.id, label: p.name] + programs = for p <- Delivery.list_programs(), do: {p.name, p.id} {:ok, socket |> assign(:program_options, [{"", nil} | programs])} endRun
scripts/formatbefore committing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/bike_brigade_web/live/campaign_live/form_component.ex` around lines 75 - 79, The program options are built heterogeneously (a tuple {"", nil} mixed with keyword lists [key: ..., value: ..., label: ...]) which breaks options_for_select/2; change the construction in the block that builds programs (where Delivery.list_programs() is mapped and assigned to :program_options) to produce a homogeneous list of 2-tuples like {label, value} (e.g., {p.name, p.id}) and prepend {"", nil} to that list instead of using keyword lists, then run scripts/format before committing.
🧹 Nitpick comments (3)
test/e2e/.gitignore (1)
1-1: Remove the leading blank line.Starting the file with a blank line is unconventional for
.gitignorefiles.✨ Proposed fix
- # Playwright🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/.gitignore` at line 1, Remove the leading blank line at the top of the .gitignore file so the file starts immediately with the first ignore pattern; open the .gitignore, delete the initial empty line (the "leading blank line" at the top of the file), and ensure the first non-empty line is the first ignore pattern.test/e2e/helpers/sandbox.ts (1)
5-5: Consider using proper Playwright types instead ofany.Using
anyforcontext,route, andrequestparameters loses TypeScript's type-checking benefits. Playwright provides proper types for these.♻️ Suggested type improvements
+import { request, test as base, BrowserContext, Route, Request } from '@playwright/test'; -import { request, test as base } from '@playwright/test'; -async function setupSandbox(context: any) { +async function setupSandbox(context: BrowserContext) { // ... - await context.route('**/*', async (route: any, request: any) => { + await context.route('**/*', async (route: Route, req: Request) => { - const headers = request.headers(); + const headers = req.headers();Also applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers/sandbox.ts` at line 5, Replace the use of any by importing and using Playwright types: change the function signature of setupSandbox to accept a BrowserContext (or BrowserContext from '@playwright/test' / 'playwright'), and update any handler parameter types from any to Route and Request (e.g., route: Route, request: Request) where they appear (including the usages flagged at line ~16). Ensure you add the corresponding import(s) for BrowserContext, Route, and Request at the top of the file so TypeScript can type-check the context, route, and request parameters.test/e2e/all.spec.ts (1)
72-75: Pagination navigation by index is fragile.Using
.nth(2)to navigate pagination assumes a fixed structure. If pagination changes (e.g., different number of page links), this test could break.♻️ Consider a more resilient selector
- await page.getByRole('navigation', { name: 'Pagination' }).getByRole('link').nth(2).click(); + // Navigate to next page using a more descriptive selector if available + await page.getByRole('navigation', { name: 'Pagination' }).getByRole('link', { name: /next|2|→/i }).click();Or if there's a "Next" button/link in the pagination, prefer that.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/all.spec.ts` around lines 72 - 75, The test uses a fragile index-based selector page.getByRole('navigation', { name: 'Pagination' }).getByRole('link').nth(2) to click a pagination link; replace it with a resilient selector (e.g., click the "Next" pagination control via page.getByRole('link', { name: /next/i }) or a stable aria-label/button text) and ensure you await the navigation/update before asserting programName visibility; update the click target and ensure the expectation still uses page.getByText(programName).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 117: Update the README heading string "End to End testing with
Playwright" to use hyphens as a compound adjective: change it to "End-to-End
testing with Playwright" wherever the heading text appears (match the exact
heading text to find it).
In `@test/e2e/helpers/sandbox.ts`:
- Around line 30-37: The teardownSandbox function creates an API request context
via request.newContext() but never disposes it; after performing the DELETE call
include explicit cleanup by awaiting requestContext.dispose() (i.e., call await
requestContext.dispose() after the delete) to mirror setupSandbox and prevent
the resource leak from requestContext.
- Around line 5-28: The requestContext created in setupSandbox is never
disposed, leaking Playwright resources; wrap the usage of the requestContext
(the variable requestContext in function setupSandbox) in a try/finally (or
ensure final cleanup) and call await requestContext.dispose() in the finally
block after you have obtained the response and sessionId but before returning;
keep the rest of the logic (the context.route handler and context.addInitScript
for sessionId) unchanged so sandbox isolation continues to work.
---
Outside diff comments:
In `@lib/bike_brigade_web/live/campaign_live/form_component.ex`:
- Around line 75-79: The program options are built heterogeneously (a tuple {"",
nil} mixed with keyword lists [key: ..., value: ..., label: ...]) which breaks
options_for_select/2; change the construction in the block that builds programs
(where Delivery.list_programs() is mapped and assigned to :program_options) to
produce a homogeneous list of 2-tuples like {label, value} (e.g., {p.name,
p.id}) and prepend {"", nil} to that list instead of using keyword lists, then
run scripts/format before committing.
---
Nitpick comments:
In `@test/e2e/.gitignore`:
- Line 1: Remove the leading blank line at the top of the .gitignore file so the
file starts immediately with the first ignore pattern; open the .gitignore,
delete the initial empty line (the "leading blank line" at the top of the file),
and ensure the first non-empty line is the first ignore pattern.
In `@test/e2e/all.spec.ts`:
- Around line 72-75: The test uses a fragile index-based selector
page.getByRole('navigation', { name: 'Pagination' }).getByRole('link').nth(2) to
click a pagination link; replace it with a resilient selector (e.g., click the
"Next" pagination control via page.getByRole('link', { name: /next/i }) or a
stable aria-label/button text) and ensure you await the navigation/update before
asserting programName visibility; update the click target and ensure the
expectation still uses page.getByText(programName).
In `@test/e2e/helpers/sandbox.ts`:
- Line 5: Replace the use of any by importing and using Playwright types: change
the function signature of setupSandbox to accept a BrowserContext (or
BrowserContext from '@playwright/test' / 'playwright'), and update any handler
parameter types from any to Route and Request (e.g., route: Route, request:
Request) where they appear (including the usages flagged at line ~16). Ensure
you add the corresponding import(s) for BrowserContext, Route, and Request at
the top of the file so TypeScript can type-check the context, route, and request
parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7623c374-3be0-4cdc-815d-cf3b81f41836
⛔ Files ignored due to path filters (1)
test/e2e/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.gitignoreREADME.mdconfig/dev.exslib/bike_brigade_web/live/campaign_live/form_component.exlib/bike_brigade_web/live/program_live/form_component.html.heexmix.exstest/e2e/.gitignoretest/e2e/all.spec.tstest/e2e/helpers/sandbox.tstest/e2e/package.jsontest/e2e/playwright.config.ts
💤 Files with no reviewable changes (1)
- config/dev.exs
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
|
|
||
| ## End to End testing with Playwright |
There was a problem hiding this comment.
Minor grammar: "End to End" should be hyphenated.
Per standard English grammar, compound adjectives before a noun should be hyphenated: "End-to-End testing".
📝 Suggested fix
-## End to End testing with Playwright
+## End-to-End Testing with Playwright🧰 Tools
🪛 LanguageTool
[grammar] ~117-~117: Use a hyphen to join words.
Context: ... limitations under the License. ## End to End testing with Playwright E2E tests r...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 117, Update the README heading string "End to End testing
with Playwright" to use hyphens as a compound adjective: change it to
"End-to-End testing with Playwright" wherever the heading text appears (match
the exact heading text to find it).
Comment on lines
+5
to
+28
| async function setupSandbox(context: any) { | ||
| const requestContext = await request.newContext(); | ||
| const response = await requestContext.post(`${BASE_URL}/sandbox`, { | ||
| headers: { | ||
| 'Cache-Control': 'no-store' | ||
| } | ||
| }); | ||
|
|
||
| const sessionId = await response.text(); | ||
|
|
||
| // Add sessionId header to all requests for sandbox isolation | ||
| await context.route('**/*', async (route: any, request: any) => { | ||
| const headers = request.headers(); | ||
| headers['x-session-id'] = sessionId; | ||
| await route.continue({ headers }); | ||
| }); | ||
|
|
||
| // Store sessionId for LiveView WebSocket connections | ||
| await context.addInitScript(({ sessionId }: { sessionId: string }) => { | ||
| (window as any).sessionId = sessionId; | ||
| }, { sessionId }); | ||
|
|
||
| return sessionId; | ||
| } |
There was a problem hiding this comment.
Request context should be disposed after use to prevent resource leaks.
The requestContext created in setupSandbox is never disposed. Playwright's APIRequestContext holds resources that should be cleaned up.
🛡️ Proposed fix to dispose request context
async function setupSandbox(context: any) {
const requestContext = await request.newContext();
- const response = await requestContext.post(`${BASE_URL}/sandbox`, {
- headers: {
- 'Cache-Control': 'no-store'
- }
- });
-
- const sessionId = await response.text();
+ try {
+ const response = await requestContext.post(`${BASE_URL}/sandbox`, {
+ headers: {
+ 'Cache-Control': 'no-store'
+ }
+ });
+ const sessionId = await response.text();
+
+ // Add sessionId header to all requests for sandbox isolation
+ await context.route('**/*', async (route: any, request: any) => {
+ const headers = request.headers();
+ headers['x-session-id'] = sessionId;
+ await route.continue({ headers });
+ });
+
+ // Store sessionId for LiveView WebSocket connections
+ await context.addInitScript(({ sessionId }: { sessionId: string }) => {
+ (window as any).sessionId = sessionId;
+ }, { sessionId });
+
+ return sessionId;
+ } finally {
+ await requestContext.dispose();
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function setupSandbox(context: any) { | |
| const requestContext = await request.newContext(); | |
| const response = await requestContext.post(`${BASE_URL}/sandbox`, { | |
| headers: { | |
| 'Cache-Control': 'no-store' | |
| } | |
| }); | |
| const sessionId = await response.text(); | |
| // Add sessionId header to all requests for sandbox isolation | |
| await context.route('**/*', async (route: any, request: any) => { | |
| const headers = request.headers(); | |
| headers['x-session-id'] = sessionId; | |
| await route.continue({ headers }); | |
| }); | |
| // Store sessionId for LiveView WebSocket connections | |
| await context.addInitScript(({ sessionId }: { sessionId: string }) => { | |
| (window as any).sessionId = sessionId; | |
| }, { sessionId }); | |
| return sessionId; | |
| } | |
| async function setupSandbox(context: any) { | |
| const requestContext = await request.newContext(); | |
| try { | |
| const response = await requestContext.post(`${BASE_URL}/sandbox`, { | |
| headers: { | |
| 'Cache-Control': 'no-store' | |
| } | |
| }); | |
| const sessionId = await response.text(); | |
| // Add sessionId header to all requests for sandbox isolation | |
| await context.route('**/*', async (route: any, request: any) => { | |
| const headers = request.headers(); | |
| headers['x-session-id'] = sessionId; | |
| await route.continue({ headers }); | |
| }); | |
| // Store sessionId for LiveView WebSocket connections | |
| await context.addInitScript(({ sessionId }: { sessionId: string }) => { | |
| (window as any).sessionId = sessionId; | |
| }, { sessionId }); | |
| return sessionId; | |
| } finally { | |
| await requestContext.dispose(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/helpers/sandbox.ts` around lines 5 - 28, The requestContext created
in setupSandbox is never disposed, leaking Playwright resources; wrap the usage
of the requestContext (the variable requestContext in function setupSandbox) in
a try/finally (or ensure final cleanup) and call await requestContext.dispose()
in the finally block after you have obtained the response and sessionId but
before returning; keep the rest of the logic (the context.route handler and
context.addInitScript for sessionId) unchanged so sandbox isolation continues to
work.
Comment on lines
+30
to
+37
| async function teardownSandbox(sessionId: string) { | ||
| const requestContext = await request.newContext(); | ||
| await requestContext.delete(`${BASE_URL}/sandbox`, { | ||
| headers: { | ||
| 'x-session-id': sessionId | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Teardown request context also needs disposal.
Same resource leak concern as in setupSandbox.
🛡️ Proposed fix
async function teardownSandbox(sessionId: string) {
const requestContext = await request.newContext();
- await requestContext.delete(`${BASE_URL}/sandbox`, {
- headers: {
- 'x-session-id': sessionId
- }
- });
+ try {
+ await requestContext.delete(`${BASE_URL}/sandbox`, {
+ headers: {
+ 'x-session-id': sessionId
+ }
+ });
+ } finally {
+ await requestContext.dispose();
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function teardownSandbox(sessionId: string) { | |
| const requestContext = await request.newContext(); | |
| await requestContext.delete(`${BASE_URL}/sandbox`, { | |
| headers: { | |
| 'x-session-id': sessionId | |
| } | |
| }); | |
| } | |
| async function teardownSandbox(sessionId: string) { | |
| const requestContext = await request.newContext(); | |
| try { | |
| await requestContext.delete(`${BASE_URL}/sandbox`, { | |
| headers: { | |
| 'x-session-id': sessionId | |
| } | |
| }); | |
| } finally { | |
| await requestContext.dispose(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/helpers/sandbox.ts` around lines 30 - 37, The teardownSandbox
function creates an API request context via request.newContext() but never
disposes it; after performing the DELETE call include explicit cleanup by
awaiting requestContext.dispose() (i.e., call await requestContext.dispose()
after the delete) to mirror setupSandbox and prevent the resource leak from
requestContext.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/bike_brigade_web/live/campaign_live/form_component.ex">
<violation number="1" location="lib/bike_brigade_web/live/campaign_live/form_component.ex:75">
P1: This keyword list format is not valid for `Phoenix.HTML.Form.options_for_select/2`. Each `[key: name, value: id, label: name]` keyword list will be interpreted as three separate options (`:key`, `:value`, `:label`) rather than a single option. The original `{p.name, p.id}` tuple format was correct.</violation>
</file>
<file name="test/e2e/helpers/sandbox.ts">
<violation number="1" location="test/e2e/helpers/sandbox.ts:6">
P2: The `APIRequestContext` created in `setupSandbox` is never disposed. Call `await requestContext.dispose()` after extracting the session ID to avoid leaking connections.</violation>
<violation number="2" location="test/e2e/helpers/sandbox.ts:13">
P1: The response from the `/sandbox` POST is used without checking its status. If the endpoint is unreachable or errors, `sessionId` will contain an error body, and every test will fail with misleading errors. Check `response.ok()` before proceeding.</violation>
<violation number="3" location="test/e2e/helpers/sandbox.ts:32">
P3: Same resource leak here — the `requestContext` is never disposed. Wrap in `try/finally` and call `await requestContext.dispose()` to clean up.</violation>
</file>
<file name="test/e2e/all.spec.ts">
<violation number="1" location="test/e2e/all.spec.ts:44">
P2: Race condition: no wait between typing the address and clicking Save. The `LiveLocation` component geocodes the address asynchronously via `phx-change`, but the test clicks Save immediately after `pressSequentially` returns. Add a wait for the geocoded address to resolve (e.g., wait for the expanded location fields or map marker to appear) to avoid flaky failures.</violation>
<violation number="2" location="test/e2e/all.spec.ts:49">
P2: Use `toHaveValue` instead of `toContainText` to assert textarea field values. `toContainText` checks `textContent`, not the form field's `value` property, and is not the idiomatic Playwright assertion for input/textarea elements.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| @impl true | ||
| def mount(socket) do | ||
| programs = for p <- Delivery.list_programs(), do: {p.name, p.id} | ||
| programs = for p <- Delivery.list_programs(), do: [key: p.name, value: p.id, label: p.name] |
There was a problem hiding this comment.
P1: This keyword list format is not valid for Phoenix.HTML.Form.options_for_select/2. Each [key: name, value: id, label: name] keyword list will be interpreted as three separate options (:key, :value, :label) rather than a single option. The original {p.name, p.id} tuple format was correct.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/bike_brigade_web/live/campaign_live/form_component.ex, line 75:
<comment>This keyword list format is not valid for `Phoenix.HTML.Form.options_for_select/2`. Each `[key: name, value: id, label: name]` keyword list will be interpreted as three separate options (`:key`, `:value`, `:label`) rather than a single option. The original `{p.name, p.id}` tuple format was correct.</comment>
<file context>
@@ -72,7 +72,7 @@ defmodule BikeBrigadeWeb.CampaignLive.FormComponent do
@impl true
def mount(socket) do
- programs = for p <- Delivery.list_programs(), do: {p.name, p.id}
+ programs = for p <- Delivery.list_programs(), do: [key: p.name, value: p.id, label: p.name]
{:ok,
</file context>
Suggested change
| programs = for p <- Delivery.list_programs(), do: [key: p.name, value: p.id, label: p.name] | |
| programs = for p <- Delivery.list_programs(), do: {p.name, p.id} |
| } | ||
| }); | ||
|
|
||
| const sessionId = await response.text(); |
There was a problem hiding this comment.
P1: The response from the /sandbox POST is used without checking its status. If the endpoint is unreachable or errors, sessionId will contain an error body, and every test will fail with misleading errors. Check response.ok() before proceeding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/helpers/sandbox.ts, line 13:
<comment>The response from the `/sandbox` POST is used without checking its status. If the endpoint is unreachable or errors, `sessionId` will contain an error body, and every test will fail with misleading errors. Check `response.ok()` before proceeding.</comment>
<file context>
@@ -0,0 +1,49 @@
+ }
+ });
+
+ const sessionId = await response.text();
+
+ // Add sessionId header to all requests for sandbox isolation
</file context>
| const BASE_URL = 'http://localhost:4002'; | ||
|
|
||
| async function setupSandbox(context: any) { | ||
| const requestContext = await request.newContext(); |
There was a problem hiding this comment.
P2: The APIRequestContext created in setupSandbox is never disposed. Call await requestContext.dispose() after extracting the session ID to avoid leaking connections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/helpers/sandbox.ts, line 6:
<comment>The `APIRequestContext` created in `setupSandbox` is never disposed. Call `await requestContext.dispose()` after extracting the session ID to avoid leaking connections.</comment>
<file context>
@@ -0,0 +1,49 @@
+const BASE_URL = 'http://localhost:4002';
+
+async function setupSandbox(context: any) {
+ const requestContext = await request.newContext();
+ const response = await requestContext.post(`${BASE_URL}/sandbox`, {
+ headers: {
</file context>
|
|
||
| // Verify the edit persisted | ||
| await page.getByRole('link', { name: `Edit , ${programName}` }).click(); | ||
| await expect(page.getByLabel('Campaign Blurb (please keep')).toContainText('This is a test program that was updated'); |
There was a problem hiding this comment.
P2: Use toHaveValue instead of toContainText to assert textarea field values. toContainText checks textContent, not the form field's value property, and is not the idiomatic Playwright assertion for input/textarea elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/all.spec.ts, line 49:
<comment>Use `toHaveValue` instead of `toContainText` to assert textarea field values. `toContainText` checks `textContent`, not the form field's `value` property, and is not the idiomatic Playwright assertion for input/textarea elements.</comment>
<file context>
@@ -0,0 +1,178 @@
+
+ // Verify the edit persisted
+ await page.getByRole('link', { name: `Edit , ${programName}` }).click();
+ await expect(page.getByLabel('Campaign Blurb (please keep')).toContainText('This is a test program that was updated');
+ });
+});
</file context>
| // Edit the program | ||
| await page.getByRole('link', { name: `Edit , ${programName}` }).click(); | ||
| await page.getByRole('textbox', { name: 'Campaign Blurb (please keep' }).fill('This is a test program that was updated'); | ||
| await page.getByRole('button', { name: 'Save' }).click(); |
There was a problem hiding this comment.
P2: Race condition: no wait between typing the address and clicking Save. The LiveLocation component geocodes the address asynchronously via phx-change, but the test clicks Save immediately after pressSequentially returns. Add a wait for the geocoded address to resolve (e.g., wait for the expanded location fields or map marker to appear) to avoid flaky failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/all.spec.ts, line 44:
<comment>Race condition: no wait between typing the address and clicking Save. The `LiveLocation` component geocodes the address asynchronously via `phx-change`, but the test clicks Save immediately after `pressSequentially` returns. Add a wait for the geocoded address to resolve (e.g., wait for the expanded location fields or map marker to appear) to avoid flaky failures.</comment>
<file context>
@@ -0,0 +1,178 @@
+ // Edit the program
+ await page.getByRole('link', { name: `Edit , ${programName}` }).click();
+ await page.getByRole('textbox', { name: 'Campaign Blurb (please keep' }).fill('This is a test program that was updated');
+ await page.getByRole('button', { name: 'Save' }).click();
+ await expect(page.getByText('Success! program updated')).toBeVisible();
+
</file context>
|
|
||
| async function teardownSandbox(sessionId: string) { | ||
| const requestContext = await request.newContext(); | ||
| await requestContext.delete(`${BASE_URL}/sandbox`, { |
There was a problem hiding this comment.
P3: Same resource leak here — the requestContext is never disposed. Wrap in try/finally and call await requestContext.dispose() to clean up.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/helpers/sandbox.ts, line 32:
<comment>Same resource leak here — the `requestContext` is never disposed. Wrap in `try/finally` and call `await requestContext.dispose()` to clean up.</comment>
<file context>
@@ -0,0 +1,49 @@
+
+async function teardownSandbox(sessionId: string) {
+ const requestContext = await request.newContext();
+ await requestContext.delete(`${BASE_URL}/sandbox`, {
+ headers: {
+ 'x-session-id': sessionId
</file context>
Contributor
|
Hype 💃 |
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.
Describe your changes
This PR adds a comprehensive end-to-end testing suite using Playwright with database sandbox isolation. The test suite covers critical user workflows including authentication, program management, campaign creation, and navigation.
Key additions:
E2E Test Suite (
test/e2e/all.spec.ts):Sandbox Test Helper (
test/e2e/helpers/sandbox.ts):Playwright Configuration:
playwright.config.tswith test configurationpackage.jsonwith Playwright and Faker dependenciesMinor fixes:
.gitignoreto exclude test artifactsTest execution:
Tests run against the test database (not dev), ensuring development data safety. The sandbox approach provides complete test isolation without manual cleanup.
Checklist before requesting a review
https://claude.ai/code/session_01L8gaYbunRb7cRLN13P8Xqf
Summary by cubic
Add Playwright end-to-end tests with per-test Ecto sandbox isolation, running against the test DB on port 4002. Covers core flows (auth, programs, campaigns, riders, navigation) and adds a
mixalias to run the test server.New Features
@playwright/testand@faker-js/faker.x-session-idfor HTTP and LiveView.Migration
npm install.MIX_ENV=test mix test.e2eto start the server on 4002.npm run test:uiornpm test.Written for commit 8cb6724. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Chores