From 074a8e5fd9392acbfbdab6a53aedb1fbe4b3199f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Fri, 26 Feb 2021 07:30:22 +0100 Subject: [PATCH] Replacing require.nil in web package (#16954) * Replacing require.nil in web package * Fixing tests * Address PR review comments Co-authored-by: Mattermod --- web/context_test.go | 2 +- web/oauth_test.go | 6 +-- web/response_writer_wrapper_test.go | 4 +- web/web_test.go | 4 +- web/webhook_test.go | 70 ++++++++++++++--------------- 5 files changed, 43 insertions(+), 43 deletions(-) diff --git a/web/context_test.go b/web/context_test.go index 4a6ced348b..74239819c7 100644 --- a/web/context_test.go +++ b/web/context_test.go @@ -29,7 +29,7 @@ func TestRequireHookId(t *testing.T) { c.Params = &Params{HookId: "abc"} c.RequireHookId() - require.Error(t, c.Err, "Should have set Error in context") + require.NotNil(t, c.Err, "Should have set Error in context") require.Equal(t, http.StatusBadRequest, c.Err.StatusCode, "Should have set status as 400") }) } diff --git a/web/oauth_test.go b/web/oauth_test.go index 5c28a7a48d..58321c63b8 100644 --- a/web/oauth_test.go +++ b/web/oauth_test.go @@ -98,7 +98,7 @@ func TestAuthorizeOAuthApp(t *testing.T) { ru, _ = url.Parse(ruri) require.NotNil(t, ru, "redirect url unparseable") values, err := url.ParseQuery(ru.Fragment) - require.Nil(t, err) + require.NoError(t, err) assert.False(t, values.Get("access_token") == "", "access_token not returned") assert.Equal(t, authRequest.State, values.Get("state"), "returned state doesn't match") @@ -350,7 +350,7 @@ func TestOAuthAccessToken(t *testing.T) { authData := &model.AuthData{ClientId: oauthApp.Id, RedirectUri: oauthApp.CallbackUrls[0], UserId: th.BasicUser.Id, Code: model.NewId(), ExpiresIn: -1} _, nErr := th.App.Srv().Store.OAuth().SaveAuthData(authData) - require.Nil(t, nErr) + require.NoError(t, nErr) data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) data.Set("client_id", oauthApp.Id) @@ -488,7 +488,7 @@ func TestOAuthComplete(t *testing.T) { _, nErr := th.App.Srv().Store.User().UpdateAuthData( th.BasicUser.Id, model.SERVICE_GITLAB, &th.BasicUser.Email, th.BasicUser.Email, true) - require.Nil(t, nErr) + require.NoError(t, nErr) redirect, resp = ApiClient.AuthorizeOAuthApp(authRequest) require.Nil(t, resp.Error) diff --git a/web/response_writer_wrapper_test.go b/web/response_writer_wrapper_test.go index 0533a9105a..1af4b5497e 100644 --- a/web/response_writer_wrapper_test.go +++ b/web/response_writer_wrapper_test.go @@ -59,7 +59,7 @@ func TestForUnsupportedHijack(t *testing.T) { req := httptest.NewRequest("GET", "/api/v4/test", nil) handler := TestHandler{func(w http.ResponseWriter, r *http.Request) { _, _, err := w.(*responseWriterWrapper).Hijack() - assert.NotNil(t, err) + assert.Error(t, err) assert.Equal(t, "Hijacker interface not supported by the wrapped ResponseWriter", err.Error()) }} handler.ServeHTTP(resp, req) @@ -70,7 +70,7 @@ func TestForSupportedHijack(t *testing.T) { req := httptest.NewRequest("GET", "/api/v4/test", nil) handler := TestHandler{func(w http.ResponseWriter, r *http.Request) { _, _, err := w.(*responseWriterWrapper).Hijack() - assert.Nil(t, err) + assert.NoError(t, err) }} handler.ServeHTTP(resp, req) } diff --git a/web/web_test.go b/web/web_test.go index 4189b4441f..d22fa36fd3 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -211,7 +211,7 @@ func TestStaticFilesRequest(t *testing.T) { // Activate the plugin manifest, activated, reterr := th.App.GetPluginsEnvironment().Activate(pluginID) - require.Nil(t, reterr) + require.NoError(t, reterr) require.NotNil(t, manifest) require.True(t, activated) @@ -306,7 +306,7 @@ func TestPublicFilesRequest(t *testing.T) { assert.NoError(t, htmlFileErr) manifest, activated, reterr := env.Activate(pluginID) - require.Nil(t, reterr) + require.NoError(t, reterr) require.NotNil(t, manifest) require.True(t, activated) diff --git a/web/webhook_test.go b/web/webhook_test.go index c8aed89a5a..a773d82da0 100644 --- a/web/webhook_test.go +++ b/web/webhook_test.go @@ -22,7 +22,7 @@ func TestIncomingWebhook(t *testing.T) { if !*th.App.Config().ServiceSettings.EnableIncomingWebhooks { _, err := http.Post(ApiClient.Url+"/hooks/123", "", strings.NewReader("123")) - assert.NotNil(t, err, "should have errored - webhooks turned off") + assert.Error(t, err, "should have errored - webhooks turned off") return } @@ -39,93 +39,93 @@ func TestIncomingWebhook(t *testing.T) { t.Run("WebhookBasics", func(t *testing.T) { payload := "payload={\"text\": \"test text\"}" resp, err := http.Post(url, "application/x-www-form-urlencoded", strings.NewReader(payload)) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) payload = "payload={\"text\": \"\"}" resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader(payload)) - require.Nil(t, err) + require.NoError(t, err) assert.NotEqual(t, http.StatusOK, resp.StatusCode, "should have errored - no text post") payload = "payload={\"text\": \"test text\", \"channel\": \"junk\"}" resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader(payload)) - require.Nil(t, err) + require.NoError(t, err) assert.NotEqual(t, http.StatusOK, resp.StatusCode, "should have errored - bad channel") payload = "payload={\"text\": \"test text\"}" resp, err = http.Post(ApiClient.Url+"/hooks/abc123", "application/x-www-form-urlencoded", strings.NewReader(payload)) - require.Nil(t, err) + require.NoError(t, err) assert.NotEqual(t, http.StatusOK, resp.StatusCode, "should have errored - bad hook") resp, err = http.Post(url, "application/json", strings.NewReader("{\"text\":\"this is a test\"}")) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) text := `this is a \"test\" that contains a newline and a tab` resp, err = http.Post(url, "application/json", strings.NewReader("{\"text\":\""+text+"\"}")) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", th.BasicChannel.Name))) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"#%s\"}", th.BasicChannel.Name))) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"@%s\"}", th.BasicUser.Username))) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader("payload={\"text\":\"this is a test\"}")) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "AppLicaTion/x-www-Form-urlencoded", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded;charset=utf-8", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded; charset=utf-8", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded wrongtext", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, resp.StatusCode) resp, err = http.Post(url, "application/json", strings.NewReader("{\"text\":\""+tooLongText+"\"}")) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader("{\"text\":\""+tooLongText+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, resp.StatusCode) resp, err = http.Post(url, "application/json", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, resp.StatusCode) payloadMultiPart := "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"username\"\r\n\r\nwebhook-bot\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"text\"\r\n\r\nthis is a test :tada:\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW--" resp, err = http.Post(ApiClient.Url+"/hooks/"+hook.Id, "multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW", strings.NewReader(payloadMultiPart)) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) resp, err = http.Post(url, "mimetype/wrong", strings.NewReader("payload={\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, resp.StatusCode) resp, err = http.Post(url, "", strings.NewReader("{\"text\":\""+text+"\"}")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) }) @@ -135,21 +135,21 @@ func TestIncomingWebhook(t *testing.T) { // Read only default channel should fail. resp, err := http.Post(url, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", model.DEFAULT_CHANNEL))) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode != http.StatusOK) // None-default channel should still work. resp, err = http.Post(url, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", th.BasicChannel.Name))) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode == http.StatusOK) // System-Admin Owned Hook - adminHook, err := th.App.CreateIncomingWebhookForChannel(th.SystemAdminUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) - require.Nil(t, err) + adminHook, appErr := th.App.CreateIncomingWebhookForChannel(th.SystemAdminUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) + require.Nil(t, appErr) adminUrl := ApiClient.Url + "/hooks/" + adminHook.Id resp, err = http.Post(adminUrl, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", model.DEFAULT_CHANNEL))) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode == http.StatusOK) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = false }) @@ -190,7 +190,7 @@ func TestIncomingWebhook(t *testing.T) { }` resp, err := http.Post(url, "application/json", strings.NewReader(attachmentPayload)) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode == http.StatusOK) attachmentPayload = `{ @@ -227,7 +227,7 @@ func TestIncomingWebhook(t *testing.T) { }` resp, err = http.Post(url, "application/json", strings.NewReader(attachmentPayload)) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode == http.StatusOK) }) @@ -243,22 +243,22 @@ func TestIncomingWebhook(t *testing.T) { payload := "payload={\"text\": \"test text\"}" resp, err2 := http.Post(apiHookUrl, "application/x-www-form-urlencoded", strings.NewReader(payload)) - require.Nil(t, err2) + require.NoError(t, err2) assert.True(t, resp.StatusCode == http.StatusOK) resp, err2 = http.Post(apiHookUrl, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", th.BasicChannel.Name))) - require.Nil(t, err2) + require.NoError(t, err2) assert.True(t, resp.StatusCode == http.StatusOK) resp, err2 = http.Post(apiHookUrl, "application/json", strings.NewReader(fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", channel.Name))) - require.Nil(t, err2) + require.NoError(t, err2) assert.True(t, resp.StatusCode == http.StatusForbidden) }) t.Run("DisableWebhooks", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = false }) resp, err := http.Post(url, "application/json", strings.NewReader("{\"text\":\"this is a test\"}")) - require.Nil(t, err) + require.NoError(t, err) assert.True(t, resp.StatusCode == http.StatusNotImplemented) }) } @@ -293,8 +293,8 @@ func TestCommandWebhooks(t *testing.T) { assert.Equal(t, http.StatusBadRequest, resp.StatusCode) for i := 0; i < 5; i++ { - response, appErr2 := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)) - require.Nil(t, appErr2) + response, err2 := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)) + require.NoError(t, err2) require.Equal(t, http.StatusOK, response.StatusCode) }