Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions testing/e2e/src/components/ChatUI.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function ChatUI({
}: ChatUIProps) {
const [input, setInput] = useState('')
const messagesRef = useRef<HTMLDivElement>(null)
const inputRef = useRef<HTMLInputElement>(null)

useEffect(() => {
if (messagesRef.current) {
Expand Down Expand Up @@ -203,14 +204,20 @@ export function ChatUI({
className="text-xs text-gray-400"
onChange={(e) => {
const file = e.target.files?.[0]
if (file && input.trim() && onSendMessageWithImage) {
onSendMessageWithImage(input.trim(), file)
// Read the prompt from the live input DOM value rather than the
// `input` React state. Attaching a file auto-sends, and under
// load a controlled input's state can lag the committed DOM
// value β€” reading state here would send an empty/partial prompt.
const text = (inputRef.current?.value ?? input).trim()
if (file && text && onSendMessageWithImage) {
onSendMessageWithImage(text, file)
setInput('')
}
}}
/>
)}
<input
ref={inputRef}
data-testid="chat-input"
type="text"
value={input}
Expand Down
30 changes: 25 additions & 5 deletions testing/e2e/tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,31 @@ export async function sendMessageWithImage(
imagePath: string,
) {
const input = page.getByTestId('chat-input')
await input.click()
await input.pressSequentially(text, { delay: 30 })
// Wait for React state to settle before attaching file
await page.waitForTimeout(200)
await page.getByTestId('image-attachment-input').setInputFiles(imagePath)
const fileInput = page.getByTestId('image-attachment-input')
const userMessages = page.getByTestId('user-message')

// Attaching the image auto-sends, using the prompt currently in the chat
// input, and the matched aimock fixture keys on the exact user text. A
// *controlled* React input is fragile here under CPU load (CI, parallel
// workers) in two ways: typing char-by-char can drop characters, leaving a
// truncated value like "cribe this image" (which 404s as "No fixture
// matched" β†’ empty `chatStream fatal`); and the attach's onChange can land
// before the typed value is committed, dispatching nothing at all. So drive
// the interaction to its observable outcome β€” the user bubble rendering β€”
// retrying both the typing and the attach until the send actually fires with
// the full prompt. A redundant re-attach is harmless: the client ignores a
// second send while the first is still streaming.
await expect(async () => {
await input.click()
await input.fill('')
await input.pressSequentially(text, { delay: 15 })
// Confirm the full prompt is committed before attaching.
expect(await input.inputValue()).toBe(text)
// Reset the selection so re-attaching the same path re-fires onChange.
await fileInput.setInputFiles([])
await fileInput.setInputFiles(imagePath)
await expect(userMessages.first()).toBeVisible({ timeout: 2_000 })
}).toPass({ timeout: 15_000, intervals: [250, 500, 1000] })
Comment on lines +41 to +64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚑ Quick win

Use message-count delta instead of .first() visibility to confirm the send actually fired.

At Line 63, userMessages.first() may already be visible from earlier messages, so this check can pass even when the new image send didn’t occur. Track a baseline count and assert it increases after attach.

Suggested fix
 export async function sendMessageWithImage(
   page: Page,
   text: string,
   imagePath: string,
 ) {
   const input = page.getByTestId('chat-input')
   const fileInput = page.getByTestId('image-attachment-input')
   const userMessages = page.getByTestId('user-message')
+  const baselineUserMessageCount = await userMessages.count()

   await expect(async () => {
     await input.click()
     await input.fill('')
     await input.pressSequentially(text, { delay: 15 })
     // Confirm the full prompt is committed before attaching.
     expect(await input.inputValue()).toBe(text)
     // Reset the selection so re-attaching the same path re-fires onChange.
     await fileInput.setInputFiles([])
     await fileInput.setInputFiles(imagePath)
-    await expect(userMessages.first()).toBeVisible({ timeout: 2_000 })
+    await expect
+      .poll(async () => await userMessages.count(), { timeout: 2_000 })
+      .toBeGreaterThan(baselineUserMessageCount)
   }).toPass({ timeout: 15_000, intervals: [250, 500, 1000] })
 }
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testing/e2e/tests/helpers.ts` around lines 41 - 64, The test assertion using
await expect(userMessages.first()).toBeVisible() does not confirm that a new
message was actually sent because the first user message may already be visible
from earlier in the conversation. Instead, capture the initial count of user
messages before attaching the image, then after the attach, verify that the
count has increased by one. This ensures the assertion confirms a new message
was added rather than just checking visibility of an existing message. Store the
initial length before the fileInput.setInputFiles calls and add a subsequent
assertion that the user message count increased.

}

export async function waitForResponse(page: Page, timeout = 15_000) {
Expand Down
56 changes: 40 additions & 16 deletions testing/e2e/tests/tools-test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,47 @@ export async function runTest(page: Page): Promise<void> {
for (let attempt = 0; attempt < 5; attempt++) {
const baselineMessageCount = await readMessageCount()
await page.click('#run-test-button')
await page.waitForTimeout(300)

const started = await page.evaluate((baseline) => {
const metadata = document.getElementById('test-metadata')
if (metadata?.getAttribute('data-is-loading') === 'true') {
return true
}

const text =
document.getElementById('messages-json-content')?.textContent || '[]'
try {
const parsed = JSON.parse(text)
return Array.isArray(parsed) && parsed.length > baseline
} catch {
return false
}
}, baselineMessageCount)
// A run "starts" only when real stream activity appears β€” not when the
// optimistic user message lands. Clicking adds one user message
// synchronously (baseline + 1); that alone must NOT count as started, or a
// stalled run (the click registered but the stream produced nothing) would
// be reported as started and the test would later time out waiting for an
// approval / completion that never comes. Real activity is: loading turned
// on, a tool call appeared, the test completed, or a *second* message (the
// assistant response) was added beyond the optimistic user message. Poll
// briefly so a slow-but-real run under CI load isn't mistaken for a stall.
const started = await page
.waitForFunction(
(baseline) => {
const metadata = document.getElementById('test-metadata')
if (metadata?.getAttribute('data-is-loading') === 'true') return true
if (
parseInt(
metadata?.getAttribute('data-tool-call-count') || '0',
10,
) > 0
)
return true
if (metadata?.getAttribute('data-test-complete') === 'true')
return true
const text =
document.getElementById('messages-json-content')?.textContent ||
'[]'
try {
const parsed = JSON.parse(text)
// > baseline + 1: the assistant message arrived (a real response),
// not just the optimistic user message.
return Array.isArray(parsed) && parsed.length > baseline + 1
} catch {
return false
}
},
baselineMessageCount,
{ timeout: 2000 },
)
.then(() => true)
.catch(() => false)

if (started) {
return
Expand Down
Loading