Skip to content

Fix state not showing up fast enough with TestDelayedEvents (de-flake v2) #869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MadLittleMods wants to merge 7 commits into
base: main
Choose a base branch
from
+93 −12
Open

Fix state not showing up fast enough with TestDelayedEvents (de-flake v2) #869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3133b5a
Fix state not showing up fast enough with `TestDelayedEvents` (#865)
MadLittleMods Apr 28, 2026
b2bb349
Fix state lookup assertion
MadLittleMods Apr 30, 2026
3b19fda
Use `SyncStateHas` which better illustrates what we want to do
MadLittleMods Apr 30, 2026
f4be099
Clean up another test
MadLittleMods Apr 30, 2026
ebc611c
Update all state tests
MadLittleMods Apr 30, 2026
b4cb936
Add final fixme for the big blob
MadLittleMods Apr 30, 2026
90d94e8
Remove duplicate sleep
MadLittleMods May 5, 2026
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
9 changes: 5 additions & 4 deletions client/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,11 @@ func SyncTimelineHasEventID(roomID string, eventID string) SyncCheckOpt {
})
}

Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

@reivilibre assigning you since you have the context from #865

Same sort of fix but applied to all of tests here

// Check that the state section for `roomID` has an event which passes the check function.
// Note that the state section of a sync response only contains the change in state up to the start
// of the timeline and will not contain the entire state of the room for incremental or
// `lazy_load_members` syncs.
// Check that the `state` section for `roomID` has an event which passes the check function.
//
// Note that the `state` section of a sync response only contains the change in state up
// to the start of the `timeline` and will not contain the entire state of the room for
// incremental or `lazy_load_members` syncs.
func SyncStateHas(roomID string, check func(gjson.Result) bool) SyncCheckOpt {
Comment on lines +218 to 223
Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

This was the reason the previous PR didn't work, #865

Just re-formatted this a bit 🤷 (can revert if not useful)

return func(clientUserID string, topLevelSyncJSON gjson.Result) error {
err := checkArrayElements(
Expand Down
96 changes: 88 additions & 8 deletions tests/msc4140/delayed_event_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ const (
DelayedEventActionSend = "send"
)

// A filter for `/sync` that excludes timeline events.
//
// This is useful if you want to see `state` in the `/sync` response without the pesky
// de-duplication with `timeline` that traditional `/sync` does.
const NoTimelineSyncFilter = `{
"room": {
"timeline": { "limit": 0 }
}
}`

// TODO: Test pagination of `GET /_matrix/client/v1/delayed_events` once
// it is implemented in a homeserver.

Expand Down Expand Up @@ -62,6 +72,7 @@ func TestDelayedEvents(t *testing.T) {
})
})

// FIXME: Too much mixing of tests that should be more independent
t.Run("delayed message events are sent on timeout", func(t *testing.T) {
var res *http.Response
var countExpected uint64
Expand Down Expand Up @@ -139,6 +150,7 @@ func TestDelayedEvents(t *testing.T) {

stateKey := "to_send_on_timeout"

// Schedule a delayed event
Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

Added comments to understand these tests better

setterKey := "setter"
setterExpected := "on_timeout"
user.MustDo(
Expand All @@ -151,8 +163,11 @@ func TestDelayedEvents(t *testing.T) {
getDelayQueryParam("900"),
)

// Ensure that a delayed event is now scheduled
matchDelayedEvents(t, user, delayedEventsNumberEqual(1))

// And includes the correct content
//
// FIXME: This assertion seems superfluous to this test and should be it's own test
Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

Added some FIXME for the messiness I'd like to see cleaned up but not doing in this PR so the purpose is more focused and understandable.

res = getDelayedEvents(t, user)
must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
Expand All @@ -168,19 +183,30 @@ func TestDelayedEvents(t *testing.T) {
}),
},
})

// Sanity check that the room state hasn't changed yet
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
})

// Wait one second which will cause the delayed state event to be sent
time.Sleep(1 * time.Second)
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))

// Check for the state change from the delayed state event (using `MustSyncUntil` to
// account for any processing or worker replication delays)
user.MustSyncUntil(t, client.SyncReq{Filter: NoTimelineSyncFilter}, client.SyncStateHas(roomID, func(ev gjson.Result) bool {
return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey
}))
Comment on lines +196 to +200
Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

The main fix to all of these tests is using MustSyncUntil to wait and account for any processing or worker/replication delays.

// Make sure the state looks as expected after
res = user.MustDo(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
match.JSONKeyEqual(setterKey, setterExpected),
},
})
// No more delayed events
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))
Comment on lines +208 to +209
Copy link
Copy Markdown
Collaborator Author

@MadLittleMods MadLittleMods Apr 30, 2026

Choose a reason for hiding this comment

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

Moved this check until after we've actually waited for the state to show up and we know the delayed event was processed.

})

t.Run("cannot update a delayed event without an action", func(t *testing.T) {
Expand Down Expand Up @@ -232,6 +258,7 @@ func TestDelayedEvents(t *testing.T) {

stateKey := "to_never_send"

// Schedule a delayed event
setterKey := "setter"
setterExpected := "none"
res = user.MustDo(
Expand All @@ -245,22 +272,33 @@ func TestDelayedEvents(t *testing.T) {
)
delayID := client.GetJSONFieldStr(t, client.ParseJSON(t, res), "delay_id")

// Wait a bit but not long enough for the delayed state event to be sent
time.Sleep(1 * time.Second)
// We should still see the scheduled delayed event (hasn't been sent yet)
matchDelayedEvents(t, user, delayedEventsNumberEqual(1))

// Sanity check that the room state hasn't changed
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
})

// Cancel the delayed event
unauthedClient.MustDo(
t,
"POST",
getPathForUpdateDelayedEvent(delayID, DelayedEventActionCancel),
client.WithJSONBody(t, map[string]interface{}{}),
)
// No more delayed events
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))

// Sanity check that the previously scheduled delayed event doesn't end up being sent anyway
//
// Wait another second which would cause the previously scheduled delayed to be sent
// as we've waited a total of 2s now (> 1.5s delay)
time.Sleep(1 * time.Second)
// Sanity check that the room state hasn't changed
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
Expand All @@ -274,6 +312,7 @@ func TestDelayedEvents(t *testing.T) {

stateKey := "to_send_on_request"

// Schedule a delayed event
setterKey := "setter"
setterExpected := "on_send"
res = user.MustDo(
Expand All @@ -287,26 +326,39 @@ func TestDelayedEvents(t *testing.T) {
)
delayID := client.GetJSONFieldStr(t, client.ParseJSON(t, res), "delay_id")

// Wait a bit but not long enough for the delayed state event to be sent
time.Sleep(1 * time.Second)
// We should still see the scheduled delayed event (hasn't been sent yet)
matchDelayedEvents(t, user, delayedEventsNumberEqual(1))

// Sanity check that the room state hasn't changed yet
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
})

// Force the delayed event to be sent immediately
unauthedClient.MustDo(
t,
"POST",
getPathForUpdateDelayedEvent(delayID, DelayedEventActionSend),
client.WithJSONBody(t, map[string]interface{}{}),
)
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))

// Check for the state change from the delayed state event (using `MustSyncUntil` to
// account for any processing or worker replication delays)
user.MustSyncUntil(t, client.SyncReq{Filter: NoTimelineSyncFilter}, client.SyncStateHas(roomID, func(ev gjson.Result) bool {
return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey
}))
// Make sure the state looks as expected after
res = user.MustDo(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
match.JSONKeyEqual(setterKey, setterExpected),
},
})
// No more delayed events
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))
})

t.Run("delayed state events can be restarted", func(t *testing.T) {
Expand All @@ -316,6 +368,7 @@ func TestDelayedEvents(t *testing.T) {

defer cleanupDelayedEvents(t, user)

// Schedule a delayed event
setterKey := "setter"
setterExpected := "on_timeout"
res = user.MustDo(
Expand All @@ -329,35 +382,53 @@ func TestDelayedEvents(t *testing.T) {
)
delayID := client.GetJSONFieldStr(t, client.ParseJSON(t, res), "delay_id")

// Wait a bit but not long enough for the delayed state event to be sent
time.Sleep(1 * time.Second)
// We should still see the scheduled delayed event (hasn't been sent yet)
matchDelayedEvents(t, user, delayedEventsNumberEqual(1))

// Sanity check that the room state hasn't changed yet
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
})

// Restart the timer on the delayed event
unauthedClient.MustDo(
t,
"POST",
getPathForUpdateDelayedEvent(delayID, DelayedEventActionRestart),
client.WithJSONBody(t, map[string]interface{}{}),
)

// Wait a bit but not long enough for the delayed state event to be sent
time.Sleep(1 * time.Second)
// We should still see the scheduled delayed event (hasn't been sent yet)
matchDelayedEvents(t, user, delayedEventsNumberEqual(1))

// Sanity check that the room state hasn't changed yet
res = user.Do(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
StatusCode: 404,
})

// Wait one second which will cause the delayed state event to be sent
time.Sleep(1 * time.Second)
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))

// Check for the state change from the delayed state event (using `MustSyncUntil` to
// account for any processing or worker replication delays)
user.MustSyncUntil(t, client.SyncReq{Filter: NoTimelineSyncFilter}, client.SyncStateHas(roomID, func(ev gjson.Result) bool {
return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey
}))
// Make sure the state looks as expected after
res = user.MustDo(t, "GET", getPathForState(roomID, eventType, stateKey))
must.MatchResponse(t, res, match.HTTPResponse{
JSON: []match.JSON{
match.JSONKeyEqual(setterKey, setterExpected),
},
})
// No more delayed events
matchDelayedEvents(t, user, delayedEventsNumberEqual(0))
})

t.Run("delayed state events are kept on server restart", func(t *testing.T) {
Expand Down Expand Up @@ -439,20 +510,27 @@ func TestDelayedEvents(t *testing.T) {
// Capture whatever number of delayed events are remaining after the server restart.
remainingDelayedEventCount := countDelayedEvents(t, delayedEventResponse)
// Sanity check that the room state was updated correctly with the delayed events
// that were sent.
user.MustDo(t, "GET", getPathForState(roomID, eventType, stateKey1))
// that were sent. (using `MustSyncUntil` to account for any processing or worker
// replication delays)
user.MustSyncUntil(t, client.SyncReq{Filter: NoTimelineSyncFilter}, client.SyncStateHas(roomID, func(ev gjson.Result) bool {
return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey1
}))

// Wait until we see another delayed event being sent (ensure things resumed and are continuing).
time.Sleep(10 * time.Second)
matchDelayedEvents(t, user,
delayedEventsNumberLessThan(remainingDelayedEventCount),
)
// Sanity check that the other delayed events also updated the room state correctly.
// (using `MustSyncUntil` to account for any processing or worker replication
// delays)
//
// FIXME: Ideally, we'd check specifically for the last one that was sent but it
// will be a bit of a juggle and fiddly to get this right so for now we just check
// one.
user.MustDo(t, "GET", getPathForState(roomID, eventType, stateKey2))
user.MustSyncUntil(t, client.SyncReq{Filter: NoTimelineSyncFilter}, client.SyncStateHas(roomID, func(ev gjson.Result) bool {
return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey2
}))
})
}

Expand Down Expand Up @@ -577,6 +655,8 @@ func matchDelayedEvents(t *testing.T, user *client.CSAPI, checks ...delayedEvent
)
}

// FIXME: Instead of using `cleanupDelayedEvents`, each test should just use their own
// room
func cleanupDelayedEvents(t *testing.T, user *client.CSAPI) {
t.Helper()
res := getDelayedEvents(t, user)
Expand Down
Loading