Skip to content

test: re-read instead of trusting the partial-update write response - #149

Merged
mogita merged 1 commit into
mainfrom
test/partial-update-reread
Aug 27, 2026
Merged

mogita merged 1 commit into
mainfrom
test/partial-update-reread

Conversation

@mogita

@mogita mogita commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

TestChatChannelIntegration/PartialUpdateChannel is the longest-running flake in this suite. It failed on main in the runs for #137 and #142, the unset half was later fixed, and it now fails on the set half instead:

chat_channel_integration_test.go:129
expected: string("red")
actual  : <nil>(<nil>)

The assertion reads Custom["color"] straight off the UpdateChannelPartial response, and the server can hydrate that response from a read replica that has not observed the write yet.

Solution

Assert the value by re-reading with require.Eventually, which is exactly what the unset half of the same test already does, and for the same stated reason. Both halves are now symmetric and neither depends on read-after-write.

No coverage is lost. The value assertion still catches a Set map serialized under the wrong key or a Custom map that fails to deserialize, which is the bug this test exists for, and the write response and GetOrCreate deserialize the same ChannelResponse type. Only the assumption that the write response is immediately consistent is gone.

How to verify

  1. go test -run 'TestChatChannelIntegration/PartialUpdateChannel' . passes. I ran it 10 times in a row against the integration app: 10 pass, 0 fail.
  2. A real regression still fails: break the Set key or the Custom deserialization and the Eventually never satisfies, so the test fails after 5s with color should be set to red.

PartialUpdateChannel asserted the custom field straight off the UpdateChannelPartial
response, which the server can hydrate from a read replica that has not observed
the write. That made it the longest-running flake in this suite: it failed on
main in the runs for #137 and #142, and again after the unset half was fixed,
this time on the set half instead.

The unset half already re-reads with require.Eventually for exactly this reason.
The set half now does the same, so both halves are symmetric and neither depends
on read-after-write.

No coverage is lost. Asserting the value still catches a Set map serialized under
the wrong key or a Custom map that fails to deserialize, which is the bug this
test exists for, and the write response and GetOrCreate deserialize the same
ChannelResponse type. Only the assumption that the write response is immediately
consistent is gone.
@mogita
mogita requested a review from tbarbugli as a code owner August 27, 2026 12:49
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita temporarily deployed to feeds-enabled-shard August 27, 2026 12:49 — with GitHub Actions Inactive
@mogita
mogita merged commit d2e699e into main Aug 27, 2026
12 checks passed
@mogita
mogita deleted the test/partial-update-reread branch August 27, 2026 13:06

This branch was previously deployed

1 inactive deployment
feeds-enabled-shard — 96eee1bb Deployed Aug 27, 2026 by mogita via 👷 Test & Build 1.20 #501
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant