-
Notifications
You must be signed in to change notification settings - Fork 61
Add tests for MSC4155 #782
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
Conversation
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.
The existing tests look great! Thanks for compiling these on top of Synapse's unit tests.
Could you add a few more sanity checks:
- A field present, but set to
null
, is treated as an empty array. - A user being in both
allowed_users
andblocked_users
should allow the user. - Glob-style matching that uses a
?
to represent a single character (spec) - Escaping a glob works, so you can match a user with ID
@*?:example.com
(which is allowed under the historical user ID spec.
These are done, with the exception of EDIT We agreed to ignore this for now, as matrix-org/matrix-spec#2156 is a problem. |
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.
This now LGTM. Thank you for doing this!
For element-hq/synapse#18288
This implements tests for matrix-org/matrix-spec-proposals#4155
Tests will not pass as I do not have rights to push to element-hq/Synapse, so it won't be able to find my branch.
Pull Request Checklist