Test that invites from ignored users are omitted from incremental syncs #267
Conversation
|
Fails on Dendrite: Guessing this isn't implemented in Dendrite yet: if so, suggest blacklisting this test. |
|
You can do that yourself by adding |
Maybe this doesn't need a helper though?
|
DMRobertson
force-pushed
the
dmr/ignored-users-invites
branch
from
b2ea172
to
0b453d9
Compare
DMRobertson
changed the title
My understanding is that |
|
Maybe, but more recently, skipping specific tests were introduced, these are triggered on |
tests/csapi/ignored_users_test.go
Outdated
| // Synapse does not have this property, as detailed in | ||
| // https://github.com/matrix-org/synapse/issues/11506. | ||
| // This reproduces that bug. |
There was a problem hiding this comment.
Maybe?
| // Synapse does not have this property, as detailed in | |
| // https://github.com/matrix-org/synapse/issues/11506. | |
| // This reproduces that bug. | |
| // Synapse does not handle this properly, as detailed in | |
| // https://github.com/matrix-org/synapse/issues/11506. | |
| // This reproduces that bug. |
I don't think this is true though since this is now fixed, it might be better to say:
This is a regression test for
link to issue.
| // https://github.com/matrix-org/synapse/issues/11506. | ||
| // This reproduces that bug. | ||
| func TestInviteFromIgnoredUsersDoesNotAppearInSync(t *testing.T) { | ||
| deployment := Deploy(t, b.BlueprintCleanHS) |
There was a problem hiding this comment.
This test could be much simpler, I think, by starting with BlueprintOneToOneRoom? I'm not sure the third user is very helpful here.
There was a problem hiding this comment.
The third user was an attempt to stop this test from being racey. We want to be convinced that the server has processed Bob's invite and chosen to ignore it. I could assert that there's no invite from Bob in Alice's next sync, but... what if that's only true because Alice synced really quickly?
In #237 (comment) suggested sending and awaiting a dummy event. The third user was my attempt at doing something similar here.
There was a problem hiding this comment.
I think you could do that by sending a dummy user into the room, yes.
There was a problem hiding this comment.
Oh, would Alice expect to see state updates for a room she's invited to, but hasn't joined yet? If so that would be much clearer.
There was a problem hiding this comment.
Oh, would Alice expect to see state updates for a room she's invited to, but hasn't joined yet? If so that would be much clearer.
No, they would not see those. I think I'm confused though -- isn't this just checking that some sort of events got processed by any room? (I.e. sync is working at all?)
|
I requested review from @kegsay since I believe he has thoughts about the generic infrastructure of complement. |
tests/csapi/ignored_users_test.go
Outdated
| chris := deployment.RegisterUser(t, "hs1", "chris", "sufficiently_long_password_chris") | ||
|
|
||
| // Alice creates a room for herself. | ||
| public_room := alice.CreateRoom(t, map[string]interface{}{ |
There was a problem hiding this comment.
Please do not use underscores for variable names.
There was a problem hiding this comment.
Sorry, old habits. Is there some way to configure gofmt/goimports or VSCode to flag this up?
tests/csapi/ignored_users_test.go
Outdated
| alice.MustDoFunc( | ||
| t, | ||
| "PUT", | ||
| []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", "m.ignored_user_list"}, |
There was a problem hiding this comment.
Dendrite doesn't implement any /v3/ endpoint. Is there some spec documentation on the changes between r0 and v3?
There was a problem hiding this comment.
Dendrite doesn't implement any
/v3/endpoint. Is there some spec documentation on the changes betweenr0andv3?
All r0 endpoints became v3 with Matrix 1.1 -- @DMRobertson Maybe we should just use r0 for now here?
There was a problem hiding this comment.
Happy to use r0; I was just blindly going by what the latest spec said.
maybe dendrite does support account data under r0?
|
Using r0 means that we PUT the account data, but the ignored invite isn't still ignored. I'd forgotten that matrix-org/dendrite#600 was a thing. |
This reverts commit 1086814.
Reproduces matrix-org/synapse#11506.
Leaving this as a draft until I fix the issue in Synapse. Sticking up a PR for now to see what Dendrite does.
Also note: fd3a555 is cherry-picked from #237