fix: improve test infrastructure based on PR review

- Fix double build in CI: remove redundant build from playwright webServer
- Export chat helpers from shared module for proper unit testing
- Replace waitForTimeout with explicit waits in E2E tests
- Add data-testid attributes to settings and new chat buttons
- Add list reporter for CI to show failures in logs
- Add Playwright browser caching to speed up CI
- Add vitest coverage configuration
- Fix conditional test assertions to use test.skip() instead of silent pass
- Remove unused variables flagged by linter
This commit is contained in:
dayuan.jiang
2026-01-04 22:25:50 +09:00
parent d1e5fc1440
commit d517268dbe
13 changed files with 205 additions and 272 deletions

View File

@@ -43,9 +43,21 @@ jobs:
- name: Install dependencies
run: npm ci
- name: Cache Playwright browsers
uses: actions/cache@v4
id: playwright-cache
with:
path: ~/.cache/ms-playwright
key: playwright-${{ runner.os }}-${{ hashFiles('**/package-lock.json') }}
- name: Install Playwright browsers
if: steps.playwright-cache.outputs.cache-hit != 'true'
run: npx playwright install chromium --with-deps
- name: Install Playwright deps (cached)
if: steps.playwright-cache.outputs.cache-hit == 'true'
run: npx playwright install-deps chromium
- name: Build app
run: npm run build

View File

@@ -18,6 +18,11 @@ import {
supportsPromptCaching,
} from "@/lib/ai-providers"
import { findCachedResponse } from "@/lib/cached-responses"
import {
isMinimalDiagram,
replaceHistoricalToolInputs,
validateFileParts,
} from "@/lib/chat-helpers"
import {
checkAndIncrementRequest,
isQuotaEnabled,
@@ -34,93 +39,6 @@ import { getUserIdFromRequest } from "@/lib/user-id"
export const maxDuration = 120
// File upload limits (must match client-side)
const MAX_FILE_SIZE = 2 * 1024 * 1024 // 2MB
const MAX_FILES = 5
// Helper function to validate file parts in messages
function validateFileParts(messages: any[]): {
valid: boolean
error?: string
} {
const lastMessage = messages[messages.length - 1]
const fileParts =
lastMessage?.parts?.filter((p: any) => p.type === "file") || []
if (fileParts.length > MAX_FILES) {
return {
valid: false,
error: `Too many files. Maximum ${MAX_FILES} allowed.`,
}
}
for (const filePart of fileParts) {
// Data URLs format: data:image/png;base64,<data>
// Base64 increases size by ~33%, so we check the decoded size
if (filePart.url?.startsWith("data:")) {
const base64Data = filePart.url.split(",")[1]
if (base64Data) {
const sizeInBytes = Math.ceil((base64Data.length * 3) / 4)
if (sizeInBytes > MAX_FILE_SIZE) {
return {
valid: false,
error: `File exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit.`,
}
}
}
}
}
return { valid: true }
}
// Helper function to check if diagram is minimal/empty
function isMinimalDiagram(xml: string): boolean {
const stripped = xml.replace(/\s/g, "")
return !stripped.includes('id="2"')
}
// Helper function to replace historical tool call XML with placeholders
// This reduces token usage and forces LLM to rely on the current diagram XML (source of truth)
// Also fixes invalid/undefined inputs from interrupted streaming
function replaceHistoricalToolInputs(messages: any[]): any[] {
return messages.map((msg) => {
if (msg.role !== "assistant" || !Array.isArray(msg.content)) {
return msg
}
const replacedContent = msg.content
.map((part: any) => {
if (part.type === "tool-call") {
const toolName = part.toolName
// Fix invalid/undefined inputs from interrupted streaming
if (
!part.input ||
typeof part.input !== "object" ||
Object.keys(part.input).length === 0
) {
// Skip tool calls with invalid inputs entirely
return null
}
if (
toolName === "display_diagram" ||
toolName === "edit_diagram"
) {
return {
...part,
input: {
placeholder:
"[XML content replaced - see current diagram XML in system context]",
},
}
}
}
return part
})
.filter(Boolean) // Remove null entries (invalid tool calls)
return { ...msg, content: replacedContent }
})
}
// Helper function to create cached stream response
function createCachedStreamResponse(xml: string): Response {
const toolCallId = `cached-${Date.now()}`

View File

@@ -1185,6 +1185,7 @@ export default function ChatPanel({
status === "streaming" || status === "submitted"
}
className="hover:bg-accent disabled:opacity-50 disabled:cursor-not-allowed"
data-testid="new-chat-button"
>
<MessageSquarePlus
className={`${isMobile ? "h-4 w-4" : "h-5 w-5"} text-muted-foreground`}
@@ -1197,6 +1198,7 @@ export default function ChatPanel({
size="icon"
onClick={() => setShowSettingsDialog(true)}
className="hover:bg-accent"
data-testid="settings-button"
>
<Settings
className={`${isMobile ? "h-4 w-4" : "h-5 w-5"} text-muted-foreground`}

89
lib/chat-helpers.ts Normal file
View File

@@ -0,0 +1,89 @@
// Shared helper functions for chat route
// Exported for testing
// File upload limits (must match client-side)
export const MAX_FILE_SIZE = 2 * 1024 * 1024 // 2MB
export const MAX_FILES = 5
// Helper function to validate file parts in messages
export function validateFileParts(messages: any[]): {
valid: boolean
error?: string
} {
const lastMessage = messages[messages.length - 1]
const fileParts =
lastMessage?.parts?.filter((p: any) => p.type === "file") || []
if (fileParts.length > MAX_FILES) {
return {
valid: false,
error: `Too many files. Maximum ${MAX_FILES} allowed.`,
}
}
for (const filePart of fileParts) {
// Data URLs format: data:image/png;base64,<data>
// Base64 increases size by ~33%, so we check the decoded size
if (filePart.url?.startsWith("data:")) {
const base64Data = filePart.url.split(",")[1]
if (base64Data) {
const sizeInBytes = Math.ceil((base64Data.length * 3) / 4)
if (sizeInBytes > MAX_FILE_SIZE) {
return {
valid: false,
error: `File exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit.`,
}
}
}
}
}
return { valid: true }
}
// Helper function to check if diagram is minimal/empty
export function isMinimalDiagram(xml: string): boolean {
const stripped = xml.replace(/\s/g, "")
return !stripped.includes('id="2"')
}
// Helper function to replace historical tool call XML with placeholders
// This reduces token usage and forces LLM to rely on the current diagram XML (source of truth)
// Also fixes invalid/undefined inputs from interrupted streaming
export function replaceHistoricalToolInputs(messages: any[]): any[] {
return messages.map((msg) => {
if (msg.role !== "assistant" || !Array.isArray(msg.content)) {
return msg
}
const replacedContent = msg.content
.map((part: any) => {
if (part.type === "tool-call") {
const toolName = part.toolName
// Fix invalid/undefined inputs from interrupted streaming
if (
!part.input ||
typeof part.input !== "object" ||
Object.keys(part.input).length === 0
) {
// Skip tool calls with invalid inputs entirely
return null
}
if (
toolName === "display_diagram" ||
toolName === "edit_diagram"
) {
return {
...part,
input: {
placeholder:
"[XML content replaced - see current diagram XML in system context]",
},
}
}
}
return part
})
.filter(Boolean) // Remove null entries (invalid tool calls)
return { ...msg, content: replacedContent }
})
}

View File

@@ -6,11 +6,9 @@ export default defineConfig({
forbidOnly: !!process.env.CI,
retries: process.env.CI ? 2 : 0,
workers: process.env.CI ? 1 : undefined,
reporter: "html",
reporter: process.env.CI ? [["list"], ["html"]] : "html",
webServer: {
command: process.env.CI
? "npm run build && npm run start"
: "npm run dev",
command: process.env.CI ? "npm run start" : "npm run dev",
port: process.env.CI ? 6001 : 6002,
reuseExistingServer: !process.env.CI,
timeout: 120 * 1000,

View File

@@ -65,18 +65,18 @@ test.describe("Copy/Paste Functionality", () => {
// Find copy button in message
const copyButton = page.locator(
'button[aria-label*="Copy"], button:has(svg.lucide-copy), button:has(svg.lucide-clipboard)',
'[data-testid="copy-button"], button[aria-label*="Copy"], button:has(svg.lucide-copy), button:has(svg.lucide-clipboard)',
)
if ((await copyButton.count()) > 0) {
await copyButton.first().click()
// Should show copied confirmation (toast or button state change)
await expect(
page
.locator('text="Copied"')
.or(page.locator("svg.lucide-check")),
).toBeVisible({ timeout: 3000 })
}
// Skip test if copy button doesn't exist
const buttonCount = await copyButton.count()
test.skip(buttonCount === 0, "Copy button not available")
await copyButton.first().click()
// Should show copied confirmation (toast or button state change)
await expect(
page.locator('text="Copied"').or(page.locator("svg.lucide-check")),
).toBeVisible({ timeout: 3000 })
})
test("paste XML into XML input works", async ({ page }) => {
@@ -96,22 +96,20 @@ test.describe("Copy/Paste Functionality", () => {
)
if ((await xmlToggle.count()) > 0) {
await xmlToggle.first().click()
await page.waitForTimeout(500)
}
// Check if XML input is now visible
if (
(await xmlInput.count()) > 0 &&
(await xmlInput.first().isVisible())
) {
const testXml = `<mxCell id="pasted" value="Pasted Node" style="rounded=1;fillColor=#d5e8d4;" vertex="1" parent="1">
// Skip test if XML input feature doesn't exist
const xmlInputCount = await xmlInput.count()
const isXmlVisible =
xmlInputCount > 0 && (await xmlInput.first().isVisible())
test.skip(!isXmlVisible, "XML input feature not available")
const testXml = `<mxCell id="pasted" value="Pasted Node" style="rounded=1;fillColor=#d5e8d4;" vertex="1" parent="1">
<mxGeometry x="100" y="100" width="120" height="60" as="geometry"/>
</mxCell>`
await xmlInput.first().fill(testXml)
await expect(xmlInput.first()).toHaveValue(testXml)
}
// Test passes if XML input doesn't exist or isn't accessible
await xmlInput.first().fill(testXml)
await expect(xmlInput.first()).toHaveValue(testXml)
})
test("keyboard shortcuts work in chat input", async ({ page }) => {
@@ -154,9 +152,8 @@ test.describe("Copy/Paste Functionality", () => {
// Undo with Ctrl+Z
await chatInput.press("ControlOrMeta+z")
// Value might revert (depends on browser/implementation)
// At minimum, shouldn't crash
await page.waitForTimeout(500)
// Verify page is still functional after undo
await expect(chatInput).toBeVisible()
})
test("chat input handles special characters", async ({ page }) => {

View File

@@ -39,11 +39,11 @@ test.describe("File Upload", () => {
),
})
// Wait for file to be processed
await page.waitForTimeout(1000)
// File input should have processed the file - check any indicator
// This test passes if no error occurs during file selection
// File input should have processed the file
// Check that no error toast appeared
await expect(
page.locator('[role="alert"][data-type="error"]'),
).not.toBeVisible({ timeout: 2000 })
})
test("can remove uploaded file", async ({ page }) => {
@@ -64,18 +64,21 @@ test.describe("File Upload", () => {
),
})
// Wait for file to be processed
await page.waitForTimeout(1000)
// Wait for file preview or no error
await expect(
page.locator('[role="alert"][data-type="error"]'),
).not.toBeVisible({ timeout: 2000 })
// Find and click remove button if it exists (X icon)
const removeButton = page.locator(
'button[aria-label*="Remove"], button:has(svg.lucide-x)',
'[data-testid="remove-file-button"], button[aria-label*="Remove"], button:has(svg.lucide-x)',
)
if ((await removeButton.count()) > 0) {
await removeButton.first().click()
await page.waitForTimeout(500)
}
// Test passes if no errors during upload and potential removal
const removeButtonCount = await removeButton.count()
test.skip(removeButtonCount === 0, "Remove file button not available")
await removeButton.first().click()
// Verify button is gone or file preview is removed
await expect(removeButton.first()).not.toBeVisible({ timeout: 2000 })
})
test("sends file with message to API", async ({ page }) => {
@@ -170,8 +173,9 @@ test.describe("File Upload", () => {
await chatForm.dispatchEvent("dragover", { dataTransfer })
await chatForm.dispatchEvent("drop", { dataTransfer })
// Should show file preview (or at least not crash)
// File might be rejected due to not being a real image, but UI should handle it
await page.waitForTimeout(1000) // Give UI time to process
// Should not crash - verify page is still functional
await expect(
page.locator('textarea[aria-label="Chat input"]'),
).toBeVisible({ timeout: 3000 })
})
})

View File

@@ -35,17 +35,19 @@ test.describe("History and Session Restore", () => {
// Find and click new chat button
const newChatButton = page.locator(
'button[aria-label*="New"], button:has(svg.lucide-plus), button:has-text("New Chat")',
'[data-testid="new-chat-button"], button[aria-label*="New"], button:has(svg.lucide-plus), button:has-text("New Chat")',
)
if ((await newChatButton.count()) > 0) {
await newChatButton.first().click()
// Skip test if new chat button doesn't exist
const buttonCount = await newChatButton.count()
test.skip(buttonCount === 0, "New chat button not available")
// Conversation should be cleared
await expect(
page.locator('text="Created your test diagram."'),
).not.toBeVisible({ timeout: 5000 })
}
await newChatButton.first().click()
// Conversation should be cleared
await expect(
page.locator('text="Created your test diagram."'),
).not.toBeVisible({ timeout: 5000 })
})
test("chat history sidebar shows past conversations", async ({ page }) => {
@@ -59,17 +61,19 @@ test.describe("History and Session Restore", () => {
'button[aria-label*="History"]:not([disabled]), button:has(svg.lucide-history):not([disabled]), button:has(svg.lucide-menu):not([disabled]), button:has(svg.lucide-sidebar):not([disabled]), button:has(svg.lucide-panel-left):not([disabled])',
)
if ((await historyButton.count()) > 0) {
await historyButton.first().click()
await page.waitForTimeout(500)
}
// Test passes if no error - history feature may or may not be available
// Skip test if history button doesn't exist
const buttonCount = await historyButton.count()
test.skip(buttonCount === 0, "History button not available")
await historyButton.first().click()
// Wait for sidebar/panel to appear or verify page still works
await expect(
page.locator('textarea[aria-label="Chat input"]'),
).toBeVisible({ timeout: 3000 })
})
test("conversation persists after page reload", async ({ page }) => {
let requestCount = 0
await page.route("**/api/chat", async (route) => {
requestCount++
await route.fulfill({
status: 200,
contentType: "text/event-stream",
@@ -103,9 +107,10 @@ test.describe("History and Session Restore", () => {
.locator("iframe")
.waitFor({ state: "visible", timeout: 30000 })
// Check if conversation persisted (depends on implementation)
// The message might or might not be there depending on local storage usage
await page.waitForTimeout(1000)
// Verify page is functional after reload
await expect(
page.locator('textarea[aria-label="Chat input"]'),
).toBeVisible({ timeout: 10000 })
})
test("diagram state persists after reload", async ({ page }) => {
@@ -136,9 +141,6 @@ test.describe("History and Session Restore", () => {
timeout: 15000,
})
// Wait for diagram to render
await page.waitForTimeout(1000)
// Reload
await page.reload({ waitUntil: "networkidle" })
await page
@@ -204,13 +206,10 @@ test.describe("History and Session Restore", () => {
.waitFor({ state: "visible", timeout: 30000 })
// Open settings
const settingsButton = page.locator(
'button[aria-label*="Settings"], button:has(svg.lucide-settings)',
)
await settingsButton.first().click()
const settingsButton = page.locator('[data-testid="settings-button"]')
await settingsButton.click()
// Settings dialog should open
await page.waitForTimeout(500)
await expect(
page.locator('[role="dialog"], [role="menu"], form').first(),
).toBeVisible({ timeout: 5000 })
@@ -225,10 +224,9 @@ test.describe("History and Session Restore", () => {
.waitFor({ state: "visible", timeout: 30000 })
// Open settings again
await settingsButton.first().click()
await settingsButton.click()
// Settings should still be accessible
await page.waitForTimeout(500)
await expect(
page.locator('[role="dialog"], [role="menu"], form').first(),
).toBeVisible({ timeout: 5000 })
@@ -245,19 +243,21 @@ test.describe("History and Session Restore", () => {
'button[aria-label*="Model"], [data-testid="model-selector"], button:has-text("Claude")',
)
if ((await modelSelector.count()) > 0) {
const initialModel = await modelSelector.first().textContent()
// Skip test if model selector doesn't exist
const selectorCount = await modelSelector.count()
test.skip(selectorCount === 0, "Model selector not available")
// Reload page
await page.reload({ waitUntil: "networkidle" })
await page
.locator("iframe")
.waitFor({ state: "visible", timeout: 30000 })
const initialModel = await modelSelector.first().textContent()
// Check model is still selected
const modelAfterReload = await modelSelector.first().textContent()
expect(modelAfterReload).toBe(initialModel)
}
// Reload page
await page.reload({ waitUntil: "networkidle" })
await page
.locator("iframe")
.waitFor({ state: "visible", timeout: 30000 })
// Check model is still selected
const modelAfterReload = await modelSelector.first().textContent()
expect(modelAfterReload).toBe(initialModel)
})
test("handles localStorage quota exceeded gracefully", async ({ page }) => {

View File

@@ -9,9 +9,7 @@ test.describe("Settings", () => {
})
test("settings dialog opens", async ({ page }) => {
const settingsButton = page.locator(
'button[aria-label*="settings"], button:has(svg[class*="settings"])',
)
const settingsButton = page.locator('[data-testid="settings-button"]')
await expect(settingsButton).toBeVisible()
await settingsButton.click()
@@ -20,9 +18,7 @@ test.describe("Settings", () => {
})
test("language selection is available", async ({ page }) => {
const settingsButton = page.locator(
'button[aria-label*="settings"], button:has(svg[class*="settings"])',
)
const settingsButton = page.locator('[data-testid="settings-button"]')
await settingsButton.click()
const dialog = page.locator('[role="dialog"]')
@@ -33,9 +29,7 @@ test.describe("Settings", () => {
})
test("draw.io theme toggle exists", async ({ page }) => {
const settingsButton = page.locator(
'button[aria-label*="settings"], button:has(svg[class*="settings"])',
)
const settingsButton = page.locator('[data-testid="settings-button"]')
await settingsButton.click()
const dialog = page.locator('[role="dialog"]')

View File

@@ -37,9 +37,7 @@ test.describe("Smoke Tests", () => {
.waitFor({ state: "visible", timeout: 30000 })
// Click settings button (gear icon)
const settingsButton = page.locator(
'button[aria-label*="settings"], button:has(svg[class*="settings"])',
)
const settingsButton = page.locator('[data-testid="settings-button"]')
await expect(settingsButton).toBeVisible()
await settingsButton.click()

View File

@@ -87,16 +87,6 @@ test.describe("Theme Switching", () => {
)
await settingsButton.first().click()
// Settings dialog should be visible
await page.waitForTimeout(500)
// Find any theme-related section - various possible labels
const themeSection = page
.locator('text="Draw.io Theme"')
.or(page.locator('text="draw.io"'))
.or(page.locator('text="Theme"'))
.or(page.locator('[aria-label*="theme"]'))
// At least some settings content should be visible
await expect(
page.locator('[role="dialog"], [role="menu"], form').first(),

View File

@@ -1,85 +1,10 @@
// @vitest-environment node
import { describe, expect, it } from "vitest"
// Test the helper functions from chat route
// These are re-implemented here for testing since they're not exported
const MAX_FILE_SIZE = 2 * 1024 * 1024 // 2MB
const MAX_FILES = 5
function validateFileParts(messages: any[]): {
valid: boolean
error?: string
} {
const lastMessage = messages[messages.length - 1]
const fileParts =
lastMessage?.parts?.filter((p: any) => p.type === "file") || []
if (fileParts.length > MAX_FILES) {
return {
valid: false,
error: `Too many files. Maximum ${MAX_FILES} allowed.`,
}
}
for (const filePart of fileParts) {
if (filePart.url?.startsWith("data:")) {
const base64Data = filePart.url.split(",")[1]
if (base64Data) {
const sizeInBytes = Math.ceil((base64Data.length * 3) / 4)
if (sizeInBytes > MAX_FILE_SIZE) {
return {
valid: false,
error: `File exceeds ${MAX_FILE_SIZE / 1024 / 1024}MB limit.`,
}
}
}
}
}
return { valid: true }
}
function isMinimalDiagram(xml: string): boolean {
const stripped = xml.replace(/\s/g, "")
return !stripped.includes('id="2"')
}
function replaceHistoricalToolInputs(messages: any[]): any[] {
return messages.map((msg) => {
if (msg.role !== "assistant" || !Array.isArray(msg.content)) {
return msg
}
const replacedContent = msg.content
.map((part: any) => {
if (part.type === "tool-call") {
const toolName = part.toolName
if (
!part.input ||
typeof part.input !== "object" ||
Object.keys(part.input).length === 0
) {
return null
}
if (
toolName === "display_diagram" ||
toolName === "edit_diagram"
) {
return {
...part,
input: {
placeholder:
"[XML content replaced - see current diagram XML in system context]",
},
}
}
}
return part
})
.filter(Boolean)
return { ...msg, content: replacedContent }
})
}
import {
isMinimalDiagram,
replaceHistoricalToolInputs,
validateFileParts,
} from "@/lib/chat-helpers"
describe("validateFileParts", () => {
it("returns valid for no files", () => {

View File

@@ -7,5 +7,11 @@ export default defineConfig({
test: {
environment: "jsdom",
include: ["tests/**/*.test.{ts,tsx}"],
coverage: {
provider: "v8",
reporter: ["text", "json", "html"],
include: ["lib/**/*.ts", "app/**/*.ts", "app/**/*.tsx"],
exclude: ["**/*.test.ts", "**/*.test.tsx", "**/*.d.ts"],
},
},
})