Remove messy Destroy test clean-up in TestPartialStateJoin#895
Remove messy Destroy test clean-up in TestPartialStateJoin#895MadLittleMods wants to merge 4 commits into
Destroy test clean-up in TestPartialStateJoin#895Conversation
All of this code has been obsoleted by #880
| defer cancel() | ||
| serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) | ||
| psjResult := beginPartialStateJoin(t, server, serverRoom, alice) | ||
| defer psjResult.Destroy(t) |
There was a problem hiding this comment.
Removing all of the Destroy usage since it's no longer necessary
| case <-sendResponseWaiter.Done(): | ||
| // Happy-path now that we're done waiting, continue serving the request now | ||
| case <-req.Context().Done(): |
There was a problem hiding this comment.
Instead of relying on Destroy to finish the waiter to allow this to continue (instead of waiting forever), we instead rely on engineered homeserver being closed and all of the in-flight requests being cancelled (req.Context().Done()).
We still have a 60 second timeout like before.
|
|
||
| // The caller is about to tear down the Complement homeserver. Leave the room, so | ||
| // that the homeserver under test stops sending it presence updates. | ||
| psj.Server.WithWaitForLeave( |
There was a problem hiding this comment.
#894 removes the WithWaitForLeave(...) part of Destroy(...)
This PR removes the need for finishing the waiters.
Which means Destroy(...) doesn't do anything anymore and we can remove it altogether. Less to think about.
| // wait is skipped when `user` had already left the room (per their own | ||
| // homeserver, so the action produces no new leave) or when this server isn't in | ||
| // the room (so the leave won't be federated to us). | ||
| func (s *server) WithWaitForLeave( |
There was a problem hiding this comment.
The WithWaitForLeave changes in the diff disappear once #894 is merged
This PR builds on that one so they are also included here for now.
1 participant
Remove messy
Destroytest clean-up inTestPartialStateJoinFollow-up to #894 which removes another part of the clean-up code in this file and #880 which introduces a better way to avoid the test pollution and obsoletes the need for all of this cleanup logic. See the PR description there for a more complete context on why.
Destroy(...)was originally introduced in this context:But thanks to #880 we no longer use the same
hostname:portfor engineered homeservers anymore.Todo
WithWaitForLeavetest clean-up inTestPartialStateJoin#894 to mergePull Request Checklist