Replacing require.nil in web package (#16954)

* Replacing require.nil in web package

* Fixing tests

* Address PR review comments

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

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")
 	})
 }

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)

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)
 }

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)
 

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})
+		adminHook, appErr := th.App.CreateIncomingWebhookForChannel(th.SystemAdminUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id})
-		require.Nil(t, err)
+		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"}`))
+		response, err2 := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`))
-		require.Nil(t, appErr2)
+		require.NoError(t, err2)
 		require.Equal(t, http.StatusOK, response.StatusCode)
 	}