From 40b9877c03cac7054ef4d868c76d47b9ef2d91f8 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Thu, 27 Oct 2022 17:35:18 +0200 Subject: [PATCH] remove MustDo (#533) --- internal/client/client.go | 27 +++------- tests/csapi/apidoc_login_test.go | 12 ++--- tests/csapi/sync_filter_test.go | 4 +- tests/federation_query_profile_test.go | 2 +- tests/knocking_test.go | 72 ++++++++++--------------- tests/msc2836_test.go | 8 +-- tests/restricted_room_hierarchy_test.go | 2 +- tests/room_hierarchy_test.go | 12 ++--- 8 files changed, 54 insertions(+), 85 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index e4a53ee3..048b4160 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -115,7 +115,7 @@ func (c *CSAPI) UploadContent(t *testing.T, fileBody []byte, fileName string, co func (c *CSAPI) DownloadContent(t *testing.T, mxcUri string) ([]byte, string) { t.Helper() origin, mediaId := SplitMxc(mxcUri) - res := c.MustDo(t, "GET", []string{"_matrix", "media", "v3", "download", origin, mediaId}, struct{}{}) + res := c.MustDoFunc(t, "GET", []string{"_matrix", "media", "v3", "download", origin, mediaId}) contentType := res.Header.Get("Content-Type") b, err := ioutil.ReadAll(res.Body) if err != nil { @@ -127,7 +127,7 @@ func (c *CSAPI) DownloadContent(t *testing.T, mxcUri string) ([]byte, string) { // CreateRoom creates a room with an optional HTTP request body. Fails the test on error. Returns the room ID. func (c *CSAPI) CreateRoom(t *testing.T, creationContent interface{}) string { t.Helper() - res := c.MustDo(t, "POST", []string{"_matrix", "client", "v3", "createRoom"}, creationContent) + res := c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "createRoom"}, WithJSONBody(t, creationContent)) body := ParseJSON(t, res) return GetJSONFieldStr(t, body, "room_id") } @@ -166,7 +166,7 @@ func (c *CSAPI) InviteRoom(t *testing.T, roomID string, userID string) { body := map[string]interface{}{ "user_id": userID, } - c.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "invite"}, body) + c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "invite"}, WithJSONBody(t, body)) } func (c *CSAPI) GetGlobalAccountData(t *testing.T, eventType string) *http.Response { @@ -186,7 +186,7 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { if e.StateKey != nil { paths = []string{"_matrix", "client", "v3", "rooms", roomID, "state", e.Type, *e.StateKey} } - res := c.MustDo(t, "PUT", paths, e.Content) + res := c.MustDoFunc(t, "PUT", paths, WithJSONBody(t, e.Content)) body := ParseJSON(t, res) eventID := GetJSONFieldStr(t, body, "event_id") t.Logf("SendEventSynced waiting for event ID %s", eventID) @@ -323,7 +323,7 @@ func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, "username": localpart, "password": password, } - res := c.MustDo(t, "POST", []string{"_matrix", "client", "v3", "register"}, reqBody) + res := c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "register"}, WithJSONBody(t, reqBody)) body, err := ioutil.ReadAll(res.Body) if err != nil { @@ -401,21 +401,6 @@ func (c *CSAPI) GetDefaultRoomVersion(t *testing.T) gomatrixserverlib.RoomVersio return gomatrixserverlib.RoomVersion(defaultVersion.Str) } -// MustDo will do the HTTP request and fail the test if the response is not 2xx -// -// Deprecated: Prefer MustDoFunc. MustDo is the older format which doesn't allow for vargs -// and will be removed in the future. MustDoFunc also logs HTTP response bodies on error. -func (c *CSAPI) MustDo(t *testing.T, method string, paths []string, jsonBody interface{}) *http.Response { - t.Helper() - res := c.DoFunc(t, method, paths, WithJSONBody(t, jsonBody)) - if res.StatusCode < 200 || res.StatusCode >= 300 { - defer res.Body.Close() - body, _ := ioutil.ReadAll(res.Body) - t.Fatalf("CSAPI.MustDo %s %s returned HTTP %d : %s", method, res.Request.URL.String(), res.StatusCode, string(body)) - } - return res -} - // WithRawBody sets the HTTP request body to `body` func WithRawBody(body []byte) RequestOpt { return func(req *http.Request) { @@ -472,7 +457,7 @@ func (c *CSAPI) MustDoFunc(t *testing.T, method string, paths []string, opts ... if res.StatusCode < 200 || res.StatusCode >= 300 { defer res.Body.Close() body, _ := ioutil.ReadAll(res.Body) - t.Fatalf("CSAPI.MustDoFunc response return non-2xx code: %s - body: %s", res.Status, string(body)) + t.Fatalf("CSAPI.MustDoFunc %s %s returned non-2xx code: %s - body: %s", method, res.Request.URL.String(), res.Status, string(body)) } return res } diff --git a/tests/csapi/apidoc_login_test.go b/tests/csapi/apidoc_login_test.go index 52275310..5a7823ab 100644 --- a/tests/csapi/apidoc_login_test.go +++ b/tests/csapi/apidoc_login_test.go @@ -43,14 +43,14 @@ func TestLogin(t *testing.T) { // sytest: POST /login can log in as a user t.Run("POST /login can login as user", func(t *testing.T) { t.Parallel() - res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, json.RawMessage(`{ + res := unauthedClient.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{ "type": "m.login.password", "identifier": { "type": "m.id.user", "user": "@test_login_user:hs1" }, "password": "superuser" - }`)) + }`))) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ @@ -63,7 +63,7 @@ func TestLogin(t *testing.T) { t.Run("POST /login returns the same device_id as that in the request", func(t *testing.T) { t.Parallel() deviceID := "test_device_id" - res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, json.RawMessage(`{ + res := unauthedClient.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{ "type": "m.login.password", "identifier": { "type": "m.id.user", @@ -71,7 +71,7 @@ func TestLogin(t *testing.T) { }, "password": "superuser", "device_id": "`+deviceID+`" - }`)) + }`))) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ @@ -84,14 +84,14 @@ func TestLogin(t *testing.T) { t.Run("POST /login can log in as a user with just the local part of the id", func(t *testing.T) { t.Parallel() - res := unauthedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "login"}, json.RawMessage(`{ + res := unauthedClient.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "login"}, client.WithRawBody(json.RawMessage(`{ "type": "m.login.password", "identifier": { "type": "m.id.user", "user": "test_login_user" }, "password": "superuser" - }`)) + }`))) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ diff --git a/tests/csapi/sync_filter_test.go b/tests/csapi/sync_filter_test.go index ca99b57e..300f9bf1 100644 --- a/tests/csapi/sync_filter_test.go +++ b/tests/csapi/sync_filter_test.go @@ -44,7 +44,7 @@ func TestSyncFilter(t *testing.T) { t.Fatalf("failed to marshal JSON request body: %s", err) } filterID := createFilter(t, authedClient, reqBody, "@alice:hs1") - res := authedClient.MustDo(t, "GET", []string{"_matrix", "client", "v3", "user", "@alice:hs1", "filter", filterID}, nil) + res := authedClient.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", "@alice:hs1", "filter", filterID}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONKeyPresent("room"), @@ -57,7 +57,7 @@ func TestSyncFilter(t *testing.T) { func createFilter(t *testing.T, authedClient *client.CSAPI, reqBody []byte, userID string) string { t.Helper() - res := authedClient.MustDo(t, "POST", []string{"_matrix", "client", "v3", "user", userID, "filter"}, json.RawMessage(reqBody)) + res := authedClient.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "user", userID, "filter"}, client.WithRawBody(reqBody)) if res.StatusCode != 200 { t.Fatalf("MatchResponse got status %d want 200", res.StatusCode) } diff --git a/tests/federation_query_profile_test.go b/tests/federation_query_profile_test.go index 387b4455..7a292725 100644 --- a/tests/federation_query_profile_test.go +++ b/tests/federation_query_profile_test.go @@ -55,7 +55,7 @@ func TestOutboundFederationProfile(t *testing.T) { // query the display name which should do an outbound federation hit unauthedClient := deployment.Client(t, "hs1", "") - res := unauthedClient.MustDo(t, "GET", []string{"_matrix", "client", "v3", "profile", remoteUserID, "displayname"}, nil) + res := unauthedClient.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "profile", remoteUserID, "displayname"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONKeyEqual("displayname", remoteDisplayName), diff --git a/tests/knocking_test.go b/tests/knocking_test.go index 581e958c..6e8fb6f8 100644 --- a/tests/knocking_test.go +++ b/tests/knocking_test.go @@ -174,15 +174,13 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki _, since := knockingUser.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) // Rescind knock - knockingUser.MustDo( + knockingUser.MustDoFunc( t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "leave"}, - struct { - Reason string `json:"reason"` - }{ - "Just kidding!", - }, + client.WithJSONBody(t, map[string]interface{}{ + "reason": "Just kidding!", + }), ) // Use our sync token from earlier to carry out an incremental sync. Initial syncs may not contain room @@ -213,17 +211,14 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki // // In the case of federation, this test will still check that a knock can be // carried out after a previous knock is rejected. - inRoomUser.MustDo( + inRoomUser.MustDoFunc( t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, - struct { - UserID string `json:"user_id"` - Reason string `json:"reason"` - }{ - knockingUser.UserID, - "I don't think so", - }, + client.WithJSONBody(t, map[string]string{ + "user_id": knockingUser.UserID, + "reason": "I don't think so", + }), ) // Wait until the leave membership event has come down sync @@ -239,17 +234,14 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki t.Run("A user can knock on a room without a reason", func(t *testing.T) { // Reject the knock - inRoomUser.MustDo( + inRoomUser.MustDoFunc( t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, - struct { - UserID string `json:"user_id"` - Reason string `json:"reason"` - }{ - knockingUser.UserID, - "Please try again", - }, + client.WithJSONBody(t, map[string]string{ + "user_id": knockingUser.UserID, + "reason": "Please try again", + }), ) // Knock again, this time without a reason @@ -257,17 +249,14 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki }) t.Run("A user in the room can accept a knock", func(t *testing.T) { - inRoomUser.MustDo( + inRoomUser.MustDoFunc( t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "invite"}, - struct { - UserID string `json:"user_id"` - Reason string `json:"reason"` - }{ - knockingUser.UserID, - "Seems like a trustworthy fellow", - }, + client.WithJSONBody(t, map[string]string{ + "user_id": knockingUser.UserID, + "reason": "Seems like a trustworthy fellow", + }), ) // Wait until the invite membership event has come down sync @@ -289,17 +278,14 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki // // In the case of federation, this test will still check that a knock can not be // carried out after a ban. - inRoomUser.MustDo( + inRoomUser.MustDoFunc( t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "ban"}, - struct { - UserID string `json:"user_id"` - Reason string `json:"reason"` - }{ - knockingUser.UserID, - "Turns out Bob wasn't that trustworthy after all!", - }, + client.WithJSONBody(t, map[string]string{ + "user_id": knockingUser.UserID, + "reason": "Turns out Bob wasn't that trustworthy after all!", + }), ) // Wait until the ban membership event has come down sync @@ -423,15 +409,13 @@ func doTestKnockRoomsInPublicRoomsDirectory(t *testing.T, roomVersion string, jo // It will then query the directory and ensure the room is listed, and has a given 'join_rule' entry func publishAndCheckRoomJoinRule(t *testing.T, c *client.CSAPI, roomID, expectedJoinRule string) { // Publish the room to the public room directory - c.MustDo( + c.MustDoFunc( t, "PUT", []string{"_matrix", "client", "v3", "directory", "list", "room", roomID}, - struct { - Visibility string `json:"visibility"` - }{ - "public", - }, + client.WithJSONBody(t, map[string]string{ + "visibility": "public", + }), ) // Check that we can see the room in the directory diff --git a/tests/msc2836_test.go b/tests/msc2836_test.go index 62e8d450..20b191ad 100644 --- a/tests/msc2836_test.go +++ b/tests/msc2836_test.go @@ -335,11 +335,11 @@ func TestFederatedEventRelationships(t *testing.T) { })) // Hit /event_relationships to make sure it spiders the whole thing by asking /event_relationships on Complement - res := alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "event_relationships"}, map[string]interface{}{ + res := alice.MustDoFunc(t, "POST", []string{"_matrix", "client", "unstable", "event_relationships"}, client.WithJSONBody(t, map[string]interface{}{ "event_id": eventE.EventID(), "max_depth": 10, "direction": "up", - }) + })) var gotEventIDs []string must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ @@ -356,12 +356,12 @@ func TestFederatedEventRelationships(t *testing.T) { must.HaveInOrder(t, gotEventIDs, []string{eventE.EventID(), eventD.EventID(), eventC.EventID(), eventA.EventID()}) // now querying for the children of A should return A,B,C (it should've been remembered B from the previous /event_relationships request) - res = alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "event_relationships"}, map[string]interface{}{ + res = alice.MustDoFunc(t, "POST", []string{"_matrix", "client", "unstable", "event_relationships"}, client.WithJSONBody(t, map[string]interface{}{ "event_id": eventA.EventID(), "max_depth": 1, "direction": "down", "recent_first": false, - }) + })) gotEventIDs = []string{} must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ diff --git a/tests/restricted_room_hierarchy_test.go b/tests/restricted_room_hierarchy_test.go index 61b6f5ed..54231706 100644 --- a/tests/restricted_room_hierarchy_test.go +++ b/tests/restricted_room_hierarchy_test.go @@ -17,7 +17,7 @@ import ( func requestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, expected_rooms []interface{}) { t.Helper() - res := user.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", space, "hierarchy"}, map[string]interface{}{}) + res := user.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", space, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", expected_rooms, func(r gjson.Result) interface{} { diff --git a/tests/room_hierarchy_test.go b/tests/room_hierarchy_test.go index 041e5d0f..1d929ff2 100644 --- a/tests/room_hierarchy_test.go +++ b/tests/room_hierarchy_test.go @@ -219,7 +219,7 @@ func TestClientSpacesSummary(t *testing.T) { // - Rooms are returned correctly along with the custom fields `room_type`. // - Events are returned correctly. t.Run("query whole graph", func(t *testing.T) { - res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{ @@ -352,7 +352,7 @@ func TestClientSpacesSummary(t *testing.T) { StateKey: &ss1, Content: map[string]interface{}{}, }) - res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{ @@ -464,7 +464,7 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { bob := deployment.Client(t, "hs1", "@bob:hs1") bob.JoinRoom(t, root, []string{"hs1"}) - res := bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res := bob.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{ @@ -482,7 +482,7 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { alice.InviteRoom(t, r1, bob.UserID) alice.InviteRoom(t, r3, bob.UserID) - res = bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res = bob.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{ @@ -499,7 +499,7 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { // Invite to SS1 and it now appears, as well as the rooms under it. alice.InviteRoom(t, ss1, bob.UserID) - res = bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res = bob.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{ @@ -627,7 +627,7 @@ func TestFederatedClientSpaces(t *testing.T) { } t.Logf("rooms: %v", allEvents) - res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}, nil) + res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONCheckOff("rooms", []interface{}{