-
Notifications
You must be signed in to change notification settings - Fork 52
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
Ensure many batches are able to be imported (MSC2716) #212
Ensure many batches are able to be imported (MSC2716) #212
Conversation
…s-work Conflicts: tests/msc2716_test.go
"dir": []string{"b"}, | ||
"limit": []string{"100"}, | ||
})) | ||
}) |
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.
Maybe move this down one more test
} | ||
|
||
// Ensure the /message response gives a 200 | ||
alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "rooms", roomID, "messages"}, client.WithContentType("application/json"), client.WithQueries(url.Values{ |
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.
It would be nice to assert the number of events returned too, and that they have different event IDs.
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.
👍 Going to close this one in favor of #214 which actually checks for all of the messages in the batches
Thanks for the review @kegsay 🦎 |
Ensure many batches are able to be imported and
/messages
still gives us a 200 OK.Used to reproduce https://gitlab.com/gitterHQ/webapp/-/merge_requests/2229#note_665906390 which led to the fix in Synapse, matrix-org/synapse#11027.
Part of MSC2716: matrix-org/matrix-spec-proposals#2716