-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add tests for /make_knock
and /send_knock
(same as the join counterparts) for when the federation server is in the middle of a partial join
#447
Conversation
ee3dc39
to
1098798
Compare
1098798
to
d017259
Compare
d017259
to
b196ba4
Compare
Taking this out of the review queue since it's been re-marked as draft. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. (I'm guessing that other than the MakeRespMakeKnock and GMSL changes that this was largely a find-and-replace for join -> knock
?
RoomVersion: room.Version, | ||
KnockEvent: builder, | ||
} | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we implicitly returning resp, err
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I find this weird but Go supports this old-school Pascal-ish style of assigning to a result variable rather than returning, except the result variable's names are declared in the function declaration.
I nicked this from what was already there; it's 'not my choice' but it seems like this may be idiomatic
|
||
// daniel then tries to /send_knock via the homeserver under test | ||
knockEvent, err := makeKnockResp.KnockEvent.Build(time.Now(), gomatrixserverlib.ServerName(testServer2.ServerName()), testServer2.KeyID, testServer2.Priv, makeKnockResp.RoomVersion) | ||
must.NotError(t, "KnockEvent.Build", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this just shorthand for if err != nil { /* fail the test with given message */ }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, seems to be
// NotError will ensure `err` is nil else terminate the test with `msg`.
func NotError(t *testing.T, msg string, err error) {
t.Helper()
if err != nil {
t.Fatalf("MustNotError: %s -> %s", msg, err)
}
}
Fixes #445.