-
Notifications
You must be signed in to change notification settings - Fork 894
test: improve E2E framework #9469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
65a3171
3cdff53
f301c09
fffe83a
fd5850a
d486d23
564aab8
5935bf6
2f4626c
e81432f
ca061d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
import { test, expect } from "@playwright/test" | ||
import { beforeCoderTest } from "../hooks" | ||
|
||
test.beforeEach(async ({ page }) => await beforeCoderTest(page)) | ||
|
||
test("list templates", async ({ page, baseURL }) => { | ||
await page.goto(`${baseURL}/templates`, { waitUntil: "networkidle" }) | ||
await page.goto(`${baseURL}/templates`, { waitUntil: "domcontentloaded" }) | ||
await expect(page).toHaveTitle("Templates - Coder") | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,10 @@ | ||
import { test } from "@playwright/test" | ||
import { createTemplate, createWorkspace, startAgent } from "../helpers" | ||
import { | ||
createTemplate, | ||
createWorkspace, | ||
startAgent, | ||
stopAgent, | ||
} from "../helpers" | ||
import { randomUUID } from "crypto" | ||
import { beforeCoderTest } from "../hooks" | ||
|
||
|
@@ -28,13 +33,13 @@ test("web terminal", async ({ context, page }) => { | |
], | ||
}) | ||
await createWorkspace(page, template) | ||
await startAgent(page, token) | ||
const agent = await startAgent(page, token) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still have the unfortunate problem that any error in a test will terminate execution and the agent won't be cleaned up. Can we somehow make it a post-test cleanup hook that always runs and checks if an agent is running? This isn't a big problem per-se but can cause trouble when running locally -> suddenly all tests fail after one failure. And also cause noise in test output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to implement an easy way but failed. The clue is to pass the PID of the process/group of process to the |
||
|
||
// Wait for the web terminal to open in a new tab | ||
const pagePromise = context.waitForEvent("page") | ||
await page.getByTestId("terminal").click() | ||
const terminal = await pagePromise | ||
await terminal.waitForLoadState("networkidle") | ||
await terminal.waitForLoadState("domcontentloaded") | ||
|
||
// Ensure that we can type in it | ||
await terminal.keyboard.type("echo hello") | ||
|
@@ -50,4 +55,5 @@ test("web terminal", async ({ context, page }) => { | |
} | ||
await new Promise((r) => setTimeout(r, 250)) | ||
} | ||
await stopAgent(agent) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should necessarily change
networkidle
todomcontentloaded
without adding additional wait checks. For instance "wait until element X is present in the html". In a sense,networkidle
should be better as it will wait longer compared todomcontentloaded
which will fire immediately after dom even if network requests are still outbound.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In previous PRs, I added extra "wait until" conditions, so in theory it should be faster and still safe with
domcontentloaded
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's perfect 👍🏻