Regression test for /sync returning 404 for redactions #386
Conversation
Lots more detail in the code, but this is a regression test for a bug in Synapse where it would return a 404 from /sync if given a since token which pointed to a redaction of an unknown event.
tests/csapi/sync_test.go
Outdated
| nextBatch := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHasEventID(roomID_2, sentinelEvent.EventID())) | ||
|
|
||
| // one more sync, to get the latest sync token | ||
| _, nextBatch = alice.MustSync(t, client.SyncReq{Since: nextBatch}) |
There was a problem hiding this comment.
It's unclear why this is necessary. We sent the redaction then the sentinel event, so if :322 is returning the sentinel event then surely this sync will just do nothing..?
There was a problem hiding this comment.
You're right, but I got this wrong because the documentation on MustSyncUntil is unclear. I fixed it in ed0c3a7.
| // In the unlikely event that you need ordering on your checks, call MustSyncUntil multiple times | ||
| // with a single checker, and reuse the returned since token, as in the "Incremental sync" example. |
There was a problem hiding this comment.
aside: does this actually work? Suppose you have two tests A and B, and you need to make sure that A happens before B. But presumably it's legitimate for them to happen in the same sync batch? In which case the suggested approach will fail, because the test for A will pass, but we'll miss B happening so the test for B will never pass.
|
(thanks for the excellent review comments btw) |
Lots more detail in the code, but this is a regression test for matrix-org/synapse#12864: Synapse would return a 404 from /sync if given a since token which pointed to a redaction of an unknown event.