Skip to content

Add tests for MSC4155#782

Merged
anoadragon453 merged 9 commits into
mainfrom
hs/msc4155-invite-filtering
Jun 3, 2025
Merged

Add tests for MSC4155#782
anoadragon453 merged 9 commits into
mainfrom
hs/msc4155-invite-filtering

Conversation

@Half-Shot
Copy link
Copy Markdown
Collaborator

@Half-Shot Half-Shot commented May 29, 2025

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

Comment thread tests/msc4155/main_test.go Outdated
Comment thread tests/msc4155/invite_filter_test.go Outdated
Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 left a 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 and blocked_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.

Comment thread tests/msc4155/invite_filter_test.go
@Half-Shot
Copy link
Copy Markdown
Collaborator Author

Half-Shot commented Jun 3, 2025

Could you add a few more sanity checks

These are done, with the exception of Escaping a glob works, so you can match a user with ID @*?:example.com. There isn't a documented way to escape globs in Matrix yet, and I wouldn't want to introduce a test without spec backing it.

EDIT We agreed to ignore this for now, as matrix-org/matrix-spec#2156 is a problem.

@Half-Shot Half-Shot requested a review from anoadragon453 June 3, 2025 11:24
Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 left a 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!

@anoadragon453 anoadragon453 merged commit 7675158 into main Jun 3, 2025
7 of 8 checks passed
@anoadragon453 anoadragon453 deleted the hs/msc4155-invite-filtering branch June 3, 2025 13:53
jevolk pushed a commit to matrix-construct/complement that referenced this pull request Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants