Conversation
…ectly in the test itself due to lack of GMSL support
| if e.StateKey != nil { | ||
| paths = []string{"_matrix", "client", "v3", "rooms", roomID, "state", e.Type, *e.StateKey} | ||
| } | ||
| qps := url.Values{} |
There was a problem hiding this comment.
What does this stand for? query parameter xxx (string? strings?)
An LLM said "query parameters" which is probably the right answer ⏩
Maybe too opaque
| "lists": map[string]any{ | ||
| "any-key": map[string]any{ | ||
| "timeline_limit": 10, | ||
| "required_state": [][]string{{"*", "*"}}, |
There was a problem hiding this comment.
Might as well request no state since it's never used
| "required_state": [][]string{{"*", "*"}}, | |
| "required_state": [][]string{}, |
There was a problem hiding this comment.
Perhaps this is because we actually create sticky state events (see makeStickyEvent(...)) but we don't ever look for it in required_state so we can still get rid of this.
| }, | ||
| }, | ||
| } | ||
| qps := url.Values{"timeout": []string{"5000"}} |
There was a problem hiding this comment.
We could align with the same timeout we use for normal /sync
Line 160 in 091cbf5
| qps := url.Values{"timeout": []string{"5000"}} | |
| qps := url.Values{"timeout": []string{"1000"}} |
| return respBody, newPos | ||
| } | ||
|
|
||
| // standardised response format for /sync and SSS |
There was a problem hiding this comment.
| // standardised response format for /sync and SSS | |
| // Normalized response format for /sync and SSS |
🤷
| resp, nextSince = cli.MustSync(t, client.SyncReq{Since: since}) | ||
| timeline = resp.Get("rooms.join." + client.GjsonEscape(roomID) + ".timeline.events").Array() | ||
| sticky = resp.Get("rooms.join." + client.GjsonEscape(roomID) + ".msc4354_sticky.events").Array() | ||
| // t.Logf("%s\b", resp.Raw) |
There was a problem hiding this comment.
Leftover debug log
Fine to leave it if you want
| if hasStickyEvent(syncResp.timelineEvents) { | ||
| ct.Fatalf(t, "timeline had the sticky event, is delayed events supported?") | ||
| } | ||
| must.Equal(t, len(syncResp.stickyEvents), 0, "events were in sticky events when they shouldn't have been") |
There was a problem hiding this comment.
| must.Equal(t, len(syncResp.stickyEvents), 0, "events were in sticky events when they shouldn't have been") | |
| must.Equal(t, len(syncResp.stickyEvents), 0, "events were listed in sticky events, is delayed events supported?") |
| // wait for the sticky event to send | ||
| time.Sleep(4 * time.Second) | ||
|
|
||
| for i := 0; i < 25; i++ { |
There was a problem hiding this comment.
(explain why 25, related to discussion above)
| } | ||
| } | ||
|
|
||
| func TestUnsignedTTL(t *testing.T) { |
There was a problem hiding this comment.
Test description would be nice
We're just checking that unsigned.msc4354_sticky_duration_ttl_ms shows up. Useful for clients because ...
|
|
||
| syncResp := gatherSyncResults(t, bob, useSimplifiedSlidingSync, roomID, stopEventID) | ||
| mustHaveStickyEventID(t, stickyEventIDNotInTimeline, syncResp.stickyEvents) | ||
| mustHaveStickyEventID(t, stickyEventIDInTimeline, syncResp.stickyEvents) |
There was a problem hiding this comment.
For my own understanding, this is more like stickyEventIDThatWouldBeInTimelineIfHistoryVisibilityWasBroken
Or perhaps, stickyEventIDInTimeline actually shows up in the timeline not just in the sticky list 🤔 - I'm guessing it's this.
⏩
| // Test that newly joined users to history_visibility: joined rooms correctly see sticky events | ||
| // in the `sticky` section. | ||
| func TestStickyEventsIgnoreHistoryVisibility(t *testing.T) { |
There was a problem hiding this comment.
We seem to be missing some tests for some expired sticky events and whether they should show up
| if len(stickyEventIDs) != 0 { | ||
| ct.Fatalf(t, "failed to see all sticky events, missing %d", len(stickyEventIDs)) | ||
| } | ||
| } |
There was a problem hiding this comment.
missing a test for this aspect of the MSC
Calculate the end time as start_time + min(sticky_duration_ms, 3600000).
| if len(stickyEventIDs) != 0 { | ||
| ct.Fatalf(t, "failed to see all sticky events, missing %d", len(stickyEventIDs)) | ||
| } | ||
| } |
There was a problem hiding this comment.
missing a test of this aspect of the MSC, I think
As with normal events, sticky events sent by ignored users MUST NOT be delivered to clients.
3 participants
Synapse PR: element-hq/synapse#18968
MSC4354
Pull Request Checklist