-
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
Add test to ensure push/notifications work after room purge #1361
Conversation
311c9c2
to
5f99718
Compare
# We expect notification count to be 2, as we'll still see two events: | ||
# the first being $last_event_id and the second the "new message" | ||
assert_eq( $room->{unread_notifications}{"notification_count"}, 2 ); |
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.
So do we expect a purge to force re-calculation of notification counts?
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.
Hmm, good question. I guess what I really want to test is that the notification count increases. Though given this is a Synapse only test I'm not hugely bothered if we test Synapse specific stuff here.
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 guess that makes sense. I think if you still have a event push summary on a purged event then it will increase properly? Not 100% sure on that though.
Co-authored-by: Patrick Cloke <[email protected]>
Looks like a test might have flaked though? |
Ugh, yes. I'm failing to track that down. |
c.f. matrix-org/synapse#13476 (comment)