Add tests for MSC4429: Profile Updates for Legacy Sync#849
Add tests for MSC4429: Profile Updates for Legacy Sync#849anoadragon453 wants to merge 13 commits into
Conversation
MadLittleMods
changed the title
Add various comments throughout for clarification.
|
The tests in this PR weren't actually running in CI 🤦 I've fixed that in 4cddfea. All were expected to succeed (despite all failing), other than the newly added Still, the tests here should be ready for another round of review. |
| repo: element-hq/synapse | ||
| tags: synapse_blacklist | ||
| packages: ./tests/msc3874 ./tests/msc3902 ./tests/msc4306 | ||
| packages: ./tests/msc3874 ./tests/msc3902 ./tests/msc4306 ./tests/msc4429 |
There was a problem hiding this comment.
The tests in this PR weren't actually running in CI 🤦 I've fixed that in 4cddfea.
I assume you meant 2b67137 but in any case, we don't really worry about updating this list.
It's already heavily out of sync with the list we run in the Synapse project, element-hq/synapse -> scripts-dev/complement.sh#L272-L289
At this point, I think we just rely on it as a general sanity check that Complement is still working with Synapse.
There was a problem hiding this comment.
I was planning to update it to match Synapse's list in a separate PR. Seems useful to prevent Complement changes breaking Synapse CI?
|
|
||
| // syncHasProfileUpdatesNull checks whether a sync response contains a null | ||
| // profile_updates value for the given user. | ||
| func syncHasProfileUpdatesNull(userID string) client.SyncCheckOpt { |
There was a problem hiding this comment.
Seems like the usage could be replaced by syncHasProfileUpdate(...)
Or at-least this function re-uses syncHasProfileUpdate(...)
There was a problem hiding this comment.
Mm, it's a bit different as syncHasProfileUpdate expects a field name and an expected value. Where in the null case, the whole profile_updates dict is replaced by a null value.
So you'd probably need to add a fourth argument to syncHasProfileUpdate to indicate that you want to check for null... and that's starting to feel a bit messy to me.
…pdates To test that caching functionality in the homeserver does not result in old profile updates being sent down erroneously.
|
This PR is good to go from my perspective. I'm now just waiting for the associated Synapse PR to be approved, before we merge it (plus the associated sytest PR) all together. |
3 participants
This PR introduces some initial Complement tests for the current draft of MSC4429: Profile Updates for Legacy Sync.
Note: This PR was almost entirely written by OpenAI Codex, but has been reviewed manually. Extra review scrutiny is advised.
Tests are expected to fail until the associated homeserver implementation has been written.
Pull Request Checklist