Add tests for MSC4429: Profile Updates for Legacy Sync#849
Add tests for MSC4429: Profile Updates for Legacy Sync#849anoadragon453 wants to merge 1 commit intomainfrom
Conversation
MadLittleMods
changed the title
| ) | ||
|
|
||
| const ( | ||
| msc4429UsersStable = "users" |
There was a problem hiding this comment.
Seems pre-mature to support stable given matrix-org/matrix-spec-proposals#4429 is no where near that point 🤔
Perhaps we just comment that part out for now. Useful for our future reference when we stabilize
|
|
||
| // Exclude 'displayname' | ||
| filter := mustBuildMSC4429Filter(t, []string{"m.status"}) | ||
| res, _ := alice.MustSync(t, client.SyncReq{Filter: filter}) |
There was a problem hiding this comment.
Might be flaky if we don't use a sync until utility for profile updates to show up. (read after write consistency)
The worker processing profile updates might not be the same one that serves /sync.
| assertNoProfileUpdate(t, res, bob.UserID, "displayname") | ||
| }) | ||
|
|
||
| t.Run("No updates without profile_fields filter", func(t *testing.T) { |
| // No filter = no profile fields returned. | ||
| _, since := alice.MustSync(t, client.SyncReq{}) | ||
| mustSetProfileField(t, bob, "m.status", map[string]interface{}{ | ||
| "text": "away", | ||
| }) |
There was a problem hiding this comment.
As a sanity check, we should have another observer user that sees the m.status with a filter, and then alice syncs without a filter
| mustSetProfileField(t, bob, "m.status", map[string]interface{}{ | ||
| "text": "away", | ||
| }) |
There was a problem hiding this comment.
Separate action from previous request
| mustSetProfileField(t, bob, "m.status", map[string]interface{}{ | |
| "text": "away", | |
| }) | |
| mustSetProfileField(t, bob, "m.status", map[string]interface{}{ | |
| "text": "away", | |
| }) |
| // No filter = no profile fields returned. | ||
| _, since := alice.MustSync(t, client.SyncReq{}) |
There was a problem hiding this comment.
We don't actually assert anything for the initial sync request
| assertNoProfileUpdate(t, res, bob.UserID, "m.status") | ||
| }) | ||
|
|
||
| t.Run("Incremental sync returns the latest update", func(t *testing.T) { |
There was a problem hiding this comment.
Perhaps we should have another test to make sure more subsequent incremental /sync include new updates for the new user.
This ensures that any caching (remembering which users were sent down before) is correct.
There was a problem hiding this comment.
I guess, we kinda have this already with "Cleared profile field is returned as null"
| func assertNoProfileUpdate(t *testing.T, res gjson.Result, userID, field string) { | ||
| t.Helper() | ||
| if update, ok := getProfileUpdate(res, userID, field); ok { | ||
| t.Fatalf("unexpected profile update for %s %s: %s", userID, field, update.Raw) |
There was a problem hiding this comment.
This is correct but the language is ambiguous whether we expected no profile update or just a different profile update.
| msc4429UsersUnstable = "org\\.matrix\\.msc4429\\.users" | ||
| ) | ||
|
|
||
| func TestMSC4429ProfileUpdates(t *testing.T) { |
There was a problem hiding this comment.
@Half-Shot mentioned on the MSC that it would be good for the homeserver to send down nulls to clear a user's profile once they no longer share any rooms with you (after leaving a room).
It would be good to add a test case for this, ideally replacing the profile object with just a null as mentioned in my follow-up comment, to indicate "all fields should be cleared".
2 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