mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): deflake TestChatMessageWithFiles/FileCapExceeded (#28091)
Fixes the flake tracked in [CODAGT-926](https://linear.app/codercom/issue/CODAGT-926/flake-testchatmessagewithfilesfilecapexceeded). ## Problem `TestChatMessageWithFiles/FileCapExceeded` asserted the rollback of a rejected over-cap send by comparing message counts taken before and after the send. `CreateChat` starts assistant generation asynchronously, so the assistant reply can be persisted between the two reads, making the count check fail even though the rejected message was correctly rolled back ("should have 1 item(s), but has 2"). ## Fix Replace the count comparison with a semantic assertion that the rejected `one too many` message was not persisted, hardened through Codex review rounds: - Scan message history for the rejected marker instead of comparing counts. - Also scan `QueuedMessages`: a busy chat queues the send before file-link validation, so a rollback regression could leave the rejected message queued rather than in history. - Close the queue-promotion race: `getChatMessages` reads history and the queue in two separate database reads, so the assertion first waits for the queue to observe empty; a promoted message must then appear in a fresh history read. ## Verification - Deterministic repro of the exact CI failure signature: waiting for the async assistant reply before the old count assertion reproduced `should have 1 item(s), but has 2` every run. - The new assertion passes under that same forced condition. - Assertion liveness (all temporary red checks reverted): persisting the marker in history fails the history scan; queuing the marker fails the queued scan; queuing the marker and letting it promote fails the post-drain history scan 3/3. - `go test ./coderd -run 'TestChatMessageWithFiles/FileCapExceeded' -count=100` and the full `TestChatMessageWithFiles` parent both pass. > Mux acted on Mike's behalf to create this PR.
This commit is contained in:
@@ -8299,8 +8299,6 @@ func TestChatMessageWithFiles(t *testing.T) {
|
||||
extraResp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "one-too-many.png", bytes.NewReader(pngData))
|
||||
require.NoError(t, err)
|
||||
|
||||
messagesBefore, err := client.GetChatMessages(ctx, chat.ID, nil)
|
||||
require.NoError(t, err)
|
||||
_, err = client.CreateChatMessage(ctx, chat.ID, codersdk.CreateChatMessageRequest{
|
||||
Content: []codersdk.ChatInputPart{
|
||||
{Type: codersdk.ChatInputPartTypeText, Text: "one too many"},
|
||||
@@ -8313,9 +8311,27 @@ func TestChatMessageWithFiles(t *testing.T) {
|
||||
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
|
||||
require.Contains(t, sdkErr.Message, "attachment limit")
|
||||
|
||||
messagesAfter, err := client.GetChatMessages(ctx, chat.ID, nil)
|
||||
// getChatMessages reads history before queued messages, so a promotion
|
||||
// can make one response miss the message in both places. Wait for the
|
||||
// queue to empty, then read history again because promotion inserts a
|
||||
// history row.
|
||||
require.Eventually(t, func() bool {
|
||||
m, err := client.GetChatMessages(ctx, chat.ID, nil)
|
||||
return err == nil && len(m.QueuedMessages) == 0
|
||||
}, testutil.WaitLong, testutil.IntervalMedium)
|
||||
|
||||
messages, err := client.GetChatMessages(ctx, chat.ID, nil)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, messagesAfter.Messages, len(messagesBefore.Messages), "rejected send should not persist a message")
|
||||
for _, msg := range messages.Messages {
|
||||
for _, part := range msg.Content {
|
||||
require.NotContains(t, part.Text, "one too many", "rejected send should not persist a message")
|
||||
}
|
||||
}
|
||||
for _, queued := range messages.QueuedMessages {
|
||||
for _, part := range queued.Content {
|
||||
require.NotContains(t, part.Text, "one too many", "rejected send should not queue a message")
|
||||
}
|
||||
}
|
||||
chatResult, err := client.GetChat(ctx, chat.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, chatResult.Files, codersdk.MaxChatFileIDs,
|
||||
|
||||
Reference in New Issue
Block a user