-
Notifications
You must be signed in to change notification settings - Fork 55
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.
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
Make sure historical state events don't come down incremental /sync
(MSC2716)
#235
Make sure historical state events don't come down incremental /sync
(MSC2716)
#235
Conversation
Part of MSC2716: matrix-org/matrix-spec-proposals#2716 Related to matrix-org/synapse#11241 Split off from #221
/sync
/sync
(MSC2716)
// eventIDAfterHistoricalImport without any the | ||
// historicalEventIDs/historicalStateEventIDs in between, we're probably | ||
// safe to assume it won't sync. | ||
alice.SyncUntil(t, since, "", "rooms.join."+client.GjsonEscape(roomID)+".timeline.events", func(r gjson.Result) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this test more clear on what's happening. We now paginate sync from before we /batch_send
until an event that occurred after /batch_send
and just make sure we didnt' see any historical state or events in between.
// historicalEventIDs/historicalStateEventIDs in between, we're probably | ||
// safe to assume it won't sync. | ||
alice.SyncUntil(t, since, "", "rooms.join."+client.GjsonEscape(roomID)+".timeline.events", func(r gjson.Result) bool { | ||
if includes(r.Get("event_id").Str, historicalEventIDs) || includes(r.Get("event_id").Str, historicalStateEventIDs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where we additionally check for the historicalStateEventIDs
now
@@ -220,7 +220,7 @@ func TestImportHistoricalMessages(t *testing.T) { | |||
validateBatchSendRes(t, as, roomID, batchSendRes, false) | |||
}) | |||
|
|||
t.Run("Historical events from /batch_send do not come down in an incremental sync", func(t *testing.T) { | |||
t.Run("Historical events from batch_send do not come down in an incremental sync", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the /
in the test name so I can actually run it individually, COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS=1 COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh TestImportHistoricalMessages/parallel/Historical_events_from_batch_send_do_not_come_down_in_an_incremental_sync
See https://gist.github.com/MadLittleMods/4ab08f51609fab759247f299a4e33406 for why there is a problem using a /
to match a single test.
Thanks for the review @kegsay 🐿 |
Part of MSC2716: matrix-org/matrix-spec-proposals#2716
Related to matrix-org/synapse#11241
Split off from #221