Update tests to accommodate /batch_send returning separate meta event fields (MSC2716)
#204
Conversation
Conflicts: tests/msc2716_test.go
…a composable functions
Synapse support was added in matrix-org/synapse#10498
Synapse change added in matrix-org/synapse#10552
…ersions Synapse changes added in matrix-org/synapse#10566
…ent that exists Add tests for: - matrix-org/synapse#10776
…fields Corresponding Synapse changes in matrix-org/synapse#10777 See matrix-org/matrix-spec-proposals#2716 (comment) Part of matrix-org/matrix-spec-proposals#2716
| @@ -112,43 +112,53 @@ func TestBackfillingHistory(t *testing.T) { | |||
| eventIDsAfter := createMessagesInRoom(t, alice, roomID, 2) | |||
|
|
|||
| // Insert the most recent chunk of backfilled history | |||
| insertTime1 := timeAfterEventBefore.Add(timeBetweenMessages * 3) | |||
| insertTime0 := timeAfterEventBefore.Add(timeBetweenMessages * 3) | |||
There was a problem hiding this comment.
Updating from insertTime1 -> insertTime0 to match the chunk explanation below
…emods/msc2716-room-creator-existing-room-version Conflicts: tests/msc2716_test.go
…g-room-version Conflicts: tests/msc2716_test.go
tests/msc2716_test.go
Outdated
| if len(historicalEventIDs) != 5 { | ||
| t.Fatalf("Expected eventID list should be length 5 but saw %d: %s", len(historicalEventIDs), historicalEventIDs) | ||
| if len(historicalEventIDs) != 2 { | ||
| t.Fatalf("Expected eventID list should be length 2 but saw %d: %s", len(historicalEventIDs), historicalEventIDs) |
There was a problem hiding this comment.
Please please please document why you expect this to be 2. I don't see you changing the number of injected events, so why shouldn't this be 5? I'm guessing the event IDs returned in the batch endpoint are the earliest or something? But this is really really not clear. It's hard enough to follow all the event IDs and markers and timestamps without randomly changing a 5 into a 2 without any reason why.
There was a problem hiding this comment.
What's changed here is we have gone from including the 3 insertion and chunk events in the event_ids response, to separating them out to their own fields.
So now event_ids in the response only includes the 2 historical events that you sent in your request which is much more obvious for a downstream user (spawned from matrix-org/matrix-spec-proposals#2716 (comment))
I've updated the assertion message to make it clear Expected 2 event_ids in the response that correspond to the 2 events we sent in the request but saw XX: ...
…n' into madlittlemods/msc2716-verify-params
…/split-batch-send-response-fields Conflicts: tests/msc2716_test.go
MadLittleMods
changed the base branch from
madlittlemods/msc2716-verify-params
to
master
|
@kegsay Thanks for the initial round of review 🐷 Good shout on the useless getters! Merging as the chain of PRs is getting a little too unwieldy. I'm willing to change anything you comment about in a follow-up though ⏩
|
Update tests to accommodate
/batch_sendreturning separate meta event fields.Corresponding Synapse changes in matrix-org/synapse#10777
See matrix-org/matrix-spec-proposals#2716 (comment)
Part of MSC2716