Conversation
Signed-off-by: Sean Quah <seanq@matrix.org>
…sponse Signed-off-by: Sean Quah <seanq@matrix.org>
Signed-off-by: Sean Quah <seanq@matrix.org>
Signed-off-by: Sean Quah <seanq@matrix.org>
Test that a homeserver can re-sync state from other homeservers in a partial-state room if the server it joined off is not working. Signed-off-by: Sean Quah <seanq@matrix.org>
internal/federation/handle.go
Outdated
| Type: "m.room.member", | ||
| StateKey: &userID, | ||
| PrevEvents: []string{room.Timeline[len(room.Timeline)-1].EventID()}, | ||
| Depth: room.Timeline[len(room.Timeline)-1].Depth() + 1, |
There was a problem hiding this comment.
This is a drive-by fix and doesn't affect the outcome of the test.
There was a problem hiding this comment.
Please don't mix this with tests where possible. The set of reviewers for $feature are different from the set of reviewers for handling Complement internals.
kegsay
left a comment
There was a problem hiding this comment.
Overall this is an excellent PR! Thanks!
internal/federation/handle.go
Outdated
| Type: "m.room.member", | ||
| StateKey: &userID, | ||
| PrevEvents: []string{room.Timeline[len(room.Timeline)-1].EventID()}, | ||
| Depth: room.Timeline[len(room.Timeline)-1].Depth() + 1, |
There was a problem hiding this comment.
Please don't mix this with tests where possible. The set of reviewers for $feature are different from the set of reviewers for handling Complement internals.
internal/federation/server.go
Outdated
| } | ||
|
|
||
| // Room returns the given room | ||
| func (s *Server) Room(roomID string) *ServerRoom { |
There was a problem hiding this comment.
This is not required. You call MustJoinRoom in the test which returns the *ServerRoom already.
| defer cancelListener() | ||
|
|
||
| // join complement to the public room | ||
| server.MustJoinRoom(t, deployment, "hs1", roomID, server.UserID("bob")) |
There was a problem hiding this comment.
This returns a *ServerRoom so you don't need to then retrieve it with the new server.Room(roomID) function on :245.
There was a problem hiding this comment.
I missed that, thank you!
| } | ||
|
|
||
| // reply to hs2 with a bogus /state_ids response | ||
| sendResponseWaiter.Finish() |
There was a problem hiding this comment.
This could have a clearer name, e.g sendStateIDsResponse, as otherwise it's unclear which response you mean and need to add comments to explain it.
There was a problem hiding this comment.
I'll rename them to follow the naming in partialStateJoinResult:
type partialStateJoinResult struct {
cancelListener func()
Server *federation.Server
ServerRoom *federation.ServerRoom
fedStateIdsRequestReceivedWaiter *Waiter
fedStateIdsSendResponseWaiter *Waiter
}|
@kegsay Thanks for reviewing. Could you re-review the latest changes? |
| defer cancelListener() | ||
|
|
||
| // join complement to the public room | ||
| room := server.MustJoinRoom(t, deployment, "hs1", roomID, server.UserID("bob")) |
There was a problem hiding this comment.
worth noting there is also a bob@hs1 created by the blueprint. It might be better to pick a fourth username.
There was a problem hiding this comment.
I went for david instead.
| queryParams := req.URL.Query() | ||
| t.Logf("Incoming state_ids request for event %s in room %s", queryParams["event_id"], roomID) | ||
| fedStateIdsRequestReceivedWaiter.Finish() | ||
| fedStateIdsSendResponseWaiter.Waitf(t, 60*time.Second, "Waiting for /state request") |
There was a problem hiding this comment.
"Waiting for /state request" doesn't seem the right message here. I think this will only time out if the test fails elsewhere anyway, so .Wait without a dedicated message might be more appropriate.
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Rename bob to david and drop the timeout message.
richvdh
left a comment
There was a problem hiding this comment.
lgtm, and I need to build stuff on top of it, so I'm going to go ahead and merge it.
Tests matrix-org/synapse#12812