Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deflake faster joins tests by leaving rooms as part of test cleanup #626

Merged
merged 3 commits into from
Mar 15, 2023

Conversation

squahtx
Copy link
Contributor

@squahtx squahtx commented Mar 14, 2023

Many of the faster joins test flakes are due to the homeserver under
test failing to contact Complement homeservers after they have been
torn down. When this happens, subsequent tests can fail if they use a
Complement homeserver that happens to have the same hostname:port as one
which the homeserver under test has previously marked as offline.

Make the homeserver under test leave rooms at the end of each test, so
that it stops trying to contact the Complement homeservers in the room.

Signed-off-by: Sean Quah seanq@matrix.org


Reviewable commit by commit.

NB: you can tease out the flakes by forcing Complement to reuse ports whenever possible:

diff --git a/internal/federation/server.go b/internal/federation/server.go
index ed7974d..4c4ccbe 100644
--- a/internal/federation/server.go
+++ b/internal/federation/server.go
@@ -455,7 +455,10 @@ func (s *Server) Listen() (cancel func()) {
        var wg sync.WaitGroup
        wg.Add(1)

-       ln, err := net.Listen("tcp", ":0") //nolint
+       ln, err := net.Listen("tcp", ":63333") //nolint
+       if err != nil {
+               ln, err = net.Listen("tcp", ":63334") //nolint
+       }
        if err != nil {
                s.t.Fatalf("ListenFederationServer: net.Listen failed: %s", err)
        }

Sean Quah added 3 commits March 10, 2023 19:37
This will allow test setup and teardown to expect PDUs and EDUs
independently of tests. In a future commit, test teardown will leave
rooms as part of cleanup and expect to see a corresponding leave event.
Make the homeserver under test leave test rooms at the end of each test,
so that it stops trying to contact other homeservers in the room.

Signed-off-by: Sean Quah <seanq@matrix.org>
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, huge thanks for this!

s.pduHandlers[pduHandlerKey] = pduHandler

return func() {
delete(s.pduHandlers, pduHandlerKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this roughly equivalent to del s.pduHandlers[pduHandlerKey] in Python?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine so

leaveChannel := make(chan *gomatrixserverlib.Event, 10)
removePDUHandler := s.AddPDUHandler(
func(e *gomatrixserverlib.Event) bool {
if membership, _ := e.Membership(); e.Type() == "m.room.member" &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the second thing returned by Membership() that gets bound(?) to _ -- a state key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's err (surprise!). membership is "" when there's no such state entry, so I elided the err check.

@squahtx
Copy link
Contributor Author

squahtx commented Mar 15, 2023

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants