Unverified 提交 971aca34 authored 作者: keppo-bot[bot]'s avatar keppo-bot[bot] 提交者: GitHub

Deflake CI E2E failures (#3311)

## Summary - Deflake CI E2E failures from https://github.com/dyad-sh/dyad/actions/runs/25126503338. - Await streamed token-usage persistence before token-count invalidation so the context-limit banner sees the real max token usage. - Harden E2E helpers around GitHub sync completion, already-applied proposals, queued Lexical submissions, visual-save button re-renders, and Windows line endings. - Update the component-tagger upgrade snapshot from 0.8.0 to 0.9.0. ## Root Cause The failing run had several independent races and one stale snapshot: - The context-limit banner could refetch token counts before the assistant message's max token usage had been saved, leaving actualMaxTokens null after a high-token stream. - GitHub assertions snapshotted the connected-repo panel while the sync button was still in its transient Syncing state. - Some E2E helpers used one-shot Lexical fills/clicks where Playwright can act during React or DOM replacement windows. - The code-editor assertion compared raw file contents, which fails on Windows CRLF output. - The select-component upgrade snapshot still expected the older component tagger version. ## Why This Fix Is Correct - Awaiting the existing max-token update makes the later chat/token invalidations observe the persisted usage instead of racing it. - GitHub sync waits now use the stable connected-repo panel state: Syncing hidden, Sync to GitHub enabled, and the panel-local success text visible. - Queued-message and visual-save actions now retry the exact UI contract the tests require: the queued row appears or the save button is visible, enabled, and clickable. - The editor test normalizes only line endings, preserving the file-content assertion while making it platform-independent. ## Test Plan - npm run fmt && npm run lint:fix && npm run ts - PYTHON=/usr/bin/python3 npm run build - PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/select_component.spec.ts --update-snapshots - PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/context_limit_banner.spec.ts e2e-tests/debugging_logs.spec.ts e2e-tests/edit_code.spec.ts e2e-tests/github.spec.ts e2e-tests/queued_message.spec.ts e2e-tests/visual_editing.spec.ts - First run: 21 passed, 6 GitHub helper strict-mode failures due duplicate success toast text. - PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/github.spec.ts - npm test 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/3311" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open in Devin Review"> </picture> </a> <!-- devin-review-badge-end --> --------- Co-authored-by: 's avatarWill Chen <7344640+wwwillchen@users.noreply.github.com>
上级 89f95ff6
...@@ -39,6 +39,10 @@ async function replaceEditorContent(page: Page, content: string) { ...@@ -39,6 +39,10 @@ async function replaceEditorContent(page: Page, content: string) {
await page.keyboard.type(content); await page.keyboard.type(content);
} }
function normalizeLineEndings(value: string) {
return value.replace(/\r\n/g, "\n");
}
test("edit code", async ({ po }) => { test("edit code", async ({ po }) => {
await po.setUp({ autoApprove: true }); await po.setUp({ autoApprove: true });
const editedFilePath = path.join("src", "components", "made-with-dyad.tsx"); const editedFilePath = path.join("src", "components", "made-with-dyad.tsx");
...@@ -107,13 +111,22 @@ test("edit code edits the right file during rapid switches", async ({ po }) => { ...@@ -107,13 +111,22 @@ test("edit code edits the right file during rapid switches", async ({ po }) => {
await expect await expect
.poll( .poll(
() => fs.readFileSync(path.join(appPath, firstOpenedFilePath), "utf8"), () =>
normalizeLineEndings(
fs.readFileSync(path.join(appPath, firstOpenedFilePath), "utf8"),
),
{ timeout: Timeout.MEDIUM }, { timeout: Timeout.MEDIUM },
) )
.toEqual(firstFileEdit); .toEqual(firstFileEdit);
await expect await expect
.poll(() => fs.readFileSync(path.join(appPath, robotsFilePath), "utf8"), { .poll(
timeout: Timeout.MEDIUM, () =>
}) normalizeLineEndings(
fs.readFileSync(path.join(appPath, robotsFilePath), "utf8"),
),
{
timeout: Timeout.MEDIUM,
},
)
.toEqual(updatedRobotsFile); .toEqual(updatedRobotsFile);
}); });
...@@ -52,9 +52,25 @@ export class ChatActions { ...@@ -52,9 +52,25 @@ export class ChatActions {
}).toPass({ timeout: Timeout.SHORT }); }).toPass({ timeout: Timeout.SHORT });
} }
clickNewChat({ index = 0 }: { index?: number } = {}) { async clickNewChat({ index = 0 }: { index?: number } = {}) {
// There is two new chat buttons... // There are two new chat buttons.
return this.page.getByTestId("new-chat-button").nth(index).click(); const previousChatId = new URL(this.page.url()).searchParams.get("id");
await this.page.getByTestId("new-chat-button").nth(index).click();
await expect(async () => {
const currentChatId = new URL(this.page.url()).searchParams.get("id");
if (previousChatId === null) {
expect(currentChatId).not.toBeNull();
} else {
expect(currentChatId).not.toBe(previousChatId);
}
const chatInput = this.getChatInput();
await expect(chatInput).toBeVisible({ timeout: 1_000 });
const text = await chatInput.textContent({ timeout: 1_000 });
expect(text?.trim() ?? "").toBe("");
}).toPass({ timeout: Timeout.MEDIUM });
} }
private getRetryButton() { private getRetryButton() {
...@@ -103,8 +119,8 @@ export class ChatActions { ...@@ -103,8 +119,8 @@ export class ChatActions {
await chatInput.fill(prompt); await chatInput.fill(prompt);
const visiblePrompt = prompt.replace(/@app:/g, "@"); const visiblePrompt = prompt.replace(/@app:/g, "@");
expect(await chatInput.textContent()).toContain(visiblePrompt); expect(await chatInput.textContent()).toContain(visiblePrompt);
await expect(sendButton).toBeEnabled(); await expect(sendButton).toBeEnabled({ timeout: 1_000 });
await sendButton.click(); await sendButton.click({ timeout: 1_000 });
}).toPass({ timeout: Timeout.MEDIUM }); }).toPass({ timeout: Timeout.MEDIUM });
if (!skipWaitForCompletion) { if (!skipWaitForCompletion) {
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
*/ */
import { Page, expect } from "@playwright/test"; import { Page, expect } from "@playwright/test";
import { Timeout } from "../../constants";
export class GitHubConnector { export class GitHubConnector {
constructor( constructor(
...@@ -29,6 +30,7 @@ export class GitHubConnector { ...@@ -29,6 +30,7 @@ export class GitHubConnector {
async clickCreateRepoButton() { async clickCreateRepoButton() {
await this.page.getByRole("button", { name: "Create Repo" }).click(); await this.page.getByRole("button", { name: "Create Repo" }).click();
await this.waitForSyncToFinish();
} }
async fillCreateRepoName(name: string) { async fillCreateRepoName(name: string) {
...@@ -60,9 +62,11 @@ export class GitHubConnector { ...@@ -60,9 +62,11 @@ export class GitHubConnector {
async clickConnectToRepoButton() { async clickConnectToRepoButton() {
await this.page.getByRole("button", { name: "Connect to repo" }).click(); await this.page.getByRole("button", { name: "Connect to repo" }).click();
await this.waitForSyncToFinish();
} }
async snapshotConnectedRepo() { async snapshotConnectedRepo() {
await this.waitForSyncToFinish();
await expect( await expect(
this.page.getByTestId("github-connected-repo"), this.page.getByTestId("github-connected-repo"),
).toMatchAriaSnapshot(); ).toMatchAriaSnapshot();
...@@ -82,6 +86,21 @@ export class GitHubConnector { ...@@ -82,6 +86,21 @@ export class GitHubConnector {
async clickSyncToGithubButton() { async clickSyncToGithubButton() {
await this.page.getByRole("button", { name: "Sync to GitHub" }).click(); await this.page.getByRole("button", { name: "Sync to GitHub" }).click();
await this.waitForSyncToFinish();
}
async waitForSyncToFinish() {
await expect(
this.page.getByRole("button", { name: "Syncing..." }),
).toBeHidden({ timeout: Timeout.LONG });
await expect(
this.page.getByRole("button", { name: "Sync to GitHub" }),
).toBeEnabled({ timeout: Timeout.LONG });
await expect(
this.page
.getByTestId("github-connected-repo")
.getByText("Successfully pushed to GitHub!"),
).toBeVisible({ timeout: Timeout.LONG });
} }
async clickDisconnectRepoButton() { async clickDisconnectRepoButton() {
......
import { test, testSkipIfWindows, Timeout } from "./helpers/test_helper"; import { test, testSkipIfWindows, Timeout } from "./helpers/test_helper";
import { expect, Locator } from "@playwright/test"; import { expect, Locator, type Page } from "@playwright/test";
async function queueMessage(page: Page, chatInput: Locator, message: string) {
await expect(async () => {
await chatInput.click();
await chatInput.fill(message);
expect(await chatInput.textContent()).toContain(message);
await chatInput.press("Enter");
await expect(page.locator("li", { hasText: message })).toBeVisible({
timeout: 1_000,
});
}).toPass({ timeout: Timeout.MEDIUM });
}
test.describe("queued messages", () => { test.describe("queued messages", () => {
let chatInput: Locator; let chatInput: Locator;
...@@ -19,8 +31,7 @@ test.describe("queued messages", () => { ...@@ -19,8 +31,7 @@ test.describe("queued messages", () => {
await expect(chatInput).toBeVisible(); await expect(chatInput).toBeVisible();
// While streaming, send another message - this should be queued // While streaming, send another message - this should be queued
await chatInput.fill("tc=2"); await queueMessage(po.page, chatInput, "tc=2");
await chatInput.press("Enter");
// Verify the queued message indicator is visible // Verify the queued message indicator is visible
// The UI shows "{count} Queued" followed by "- {status}" // The UI shows "{count} Queued" followed by "- {status}"
...@@ -55,12 +66,9 @@ test.describe("queued messages", () => { ...@@ -55,12 +66,9 @@ test.describe("queued messages", () => {
await expect(chatInput).toBeVisible(); await expect(chatInput).toBeVisible();
// Queue 3 messages while streaming // Queue 3 messages while streaming
await chatInput.fill("tc=first"); await queueMessage(po.page, chatInput, "tc=first");
await chatInput.press("Enter"); await queueMessage(po.page, chatInput, "tc=second");
await chatInput.fill("tc=second"); await queueMessage(po.page, chatInput, "tc=third");
await chatInput.press("Enter");
await chatInput.fill("tc=third");
await chatInput.press("Enter");
// Verify 3 messages are queued // Verify 3 messages are queued
await expect(po.page.getByText("3 Queued")).toBeVisible(); await expect(po.page.getByText("3 Queued")).toBeVisible();
...@@ -120,8 +128,7 @@ test.describe("queued messages", () => { ...@@ -120,8 +128,7 @@ test.describe("queued messages", () => {
await expect(chatInput).toBeVisible(); await expect(chatInput).toBeVisible();
// While streaming, queue a second message // While streaming, queue a second message
await chatInput.fill("tc=2"); await queueMessage(po.page, chatInput, "tc=2");
await chatInput.press("Enter");
// Verify the queued message indicator is visible // Verify the queued message indicator is visible
await expect( await expect(
...@@ -193,8 +200,7 @@ testSkipIfWindows( ...@@ -193,8 +200,7 @@ testSkipIfWindows(
await expect(po.page.getByText("logo.png")).toBeVisible(); await expect(po.page.getByText("logo.png")).toBeVisible();
// Queue a message with both attachment and selected component // Queue a message with both attachment and selected component
await chatInput.fill("queued with extras"); await queueMessage(po.page, chatInput, "queued with extras");
await chatInput.press("Enter");
// After queuing, both should be cleared // After queuing, both should be cleared
await expect( await expect(
...@@ -264,8 +270,7 @@ testSkipIfWindows( ...@@ -264,8 +270,7 @@ testSkipIfWindows(
timeout: Timeout.SHORT, timeout: Timeout.SHORT,
}); });
await chatInput.fill("queued with component"); await queueMessage(po.page, chatInput, "queued with component");
await chatInput.press("Enter");
await expect(po.page.getByText(/\d+ Queued/)).toBeVisible(); await expect(po.page.getByText(/\d+ Queued/)).toBeVisible();
// Edit the queued message — components should be restored // Edit the queued message — components should be restored
......
import { expect } from "@playwright/test"; import { expect, type Page } from "@playwright/test";
import { testSkipIfWindows, Timeout } from "./helpers/test_helper"; import { testSkipIfWindows, Timeout } from "./helpers/test_helper";
const fs = require("fs"); const fs = require("fs");
const path = require("path"); const path = require("path");
async function saveVisualChanges(page: Page) {
await expect(async () => {
const saveButton = page.getByRole("button", { name: "Save Changes" });
await expect(saveButton).toBeVisible({ timeout: 1_000 });
await expect(saveButton).toBeEnabled({ timeout: 1_000 });
await saveButton.click();
}).toPass({ timeout: Timeout.MEDIUM });
}
testSkipIfWindows("edit style of one selected component", async ({ po }) => { testSkipIfWindows("edit style of one selected component", async ({ po }) => {
await po.setUpDyadPro(); await po.setUpDyadPro();
await po.sendPrompt("tc=basic"); await po.sendPrompt("tc=basic");
...@@ -57,7 +66,7 @@ testSkipIfWindows("edit style of one selected component", async ({ po }) => { ...@@ -57,7 +66,7 @@ testSkipIfWindows("edit style of one selected component", async ({ po }) => {
}); });
// Save the changes // Save the changes
await po.page.getByRole("button", { name: "Save Changes" }).click(); await saveVisualChanges(po.page);
// Wait for the success toast // Wait for the success toast
await po.toastNotifications.waitForToastWithText( await po.toastNotifications.waitForToastWithText(
...@@ -126,7 +135,7 @@ testSkipIfWindows("edit text of the selected component", async ({ po }) => { ...@@ -126,7 +135,7 @@ testSkipIfWindows("edit text of the selected component", async ({ po }) => {
}); });
// Save the changes // Save the changes
await po.page.getByRole("button", { name: "Save Changes" }).click(); await saveVisualChanges(po.page);
// Wait for the success toast // Wait for the success toast
await po.toastNotifications.waitForToastWithText( await po.toastNotifications.waitForToastWithText(
...@@ -200,7 +209,7 @@ testSkipIfWindows("swap image via URL", async ({ po }) => { ...@@ -200,7 +209,7 @@ testSkipIfWindows("swap image via URL", async ({ po }) => {
}); });
// Save the changes // Save the changes
await po.page.getByRole("button", { name: "Save Changes" }).click(); await saveVisualChanges(po.page);
// Wait for the success toast // Wait for the success toast
await po.toastNotifications.waitForToastWithText( await po.toastNotifications.waitForToastWithText(
......
...@@ -123,8 +123,10 @@ If `npm run build` fails while rebuilding native modules with `ImportError` from ...@@ -123,8 +123,10 @@ If `npm run build` fails while rebuilding native modules with `ImportError` from
- **Keyboard navigation events (ArrowUp/ArrowDown)**: Add `await page.waitForTimeout(100)` between sequential keyboard presses to let the UI state settle. Rapid keypresses can cause race conditions in menu navigation. - **Keyboard navigation events (ArrowUp/ArrowDown)**: Add `await page.waitForTimeout(100)` between sequential keyboard presses to let the UI state settle. Rapid keypresses can cause race conditions in menu navigation.
- **Navigation to tabs**: Use `await expect(link).toBeVisible({ timeout: Timeout.EXTRA_LONG })` before clicking tab links (especially in `goToAppsTab()`). Electron sidebar links can take time to render during app initialization. - **Navigation to tabs**: Use `await expect(link).toBeVisible({ timeout: Timeout.EXTRA_LONG })` before clicking tab links (especially in `goToAppsTab()`). Electron sidebar links can take time to render during app initialization.
- **Confirming flakiness**: Use `PLAYWRIGHT_RETRIES=0 PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/<spec> --repeat-each=10` to reproduce flaky tests. `PLAYWRIGHT_RETRIES=0` is critical — CI defaults to 2 retries, hiding flakiness. - **Confirming flakiness**: Use `PLAYWRIGHT_RETRIES=0 PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/<spec> --repeat-each=10` to reproduce flaky tests. `PLAYWRIGHT_RETRIES=0` is critical — CI defaults to 2 retries, hiding flakiness.
- **`expect(...).toPass()` wrappers**: Give inner Playwright actions/assertions short explicit timeouts. Default 30s click/expect timeouts can consume the whole `toPass()` budget, so the retry wrapper never actually retries.
- **Monaco file-switch assertions**: For code-editor tests, don't stop at waiting for the editor textbox to appear. Wait until Monaco's active model URI matches the file you clicked; otherwise the test can type into a still-switching editor model and miss real file-switch races. - **Monaco file-switch assertions**: For code-editor tests, don't stop at waiting for the editor textbox to appear. Wait until Monaco's active model URI matches the file you clicked; otherwise the test can type into a still-switching editor model and miss real file-switch races.
- **Monaco race repros**: If a file-editor bug only appears during quick tab/file changes, alternate between the affected files several times in one test before declaring it non-reproducible. A single switch often misses save-vs-switch timing bugs that show up immediately under `--repeat-each`. - **Monaco race repros**: If a file-editor bug only appears during quick tab/file changes, alternate between the affected files several times in one test before declaring it non-reproducible. A single switch often misses save-vs-switch timing bugs that show up immediately under `--repeat-each`.
- **GitHub sync success assertions**: Scope "Successfully pushed to GitHub!" assertions to `getByTestId("github-connected-repo")`; the same text can also appear in a toast, causing Playwright strict-mode failures.
## Real Socket Firewall E2E tests ## Real Socket Firewall E2E tests
......
...@@ -26,7 +26,7 @@ import { useTranslation } from "react-i18next"; ...@@ -26,7 +26,7 @@ import { useTranslation } from "react-i18next";
import { useSettings } from "@/hooks/useSettings"; import { useSettings } from "@/hooks/useSettings";
import { ipc } from "@/ipc/types"; import { ipc } from "@/ipc/types";
import { import {
chatInputValueAtom, chatInputValuesByIdAtom,
chatMessagesByIdAtom, chatMessagesByIdAtom,
selectedChatIdAtom, selectedChatIdAtom,
pendingAgentConsentsAtom, pendingAgentConsentsAtom,
...@@ -113,7 +113,23 @@ const showTokenBarAtom = atom(false); ...@@ -113,7 +113,23 @@ const showTokenBarAtom = atom(false);
export function ChatInput({ chatId }: { chatId?: number }) { export function ChatInput({ chatId }: { chatId?: number }) {
const { t } = useTranslation("chat"); const { t } = useTranslation("chat");
const posthog = usePostHog(); const posthog = usePostHog();
const [inputValue, setInputValue] = useAtom(chatInputValueAtom); const inputValuesById = useAtomValue(chatInputValuesByIdAtom);
const setInputValuesById = useSetAtom(chatInputValuesByIdAtom);
const inputValue = chatId ? (inputValuesById.get(chatId) ?? "") : "";
const setInputValue = useCallback(
(newValue: string | ((prev: string) => string)) => {
if (!chatId) return;
setInputValuesById((currentMap) => {
const prev = currentMap.get(chatId) ?? "";
const next = typeof newValue === "function" ? newValue(prev) : newValue;
const newMap = new Map(currentMap);
newMap.set(chatId, next);
return newMap;
});
},
[chatId, setInputValuesById],
);
const { settings } = useSettings(); const { settings } = useSettings();
const { const {
selectedMode: chatMode, selectedMode: chatMode,
......
...@@ -1131,7 +1131,7 @@ This conversation includes one or more image attachments. When the user uploads ...@@ -1131,7 +1131,7 @@ This conversation includes one or more image attachments. When the user uploads
system: systemPromptOverride, system: systemPromptOverride,
tools, tools,
messages: chatMessages.filter((m) => m.content), messages: chatMessages.filter((m) => m.content),
onFinish: (response) => { onFinish: async (response) => {
const totalTokens = response.usage?.totalTokens; const totalTokens = response.usage?.totalTokens;
if (typeof totalTokens === "number") { if (typeof totalTokens === "number") {
...@@ -1140,7 +1140,7 @@ This conversation includes one or more image attachments. When the user uploads ...@@ -1140,7 +1140,7 @@ This conversation includes one or more image attachments. When the user uploads
maxTokensUsed = Math.max(maxTokensUsed ?? 0, totalTokens); maxTokensUsed = Math.max(maxTokensUsed ?? 0, totalTokens);
// Persist the aggregated token usage on the placeholder assistant message // Persist the aggregated token usage on the placeholder assistant message
void db await db
.update(messages) .update(messages)
.set({ maxTokensUsed: maxTokensUsed }) .set({ maxTokensUsed: maxTokensUsed })
.where(eq(messages.id, placeholderAssistantMessage.id)) .where(eq(messages.id, placeholderAssistantMessage.id))
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论