-
Notifications
You must be signed in to change notification settings - Fork 269
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
fix(event cache): keep the previous-batch token when we haven't enabled storage #4495
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4495 +/- ##
==========================================
- Coverage 85.36% 85.36% -0.01%
==========================================
Files 284 284
Lines 31886 31903 +17
==========================================
+ Hits 27219 27233 +14
- Misses 4667 4670 +3 ☔ View full report in Codecov by Sentry. |
This removes a few manual uses of `ResponseTemplate`, which is sweet and guarantees some better typing for those responses overall.
9023340
to
bb444d0
Compare
MatrixMock { server: self.server, mock } | ||
} | ||
} | ||
|
||
/// A response to a [`RoomMessagesEndpoint`] query. | ||
pub struct RoomMessagesResponse { |
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.
Can we call this a bit differently? It's not the real Ruma response we use everywhere else, it's a test helper. Perhaps RoomMessageResponseTemplate
?
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.
MockRoomMessages
?
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.
Well we're not mocking, and it's not a message. It's about the response that you would send out if you were to send a room message. Then we pass that response to the mock server which will mock things for us.
I picked the Template
naming because wiremock, which we use and wrap here, is using that term as well: https://docs.rs/wiremock/latest/wiremock/struct.ResponseTemplate.html#.
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.
I like RoomMessageResponseTemplate
, thanks!
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.
Approving the real commit :-).
MatrixMock { server: self.server, mock } | ||
} | ||
} | ||
|
||
/// A response to a [`RoomMessagesEndpoint`] query. | ||
pub struct RoomMessagesResponse { |
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.
MockRoomMessages
?
When we don't have enabled storage, we should keep all the previous-batch tokens, otherwise we'll miss events, lacking any form of caching.
Now with a test.
Part of #3280.