Repository navigation
test(coderd): share one server across the chat message pagination tests - #30965
ibetitsmike wants to merge 2 commits into
Conversation
TestGetChatMessages_Pagination started a coderd server with its own database in each of its ten leaves. Build the server, owner, and chat model once in the parent, merge the six successful-page leaves and the two malformed after_id leaves into two tables, and keep the transposed cursor and burst walk subtests. Each leaf still seeds its own chat.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review
|
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
TestGetChatMessages_Paginationstarted a coderd server with its own database in each of its ten leaves, although the leaves differ only in cursors and their own chat rows. This PR builds the client, database, owner, and chat model once in the parent. The six successful-page leaves become one table (options, expected ID positions,HasMore, queued count), and the two malformedafter_idleaves become another. The transposed or equal cursor case and the burst walk stay separate subtests, and every subtest keeps its name.Why it is safe: every leaf still creates its own chat, messages, and queued message, and every request and query is scoped to that chat. No leaf changes users, roles, models, or deployment values. I ran 24 mutants of
getChatMessagesand its two paginated queries (limit+1 andHasMore, truncation, ASC dispatch, queued suppression,after_idparsing, the transposed check, cursor comparison, and ordering) against the old and the new test withgo test -overlay. The new test fails on all 20 mutants that the old test fails on, in the same leaves. It also fails on the two mutants that drop thechat_idfilter, which the old test missed because each leaf had its own database. Two mutants survive both tests: the DESC path fetchinglimitrows instead oflimit+1, andHasMoreusing>=. No test onmaincovers them, and this PR does not add coverage.Review focus: the shared database interleaves message IDs across leaves. Assertions use only the IDs that each leaf's
seedChatreturned, so interleaving cannot change a result for a correct handler.AfterIDAtOrAboveMaxReturnsEmptynow seeds five messages instead of three, like the other page cases, and still setsafter_idto the last ID.Timing (prebuilt test binaries,
-test.parallel 4, median of 5 alternating runs on a shared host): binary wall time went from 1.21 s to 0.72 s. Leaf times summed went from 3.89 s to 0.39 s, plus 0.48 s for the one shared server, and coderd boots went from 10 to 1.Validation:
go test ./coderd -run '^TestGetChatMessages_Pagination$' -count=10: pass-race -count=2: passgo vet ./coderdandgolangci-lint run ./coderd/...: clean; the pre-commit hook passed