Merge branch 'master' into mark-as-unread

.circleci/config.yml

@@ -66,8 +66,8 @@ jobs:
             cd mattermost-server
             make config-reset
             make check-style BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}'
-            GOFLAGS=-p=8 make build BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}'
+            make build BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}'
-            GOFLAGS=-p=8 make package BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}'
+            make package BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}'
       - store_artifacts:
           path: /go/src/github.com/mattermost/mattermost-server/dist/mattermost-team-linux-amd64.tar.gz
       - store_artifacts:

Makefile

@@ -89,7 +89,7 @@ PLUGIN_PACKAGES += mattermost-plugin-github-v0.11.0
 PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.1.1
 PLUGIN_PACKAGES += mattermost-plugin-aws-SNS-v1.0.2
 PLUGIN_PACKAGES += mattermost-plugin-antivirus-v0.1.1
-PLUGIN_PACKAGES += mattermost-plugin-jira-v2.2.1
+PLUGIN_PACKAGES += mattermost-plugin-jira-v2.2.2
 PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.0.1
 PLUGIN_PACKAGES += mattermost-plugin-jenkins-v1.0.0
 

README.md

@@ -46,7 +46,7 @@ Receive notifications of critical security updates. The sophistication of online
 
 ## Get Involved
 
-- [Contribute Code](http://docs.mattermost.com/developer/contribution-guide.html)
+- [Contribute Code](https://developers.mattermost.com/contribute/getting-started/)
 - [Find "Help Wanted" projects](https://github.com/mattermost/mattermost-server/issues?page=1&q=is%3Aissue+is%3Aopen+%22Help+Wanted%22&utf8=%E2%9C%93)
 - [Join Developer Discussion on a Mattermost Server for contributors](https://pre-release.mattermost.com/signup_user_complete/?id=f1924a8db44ff3bb41c96424cdc20676)
 - [File Bugs](http://www.mattermost.org/filing-issues/)

api4/system_test.go

@@ -383,7 +383,7 @@ func TestGetAnalyticsOld(t *testing.T) {
 	rows2, resp2 = th.SystemAdminClient.GetAnalyticsOld("standard", "")
 	CheckNoError(t, resp2)
 	assert.Equal(t, "total_websocket_connections", rows2[5].Name)
-	assert.Equal(t, float64(1), rows2[5].Value)
+	assert.Equal(t, float64(th.App.TotalWebsocketConnections()), rows2[5].Value)
 
 	WebSocketClient.Close()
 

app/channel.go

@@ -95,7 +95,7 @@ func (a *App) JoinDefaultChannels(teamId string, user *model.User, shouldBeAdmin
 
 		_, err = a.Srv.Store.Channel().SaveMember(cm)
 		if histErr := a.Srv.Store.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis()); histErr != nil {
-			mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", histErr))
+			mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(histErr))
 		}
 
 		if *a.Config().ServiceSettings.ExperimentalEnableDefaultChannelLeaveJoinMessages {
@@ -126,21 +126,21 @@ func (a *App) postJoinMessageForDefaultChannel(user *model.User, requestor *mode
 	if channel.Name == model.DEFAULT_CHANNEL {
 		if requestor == nil {
 			if err := a.postJoinTeamMessage(user, channel); err != nil {
-				mlog.Error(fmt.Sprint("Failed to post join/leave message", err))
+				mlog.Error("Failed to post join/leave message", mlog.Err(err))
 			}
 		} else {
 			if err := a.postAddToTeamMessage(requestor, user, channel, ""); err != nil {
-				mlog.Error(fmt.Sprint("Failed to post join/leave message", err))
+				mlog.Error("Failed to post join/leave message", mlog.Err(err))
 			}
 		}
 	} else {
 		if requestor == nil {
 			if err := a.postJoinChannelMessage(user, channel); err != nil {
-				mlog.Error(fmt.Sprint("Failed to post join/leave message", err))
+				mlog.Error("Failed to post join/leave message", mlog.Err(err))
 			}
 		} else {
 			if err := a.PostAddToChannelMessage(requestor, user, channel, ""); err != nil {
-				mlog.Error(fmt.Sprint("Failed to post join/leave message", err))
+				mlog.Error("Failed to post join/leave message", mlog.Err(err))
 			}
 		}
 	}
@@ -247,7 +247,7 @@ func (a *App) CreateChannel(channel *model.Channel, addMember bool) (*model.Chan
 			return nil, err
 		}
 		if err := a.Srv.Store.ChannelMemberHistory().LogJoinEvent(channel.CreatorId, sc.Id, model.GetMillis()); err != nil {
-			mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", err))
+			mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(err))
 		}
 
 		a.InvalidateCacheForUser(channel.CreatorId)
@@ -366,10 +366,10 @@ func (a *App) createDirectChannel(userId string, otherUserId string) (*model.Cha
 	}
 
 	if err = a.Srv.Store.ChannelMemberHistory().LogJoinEvent(userId, channel.Id, model.GetMillis()); err != nil {
-		mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", err))
+		mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(err))
 	}
 	if err = a.Srv.Store.ChannelMemberHistory().LogJoinEvent(otherUserId, channel.Id, model.GetMillis()); err != nil {
-		mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", err))
+		mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(err))
 	}
 
 	return channel, nil
@@ -399,7 +399,7 @@ func (a *App) WaitForChannelMembership(channelId string, userId string) {
 		}
 	}
 
-	mlog.Error(fmt.Sprintf("WaitForChannelMembership giving up channelId=%v userId=%v", channelId, userId), mlog.String("user_id", userId))
+	mlog.Error("WaitForChannelMembership giving up", mlog.String("channel_id", channelId), mlog.String("user_id", userId))
 }
 
 func (a *App) CreateGroupChannel(userIds []string, creatorId string) (*model.Channel, *model.AppError) {
@@ -477,7 +477,7 @@ func (a *App) createGroupChannel(userIds []string, creatorId string) (*model.Cha
 			return nil, err
 		}
 		if err := a.Srv.Store.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis()); err != nil {
-			mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", err))
+			mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(err))
 		}
 	}
 
@@ -861,21 +861,21 @@ func (a *App) DeleteChannel(channel *model.Channel, userId string) *model.AppErr
 		}
 
 		if _, err := a.CreatePost(post, channel, false); err != nil {
-			mlog.Error(fmt.Sprintf("Failed to post archive message %v", err))
+			mlog.Error("Failed to post archive message", mlog.Err(err))
 		}
 	}
 
 	now := model.GetMillis()
 	for _, hook := range incomingHooks {
 		if err := a.Srv.Store.Webhook().DeleteIncoming(hook.Id, now); err != nil {
-			mlog.Error(fmt.Sprintf("Encountered error deleting incoming webhook, id=%v", hook.Id))
+			mlog.Error("Encountered error deleting incoming webhook", mlog.String("hook_id", hook.Id), mlog.Err(err))
 		}
 		a.InvalidateCacheForWebhook(hook.Id)
 	}
 
 	for _, hook := range outgoingHooks {
 		if err := a.Srv.Store.Webhook().DeleteOutgoing(hook.Id, now); err != nil {
-			mlog.Error(fmt.Sprintf("Encountered error deleting outgoing webhook, id=%v", hook.Id))
+			mlog.Error("Encountered error deleting outgoing webhook", mlog.String("hook_id", hook.Id), mlog.Err(err))
 		}
 	}
 
@@ -926,13 +926,13 @@ func (a *App) addUserToChannel(user *model.User, channel *model.Channel, teamMem
 		SchemeUser:  !user.IsGuest(),
 	}
 	if _, err = a.Srv.Store.Channel().SaveMember(newMember); err != nil {
-		mlog.Error(fmt.Sprintf("Failed to add member user_id=%v channel_id=%v err=%v", user.Id, channel.Id, err), mlog.String("user_id", user.Id))
+		mlog.Error("Failed to add member", mlog.String("user_id", user.Id), mlog.String("channel_id", channel.Id), mlog.Err(err))
 		return nil, model.NewAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.app_error", nil, "", http.StatusInternalServerError)
 	}
 	a.WaitForChannelMembership(channel.Id, user.Id)
 
 	if err = a.Srv.Store.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis()); err != nil {
-		mlog.Warn(fmt.Sprintf("Failed to update ChannelMemberHistory table %v", err))
+		mlog.Warn("Failed to update ChannelMemberHistory table", mlog.Err(err))
 	}
 
 	a.InvalidateCacheForUser(user.Id)
@@ -1918,13 +1918,13 @@ func (a *App) MarkChannelsAsViewed(channelIds []string, userId string, currentSe
 		for _, channelId := range channelIds {
 			channel, errCh := a.Srv.Store.Channel().Get(channelId, true)
 			if errCh != nil {
-				mlog.Warn(fmt.Sprintf("Failed to get channel %v", errCh))
+				mlog.Warn("Failed to get channel", mlog.Err(errCh))
 				continue
 			}
 
 			member, err := a.Srv.Store.Channel().GetMember(channelId, userId)
 			if err != nil {
-				mlog.Warn(fmt.Sprintf("Failed to get membership %v", err))
+				mlog.Warn("Failed to get membership", mlog.Err(err))
 				continue
 			}
 

app/diagnostics.go

@@ -487,6 +487,7 @@ func (a *App) trackConfig() {
 		"isempty_group_filter":                   isDefault(*cfg.LdapSettings.GroupFilter, ""),
 		"isdefault_group_display_name_attribute": isDefault(*cfg.LdapSettings.GroupDisplayNameAttribute, model.LDAP_SETTINGS_DEFAULT_GROUP_DISPLAY_NAME_ATTRIBUTE),
 		"isdefault_group_id_attribute":           isDefault(*cfg.LdapSettings.GroupIdAttribute, model.LDAP_SETTINGS_DEFAULT_GROUP_ID_ATTRIBUTE),
+		"isempty_guest_filter":                   isDefault(*cfg.LdapSettings.GuestFilter, ""),
 	})
 
 	a.SendDiagnostic(TRACK_CONFIG_COMPLIANCE, map[string]interface{}{

app/email.go

@@ -347,13 +347,13 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, senderUserId
 			data := model.MapToJson(props)
 
 			if err := a.Srv.Store.Token().Save(token); err != nil {
-				mlog.Error(fmt.Sprintf("Failed to send invite email successfully err=%v", err))
+				mlog.Error("Failed to send invite email successfully ", mlog.Err(err))
 				continue
 			}
 			bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&t=%s", siteURL, url.QueryEscape(data), url.QueryEscape(token.Token))
 
 			if err := a.SendMail(invite, subject, bodyPage.Render()); err != nil {
-				mlog.Error(fmt.Sprintf("Failed to send invite email successfully err=%v", err))
+				mlog.Error("Failed to send invite email successfully ", mlog.Err(err))
 			}
 		}
 	}
@@ -435,24 +435,26 @@ func (a *App) SendGuestInviteEmails(team *model.Team, channels []*model.Channel,
 			data := model.MapToJson(props)
 
 			if err := a.Srv.Store.Token().Save(token); err != nil {
-				mlog.Error(fmt.Sprintf("Failed to send invite email successfully err=%v", err))
+				mlog.Error("Failed to send invite email successfully ", mlog.Err(err))
 				continue
 			}
 			bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&t=%s", siteURL, url.QueryEscape(data), url.QueryEscape(token.Token))
 
 			if !*a.Config().EmailSettings.SendEmailNotifications {
-				mlog.Info(fmt.Sprintf("sending invitation to %v %v", invite, bodyPage.Props["Link"]))
+				mlog.Info("sending invitation ", mlog.String("to", invite), mlog.String("link", bodyPage.Props["Link"].(string)))
 			}
 
 			embeddedFiles := make(map[string]io.Reader)
+			if message != "" {
 				if senderProfileImage != nil {
 					embeddedFiles = map[string]io.Reader{
 						"user-avatar.png": bytes.NewReader(senderProfileImage),
 					}
 				}
+			}
 
 			if err := a.SendMailWithEmbeddedFiles(invite, subject, bodyPage.Render(), embeddedFiles); err != nil {
-				mlog.Error(fmt.Sprintf("Failed to send invite email successfully err=%v", err))
+				mlog.Error("Failed to send invite email successfully", mlog.Err(err))
 			}
 		}
 	}

app/import_test.go

@@ -6,7 +6,6 @@ package app
 import (
 	"net/http"
 	"path/filepath"
-	"runtime/debug"
 	"strings"
 	"testing"
 
@@ -34,73 +33,43 @@ func ptrBool(b bool) *bool {
 }
 
 func checkPreference(t *testing.T, a *App, userId string, category string, name string, value string) {
-	if preferences, err := a.Srv.Store.Preference().GetCategory(userId, category); err != nil {
+	preferences, err := a.Srv.Store.Preference().GetCategory(userId, category)
-		debug.PrintStack()
+	require.Nilf(t, err, "Failed to get preferences for user %v with category %v", userId, category)
-		t.Fatalf("Failed to get preferences for user %v with category %v", userId, category)
-	} else {
 	found := false
 	for _, preference := range preferences {
 		if preference.Name == name {
 			found = true
-				if preference.Value != value {
+			require.Equal(t, preference.Value, value, "Preference for user %v in category %v with name %v has value %v, expected %v", userId, category, name, preference.Value, value)
-					debug.PrintStack()
-					t.Fatalf("Preference for user %v in category %v with name %v has value %v, expected %v", userId, category, name, preference.Value, value)
-				}
 			break
 		}
 	}
-		if !found {
+	require.Truef(t, found, "Did not find preference for user %v in category %v with name %v", userId, category, name)
-			debug.PrintStack()
-			t.Fatalf("Did not find preference for user %v in category %v with name %v", userId, category, name)
-		}
-	}
 }
 
 func checkNotifyProp(t *testing.T, user *model.User, key string, value string) {
-	if actual, ok := user.NotifyProps[key]; !ok {
+	actual, ok := user.NotifyProps[key]
-		debug.PrintStack()
+	require.True(t, ok, "Notify prop %v not found. User: %v", key, user.Id)
-		t.Fatalf("Notify prop %v not found. User: %v", key, user.Id)
+	require.Equalf(t, actual, value, "Notify Prop %v was %v but expected %v. User: %v", key, actual, value, user.Id)
-	} else if actual != value {
-		debug.PrintStack()
-		t.Fatalf("Notify Prop %v was %v but expected %v. User: %v", key, actual, value, user.Id)
-	}
 }
 
 func checkError(t *testing.T, err *model.AppError) {
-	if err == nil {
+	require.NotNil(t, err, "Should have returned an error.")
-		debug.PrintStack()
-		t.Fatal("Should have returned an error.")
-	}
 }
 
 func checkNoError(t *testing.T, err *model.AppError) {
-	if err != nil {
+	require.Nil(t, err, "Unexpected Error: %v", err)
-		debug.PrintStack()
-		t.Fatalf("Unexpected Error: %v", err.Error())
-	}
 }
 
 func AssertAllPostsCount(t *testing.T, a *App, initialCount int64, change int64, teamName string) {
-	if result, err := a.Srv.Store.Post().AnalyticsPostCount(teamName, false, false); err != nil {
+	result, err := a.Srv.Store.Post().AnalyticsPostCount(teamName, false, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	require.Equal(t, initialCount+change, result, "Did not find the expected number of posts.")
-		if initialCount+change != result {
-			debug.PrintStack()
-			t.Fatalf("Did not find the expected number of posts.")
-		}
-	}
 }
 
 func AssertChannelCount(t *testing.T, a *App, channelType string, expectedCount int64) {
-	if count, err := a.Srv.Store.Channel().AnalyticsTypeCount("", channelType); err == nil {
+	count, err := a.Srv.Store.Channel().AnalyticsTypeCount("", channelType)
-		if count != expectedCount {
+	require.Equalf(t, expectedCount, count, "Channel count of type: %v. Expected: %v, Got: %v", channelType, expectedCount, count)
-			debug.PrintStack()
+	require.Nil(t, err, "Failed to get channel count.")
-			t.Fatalf("Channel count of type: %v. Expected: %v, Got: %v", channelType, expectedCount, count)
-		}
-	} else {
-		debug.PrintStack()
-		t.Fatalf("Failed to get channel count.")
-	}
 }
 
 func TestImportImportLine(t *testing.T) {
@@ -112,51 +81,43 @@ func TestImportImportLine(t *testing.T) {
 		Type: "gibberish",
 	}
 
-	if err := th.App.ImportLine(line, false); err == nil {
+	err := th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with invalid type.")
+	require.NotNil(t, err, "Expected an error when importing a line with invalid type.")
-	}
 
 	// Try import line with team type but nil team.
 	line.Type = "team"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line of type team with a nil team.")
+	require.NotNil(t, err, "Expected an error when importing a line of type team with a nil team.")
-	}
 
 	// Try import line with channel type but nil channel.
 	line.Type = "channel"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type channel with a nil channel.")
+	require.NotNil(t, err, "Expected an error when importing a line with type channel with a nil channel.")
-	}
 
 	// Try import line with user type but nil user.
 	line.Type = "user"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type uesr with a nil user.")
+	require.NotNil(t, err, "Expected an error when importing a line with type user with a nil user.")
-	}
 
 	// Try import line with post type but nil post.
 	line.Type = "post"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type post with a nil post.")
+	require.NotNil(t, err, "Expected an error when importing a line with type post with a nil post.")
-	}
 
 	// Try import line with direct_channel type but nil direct_channel.
 	line.Type = "direct_channel"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type direct_channel with a nil direct_channel.")
+	require.NotNil(t, err, "Expected an error when importing a line with type direct_channel with a nil direct_channel.")
-	}
 
 	// Try import line with direct_post type but nil direct_post.
 	line.Type = "direct_post"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type direct_post with a nil direct_post.")
+	require.NotNil(t, err, "Expected an error when importing a line with type direct_post with a nil direct_post.")
-	}
 
 	// Try import line with scheme type but nil scheme.
 	line.Type = "scheme"
-	if err := th.App.ImportLine(line, false); err == nil {
+	err = th.App.ImportLine(line, false)
-		t.Fatalf("Expected an error when importing a line with type scheme with a nil scheme.")
+	require.NotNil(t, err, "Expected an error when importing a line with type scheme with a nil scheme.")
-	}
 }
 
 func TestStopOnError(t *testing.T) {
@@ -208,24 +169,24 @@ func TestImportBulkImport(t *testing.T) {
 {"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username2 + `", "` + username3 + `"], "user": "` + username + `", "message": "Hello Group Channel", "create_at": 123456789015}}
 {"type": "emoji", "emoji": {"name": "` + emojiName + `", "image": "` + testImage + `"}}`
 
-	if err, line := th.App.BulkImport(strings.NewReader(data1), false, 2); err != nil || line != 0 {
+	err, line := th.App.BulkImport(strings.NewReader(data1), false, 2)
-		t.Fatalf("BulkImport should have succeeded: %v, %v", err.Error(), line)
+	require.Nil(t, err, "BulkImport should have succeeded")
-	}
+	require.Equal(t, 0, line, "BulkImport line should be 0")
 
 	// Run bulk import using a string that contains a line with invalid json.
 	data2 := `{"type": "version", "version": 1`
-	if err, line := th.App.BulkImport(strings.NewReader(data2), false, 2); err == nil || line != 1 {
+	err, line = th.App.BulkImport(strings.NewReader(data2), false, 2)
-		t.Fatalf("Should have failed due to invalid JSON on line 1.")
+	require.NotNil(t, err, "Should have failed due to invalid JSON on line 1.")
-	}
+	require.Equal(t, 1, line, "Should have failed due to invalid JSON on line 1.")
 
 	// Run bulk import using valid JSON but missing version line at the start.
 	data3 := `{"type": "team", "team": {"type": "O", "display_name": "lskmw2d7a5ao7ppwqh5ljchvr4", "name": "` + teamName + `"}}
 {"type": "channel", "channel": {"type": "O", "display_name": "xr6m6udffngark2uekvr3hoeny", "team": "` + teamName + `", "name": "` + channelName + `"}}
 {"type": "user", "user": {"username": "kufjgnkxkrhhfgbrip6qxkfsaa", "email": "kufjgnkxkrhhfgbrip6qxkfsaa@example.com"}}
 {"type": "user", "user": {"username": "bwshaim6qnc2ne7oqkd5b2s2rq", "email": "bwshaim6qnc2ne7oqkd5b2s2rq@example.com", "teams": [{"name": "` + teamName + `", "channels": [{"name": "` + channelName + `"}]}]}}`
-	if err, line := th.App.BulkImport(strings.NewReader(data3), false, 2); err == nil || line != 1 {
+	err, line = th.App.BulkImport(strings.NewReader(data3), false, 2)
-		t.Fatalf("Should have failed due to missing version line on line 1.")
+	require.NotNil(t, err, "Should have failed due to missing version line on line 1.")
-	}
+	require.Equal(t, 1, line, "Should have failed due to missing version line on line 1.")
 
 	t.Run("First item after version without type", func(t *testing.T) {
 		data := `{"type": "version", "version": 1}
@@ -241,20 +202,18 @@ func TestImportProcessImportDataFileVersionLine(t *testing.T) {
 		Type:    "version",
 		Version: ptrInt(1),
 	}
-	if version, err := processImportDataFileVersionLine(data); err != nil || version != 1 {
+	version, err := processImportDataFileVersionLine(data)
-		t.Fatalf("Expected no error and version 1.")
+	require.Nil(t, err, "Expected no error")
-	}
+	require.Equal(t, 1, version, "Expected version 1")
 
 	data.Type = "NotVersion"
-	if _, err := processImportDataFileVersionLine(data); err == nil {
+	_, err = processImportDataFileVersionLine(data)
-		t.Fatalf("Expected error on invalid version line.")
+	require.NotNil(t, err, "Expected error on invalid version line.")
-	}
 
 	data.Type = "version"
 	data.Version = nil
-	if _, err := processImportDataFileVersionLine(data); err == nil {
+	_, err = processImportDataFileVersionLine(data)
-		t.Fatalf("Expected error on invalid version line.")
+	require.NotNil(t, err, "Expected error on invalid version line.")
-	}
 }
 
 func GetAttachments(userId string, th *TestHelper, t *testing.T) []*model.FileInfo {
@@ -267,12 +226,11 @@ func AssertFileIdsInPost(files []*model.FileInfo, th *TestHelper, t *testing.T)
 	postId := files[0].PostId
 	assert.NotNil(t, postId)
 
-	if posts, err := th.App.Srv.Store.Post().GetPostsByIds([]string{postId}); err != nil {
+	posts, err := th.App.Srv.Store.Post().GetPostsByIds([]string{postId})
-		t.Fatal(err.Error())
+	require.Nil(t, err)
-	} else {
+
 	assert.Equal(t, len(posts), 1)
 	for _, file := range files {
 		assert.Contains(t, posts[0].FileIds, file.Id)
 	}
-	}
 }

cmd/mattermost/commands/sampledata.go

@@ -17,9 +17,15 @@ import (
 
 	"github.com/icrowley/fake"
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/model"
 	"github.com/spf13/cobra"
 )
 
+const (
+	DEACTIVATED_USER = "deactivated"
+	GUEST_USER       = "guest"
+)
+
 var SampleDataCmd = &cobra.Command{
 	Use:   "sampledata",
 	Short: "Generate sample data",
@@ -32,6 +38,7 @@ func init() {
 	SampleDataCmd.Flags().Int("channels-per-team", 10, "The number of sample channels per team.")
 	SampleDataCmd.Flags().IntP("users", "u", 15, "The number of sample users.")
 	SampleDataCmd.Flags().IntP("guests", "g", 1, "The number of sample guests.")
+	SampleDataCmd.Flags().Int("deactivated-users", 0, "The number of deactivated users.")
 	SampleDataCmd.Flags().Int("team-memberships", 2, "The number of sample team memberships per user.")
 	SampleDataCmd.Flags().Int("channel-memberships", 5, "The number of sample channel memberships per user in a team.")
 	SampleDataCmd.Flags().Int("posts-per-channel", 100, "The number of sample post per channel.")
@@ -165,6 +172,10 @@ func sampleDataCmdF(command *cobra.Command, args []string) error {
 	if err != nil || users < 0 {
 		return errors.New("Invalid users parameter")
 	}
+	deactivatedUsers, err := command.Flags().GetInt("deactivated-users")
+	if err != nil || deactivatedUsers < 0 {
+		return errors.New("Invalid deactivated-users parameter")
+	}
 	guests, err := command.Flags().GetInt("guests")
 	if err != nil || guests < 0 {
 		return errors.New("Invalid guests parameter")
@@ -283,12 +294,17 @@ func sampleDataCmdF(command *cobra.Command, args []string) error {
 
 	allUsers := []string{}
 	for i := 0; i < users; i++ {
-		userLine := createUser(i, teamMemberships, channelMemberships, teamsAndChannels, profileImages, false)
+		userLine := createUser(i, teamMemberships, channelMemberships, teamsAndChannels, profileImages, "")
 		encoder.Encode(userLine)
 		allUsers = append(allUsers, *userLine.User.Username)
 	}
 	for i := 0; i < guests; i++ {
-		userLine := createUser(i, teamMemberships, channelMemberships, teamsAndChannels, profileImages, true)
+		userLine := createUser(i, teamMemberships, channelMemberships, teamsAndChannels, profileImages, GUEST_USER)
+		encoder.Encode(userLine)
+		allUsers = append(allUsers, *userLine.User.Username)
+	}
+	for i := 0; i < deactivatedUsers; i++ {
+		userLine := createUser(i, teamMemberships, channelMemberships, teamsAndChannels, profileImages, DEACTIVATED_USER)
 		encoder.Encode(userLine)
 		allUsers = append(allUsers, *userLine.User.Username)
 	}
@@ -355,23 +371,34 @@ func sampleDataCmdF(command *cobra.Command, args []string) error {
 	return nil
 }
 
-func createUser(idx int, teamMemberships int, channelMemberships int, teamsAndChannels map[string][]string, profileImages []string, guest bool) app.LineImportData {
+func createUser(idx int, teamMemberships int, channelMemberships int, teamsAndChannels map[string][]string, profileImages []string, userType string) app.LineImportData {
-	password := fmt.Sprintf("SampleUs@r-%d", idx)
-	email := fmt.Sprintf("user-%d@sample.mattermost.com", idx)
-	if guest {
-		password = fmt.Sprintf("SampleGu@st-%d", idx)
-		email = fmt.Sprintf("guest-%d@sample.mattermost.com", idx)
-	}
 	firstName := fake.FirstName()
 	lastName := fake.LastName()
+	position := fake.JobTitle()
+
 	username := fmt.Sprintf("%s.%s", strings.ToLower(firstName), strings.ToLower(lastName))
-	if guest {
+	roles := "system_user"
+
+	var password string
+	var email string
+
+	switch userType {
+	case GUEST_USER:
+		password = fmt.Sprintf("SampleGu@st-%d", idx)
+		email = fmt.Sprintf("guest-%d@sample.mattermost.com", idx)
+		roles = "system_guest"
 		if idx == 0 {
 			username = "guest"
 			password = "SampleGu@st1"
 			email = "guest@sample.mattermost.com"
 		}
-	} else if idx == 0 {
+	case DEACTIVATED_USER:
+		password = fmt.Sprintf("SampleDe@ctivated-%d", idx)
+		email = fmt.Sprintf("deactivated-%d@sample.mattermost.com", idx)
+	default:
+		password = fmt.Sprintf("SampleUs@r-%d", idx)
+		email = fmt.Sprintf("user-%d@sample.mattermost.com", idx)
+		if idx == 0 {
 			username = "sysadmin"
 			password = "Sys@dmin-sample1"
 			email = "sysadmin@sample.mattermost.com"
@@ -379,13 +406,10 @@ func createUser(idx int, teamMemberships int, channelMemberships int, teamsAndCh
 			username = "user-1"
 		}
 
-	position := fake.JobTitle()
+		if idx%5 == 0 {
-	roles := "system_user"
-	if guest {
-		roles = "system_guest"
-	} else if idx%5 == 0 {
 			roles = "system_admin system_user"
 		}
+	}
 
 	// The 75% of the users have custom profile image
 	var profileImage *string = nil
@@ -450,10 +474,15 @@ func createUser(idx int, teamMemberships int, channelMemberships int, teamsAndCh
 		team := possibleTeams[position]
 		possibleTeams = append(possibleTeams[:position], possibleTeams[position+1:]...)
 		if teamChannels, err := teamsAndChannels[team]; err {
-			teams = append(teams, createTeamMembership(channelMemberships, teamChannels, &team, guest))
+			teams = append(teams, createTeamMembership(channelMemberships, teamChannels, &team, userType == GUEST_USER))
 		}
 	}
 
+	var deleteAt int64
+	if userType == DEACTIVATED_USER {
+		deleteAt = model.GetMillis()
+	}
+
 	user := app.UserImportData{
 		ProfileImage:       profileImage,
 		Username:           &username,
@@ -470,6 +499,7 @@ func createUser(idx int, teamMemberships int, channelMemberships int, teamsAndCh
 		MessageDisplay:     &messageDisplay,
 		ChannelDisplayMode: &channelDisplayMode,
 		TutorialStep:       &tutorialStep,
+		DeleteAt:           &deleteAt,
 	}
 	return app.LineImportData{
 		Type: "user",

config/unmarshal_test.go

@@ -161,19 +161,17 @@ func TestConfigFromEnviroVars(t *testing.T) {
 		assert.Equal(t, "From Environment", *cfg.TeamSettings.SiteName)
 		assert.Equal(t, "Custom Brand", *cfg.TeamSettings.CustomBrandText)
 
-		if teamSettings, ok := envCfg["TeamSettings"]; !ok {
+		teamSettings, ok := envCfg["TeamSettings"]
-			t.Fatal("TeamSettings is missing from envConfig")
+		require.True(t, ok, "TeamSettings is missing from envConfig")
-		} else if teamSettingsAsMap, ok := teamSettings.(map[string]interface{}); !ok {
-			t.Fatal("TeamSettings is not a map in envConfig")
-		} else {
-			if siteNameInEnv, ok := teamSettingsAsMap["SiteName"].(bool); !ok || !siteNameInEnv {
-				t.Fatal("SiteName should be in envConfig")
-			}
 
-			if customBrandTextInEnv, ok := teamSettingsAsMap["CustomBrandText"].(bool); !ok || !customBrandTextInEnv {
+		teamSettingsAsMap, ok := teamSettings.(map[string]interface{})
-				t.Fatal("SiteName should be in envConfig")
+		require.True(t, ok, "TeamSettings is not a map in envConfig")
-			}
+
-		}
+		siteNameInEnv, ok := teamSettingsAsMap["SiteName"].(bool)
+		require.True(t, ok || siteNameInEnv, "SiteName should be in envConfig")
+
+		customBrandTextInEnv, ok := teamSettingsAsMap["CustomBrandText"].(bool)
+		require.True(t, ok || customBrandTextInEnv, "SiteName should be in envConfig")
 
 		os.Unsetenv("MM_TEAMSETTINGS_SITENAME")
 		os.Unsetenv("MM_TEAMSETTINGS_CUSTOMBRANDTEXT")
@@ -183,9 +181,8 @@ func TestConfigFromEnviroVars(t *testing.T) {
 
 		assert.Equal(t, "Mattermost", *cfg.TeamSettings.SiteName)
 
-		if _, ok := envCfg["TeamSettings"]; ok {
+		_, ok = envCfg["TeamSettings"]
-			t.Fatal("TeamSettings should be missing from envConfig")
+		require.False(t, ok, "TeamSettings should be missing from envConfig")
-		}
 	})
 
 	t.Run("boolean setting", func(t *testing.T) {
@@ -195,19 +192,16 @@ func TestConfigFromEnviroVars(t *testing.T) {
 		cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true)
 		require.Nil(t, err)
 
-		if *cfg.ServiceSettings.EnableCommands {
+		require.False(t, *cfg.ServiceSettings.EnableCommands, "Couldn't read config from environment var")
-			t.Fatal("Couldn't read config from environment var")
-		}
 
-		if serviceSettings, ok := envCfg["ServiceSettings"]; !ok {
+		serviceSettings, ok := envCfg["ServiceSettings"]
-			t.Fatal("ServiceSettings is missing from envConfig")
+		require.True(t, ok, "ServiceSettings is missing from envConfig")
-		} else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok {
+
-			t.Fatal("ServiceSettings is not a map in envConfig")
+		serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{})
-		} else {
+		require.True(t, ok, "ServiceSettings is not a map in envConfig")
-			if enableCommandsInEnv, ok := serviceSettingsAsMap["EnableCommands"].(bool); !ok || !enableCommandsInEnv {
+
-				t.Fatal("EnableCommands should be in envConfig")
+		enableCommandsInEnv, ok := serviceSettingsAsMap["EnableCommands"].(bool)
-			}
+		require.True(t, ok || enableCommandsInEnv, "EnableCommands should be in envConfig")
-		}
 	})
 
 	t.Run("integer setting", func(t *testing.T) {
@@ -219,15 +213,14 @@ func TestConfigFromEnviroVars(t *testing.T) {
 
 		assert.Equal(t, 400, *cfg.ServiceSettings.ReadTimeout)
 
-		if serviceSettings, ok := envCfg["ServiceSettings"]; !ok {
+		serviceSettings, ok := envCfg["ServiceSettings"]
-			t.Fatal("ServiceSettings is missing from envConfig")
+		require.True(t, ok, "ServiceSettings is missing from envConfig")
-		} else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok {
+
-			t.Fatal("ServiceSettings is not a map in envConfig")
+		serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{})
-		} else {
+		require.True(t, ok, "ServiceSettings is not a map in envConfig")
-			if readTimeoutInEnv, ok := serviceSettingsAsMap["ReadTimeout"].(bool); !ok || !readTimeoutInEnv {
+
-				t.Fatal("ReadTimeout should be in envConfig")
+		readTimeoutInEnv, ok := serviceSettingsAsMap["ReadTimeout"].(bool)
-			}
+		require.True(t, ok || readTimeoutInEnv, "ReadTimeout should be in envConfig")
-		}
 	})
 
 	t.Run("setting missing from config.json", func(t *testing.T) {
@@ -239,15 +232,14 @@ func TestConfigFromEnviroVars(t *testing.T) {
 
 		assert.Equal(t, "https://example.com", *cfg.ServiceSettings.SiteURL)
 
-		if serviceSettings, ok := envCfg["ServiceSettings"]; !ok {
+		serviceSettings, ok := envCfg["ServiceSettings"]
-			t.Fatal("ServiceSettings is missing from envConfig")
+		require.True(t, ok, "ServiceSettings is missing from envConfig")
-		} else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok {
+
-			t.Fatal("ServiceSettings is not a map in envConfig")
+		serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{})
-		} else {
+		require.True(t, ok, "ServiceSettings is not a map in envConfig")
-			if siteURLInEnv, ok := serviceSettingsAsMap["SiteURL"].(bool); !ok || !siteURLInEnv {
+
-				t.Fatal("SiteURL should be in envConfig")
+		siteURLInEnv, ok := serviceSettingsAsMap["SiteURL"].(bool)
-			}
+		require.True(t, ok || siteURLInEnv, "SiteURL should be in envConfig")
-		}
 	})
 
 	t.Run("empty string setting", func(t *testing.T) {
@@ -259,15 +251,14 @@ func TestConfigFromEnviroVars(t *testing.T) {
 
 		assert.Empty(t, *cfg.SupportSettings.TermsOfServiceLink)
 
-		if supportSettings, ok := envCfg["SupportSettings"]; !ok {
+		supportSettings, ok := envCfg["SupportSettings"]
-			t.Fatal("SupportSettings is missing from envConfig")
+		require.True(t, ok, "SupportSettings is missing from envConfig")
-		} else if supportSettingsAsMap, ok := supportSettings.(map[string]interface{}); !ok {
+
-			t.Fatal("SupportSettings is not a map in envConfig")
+		supportSettingsAsMap, ok := supportSettings.(map[string]interface{})
-		} else {
+		require.True(t, ok, "SupportSettings is not a map in envConfig")
-			if termsOfServiceLinkInEnv, ok := supportSettingsAsMap["TermsOfServiceLink"].(bool); !ok || !termsOfServiceLinkInEnv {
+
-				t.Fatal("TermsOfServiceLink should be in envConfig")
+		termsOfServiceLinkInEnv, ok := supportSettingsAsMap["TermsOfServiceLink"].(bool)
-			}
+		require.True(t, ok || termsOfServiceLinkInEnv, "TermsOfServiceLink should be in envConfig")
-		}
 	})
 
 	t.Run("plugin directory settings", func(t *testing.T) {
@@ -285,18 +276,17 @@ func TestConfigFromEnviroVars(t *testing.T) {
 		assert.Equal(t, "/temp/plugins", *cfg.PluginSettings.Directory)
 		assert.Equal(t, "/temp/clientplugins", *cfg.PluginSettings.ClientDirectory)
 
-		if pluginSettings, ok := envCfg["PluginSettings"]; !ok {
+		pluginSettings, ok := envCfg["PluginSettings"]
-			t.Fatal("PluginSettings is missing from envConfig")
+		require.True(t, ok, "PluginSettings is missing from envConfig")
-		} else if pluginSettingsAsMap, ok := pluginSettings.(map[string]interface{}); !ok {
+
-			t.Fatal("PluginSettings is not a map in envConfig")
+		pluginSettingsAsMap, ok := pluginSettings.(map[string]interface{})
-		} else {
+		require.True(t, ok, "PluginSettings is not a map in envConfig")
-			if directory, ok := pluginSettingsAsMap["Directory"].(bool); !ok || !directory {
+
-				t.Fatal("Directory should be in envConfig")
+		directory, ok := pluginSettingsAsMap["Directory"].(bool)
-			}
+		require.True(t, ok || directory, "Directory should be in envConfig")
-			if clientDirectory, ok := pluginSettingsAsMap["ClientDirectory"].(bool); !ok || !clientDirectory {
+
-				t.Fatal("ClientDirectory should be in envConfig")
+		clientDirectory, ok := pluginSettingsAsMap["ClientDirectory"].(bool)
-			}
+		require.True(t, ok || clientDirectory, "ClientDirectory should be in envConfig")
-		}
 	})
 
 	t.Run("plugin specific settings cannot be overridden via environment", func(t *testing.T) {
@@ -310,45 +300,38 @@ func TestConfigFromEnviroVars(t *testing.T) {
 		cfg, envCfg, err := unmarshalConfig(strings.NewReader(config), true)
 		require.Nil(t, err)
 
-		if pluginsJira, ok := cfg.PluginSettings.Plugins["jira"]; !ok {
+		pluginsJira, ok := cfg.PluginSettings.Plugins["jira"]
-			t.Fatal("PluginSettings.Plugins.jira is missing from config")
+		require.True(t, ok, "PluginSettings.Plugins.jira is missing from config")
-		} else {
+
-			if enabled, ok := pluginsJira["enabled"]; !ok {
+		enabled, ok := pluginsJira["enabled"]
-				t.Fatal("PluginSettings.Plugins.jira.enabled is missing from config")
+		require.True(t, ok, "PluginSettings.Plugins.jira.enabled is missing from config")
-			} else {
 		assert.Equal(t, "true", enabled)
-			}
 
-			if secret, ok := pluginsJira["secret"]; !ok {
+		secret, ok := pluginsJira["secret"]
-				t.Fatal("PluginSettings.Plugins.jira.secret is missing from config")
+		require.True(t, ok, "PluginSettings.Plugins.jira.secret is missing from config")
-			} else {
 		assert.Equal(t, "config-secret", secret)
-			}
-		}
 
-		if pluginStatesJira, ok := cfg.PluginSettings.PluginStates["jira"]; !ok {
+		pluginStatesJira, ok := cfg.PluginSettings.PluginStates["jira"]
-			t.Fatal("PluginSettings.PluginStates.jira is missing from config")
+		require.True(t, ok, "PluginSettings.PluginStates.jira is missing from config")
-		} else {
 		require.Equal(t, true, pluginStatesJira.Enable)
-		}
 
-		if pluginSettings, ok := envCfg["PluginSettings"]; !ok {
+		pluginSettings, ok := envCfg["PluginSettings"]
-			t.Fatal("PluginSettings is missing from envConfig")
+		require.True(t, ok, "PluginSettings is missing from envConfig")
-		} else if pluginSettingsAsMap, ok := pluginSettings.(map[string]interface{}); !ok {
-			t.Fatal("PluginSettings is not a map in envConfig")
-		} else {
-			if plugins, ok := pluginSettingsAsMap["Plugins"].(map[string]interface{}); !ok {
-				t.Fatal("PluginSettings.Plugins is not a map in envConfig")
-			} else if _, ok := plugins["jira"].(map[string]interface{}); ok {
-				t.Fatal("PluginSettings.Plugins.jira should not be a map in envConfig")
-			}
 
-			if pluginStates, ok := pluginSettingsAsMap["PluginStates"].(map[string]interface{}); !ok {
+		pluginSettingsAsMap, ok := pluginSettings.(map[string]interface{})
-				t.Fatal("PluginSettings.PluginStates is missing from envConfig")
+		require.True(t, ok, "PluginSettings is not a map in envConfig")
-			} else if _, ok := pluginStates["jira"].(map[string]interface{}); ok {
+
-				t.Fatal("PluginSettings.PluginStates.jira should not be a map in envConfig")
+		plugins, ok := pluginSettingsAsMap["Plugins"].(map[string]interface{})
-			}
+		require.True(t, ok, "PluginSettings.Plugins is not a map in envConfig")
-		}
+
+		_, ok = plugins["jira"].(map[string]interface{})
+		require.False(t, ok, "PluginSettings.Plugins.jira should not be a map in envConfig")
+
+		pluginStates, ok := pluginSettingsAsMap["PluginStates"].(map[string]interface{})
+		require.True(t, ok, "PluginSettings.PluginStates is missing from envConfig")
+
+		_, ok = pluginStates["jira"].(map[string]interface{})
+		require.False(t, ok, "PluginSettings.PluginStates.jira should not be a map in envConfig")
 	})
 }
 

i18n/en.json

@@ -3954,6 +3954,10 @@
     "id": "ent.ldap.validate_filter.app_error",
     "translation": "Invalid AD/LDAP Filter"
   },
+  {
+    "id": "ent.ldap.validate_guest_filter.app_error",
+    "translation": "Invalid AD/LDAP Guest Filter"
+  },
   {
     "id": "ent.ldap_groups.group_search_error",
     "translation": "error retrieving ldap group"

model/channel_member_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestChannelMemberJson(t *testing.T) {
@@ -13,22 +15,16 @@ func TestChannelMemberJson(t *testing.T) {
 	json := o.ToJson()
 	ro := ChannelMemberFromJson(strings.NewReader(json))
 
-	if o.ChannelId != ro.ChannelId {
+	require.Equal(t, o.ChannelId, ro.ChannelId, "ids do not match")
-		t.Fatal("Ids do not match")
-	}
 }
 
 func TestChannelMemberIsValid(t *testing.T) {
 	o := ChannelMember{}
 
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.ChannelId = NewId()
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.NotifyProps = GetDefaultChannelNotifyProps()
 	o.UserId = NewId()
@@ -40,34 +36,22 @@ func TestChannelMemberIsValid(t *testing.T) {
 	}*/
 
 	o.NotifyProps["desktop"] = "junk"
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.NotifyProps["desktop"] = "123456789012345678901"
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.NotifyProps["desktop"] = CHANNEL_NOTIFY_ALL
-	if err := o.IsValid(); err != nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal(err)
-	}
 
 	o.NotifyProps["mark_unread"] = "123456789012345678901"
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal("should be invalid")
-	}
 
 	o.NotifyProps["mark_unread"] = CHANNEL_MARK_UNREAD_ALL
-	if err := o.IsValid(); err != nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal(err)
-	}
 
 	o.Roles = ""
-	if err := o.IsValid(); err != nil {
+	require.Error(t, o.IsValid(), "should be invalid")
-		t.Fatal(err)
-	}
 }
 
 func TestChannelUnreadJson(t *testing.T) {
@@ -75,11 +59,6 @@ func TestChannelUnreadJson(t *testing.T) {
 	json := o.ToJson()
 	ro := ChannelUnreadFromJson(strings.NewReader(json))
 
-	if o.TeamId != ro.TeamId {
+	require.Equal(t, o.TeamId, ro.TeamId, "team Ids do not match")
-		t.Fatal("Team Ids do not match")
+	require.Equal(t, o.MentionCount, ro.MentionCount, "mention count do not match")
-	}
-
-	if o.MentionCount != ro.MentionCount {
-		t.Fatal("MentionCount do not match")
-	}
 }

model/channel_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestChannelJson(t *testing.T) {
@@ -13,27 +15,21 @@ func TestChannelJson(t *testing.T) {
 	json := o.ToJson()
 	ro := ChannelFromJson(strings.NewReader(json))
 
-	if o.Id != ro.Id {
+	require.Equal(t, o.Id, ro.Id)
-		t.Fatal("Ids do not match")
-	}
 
 	p := ChannelPatch{Name: new(string)}
 	*p.Name = NewId()
 	json = p.ToJson()
 	rp := ChannelPatchFromJson(strings.NewReader(json))
 
-	if *p.Name != *rp.Name {
+	require.Equal(t, *p.Name, *rp.Name)
-		t.Fatal("names do not match")
-	}
 }
 
 func TestChannelCopy(t *testing.T) {
 	o := Channel{Id: NewId(), Name: NewId()}
 	ro := o.DeepCopy()
 
-	if o.Id != ro.Id {
+	require.Equal(t, o.Id, ro.Id, "Ids do not match")
-		t.Fatal("Ids do not match")
-	}
 }
 
 func TestChannelPatch(t *testing.T) {
@@ -47,97 +43,57 @@ func TestChannelPatch(t *testing.T) {
 	o := Channel{Id: NewId(), Name: NewId()}
 	o.Patch(p)
 
-	if *p.Name != o.Name {
+	require.Equal(t, *p.Name, o.Name)
-		t.Fatal("do not match")
+	require.Equal(t, *p.DisplayName, o.DisplayName)
-	}
+	require.Equal(t, *p.Header, o.Header)
-	if *p.DisplayName != o.DisplayName {
+	require.Equal(t, *p.Purpose, o.Purpose)
-		t.Fatal("do not match")
+	require.Equal(t, *p.GroupConstrained, *o.GroupConstrained)
-	}
-	if *p.Header != o.Header {
-		t.Fatal("do not match")
-	}
-	if *p.Purpose != o.Purpose {
-		t.Fatal("do not match")
-	}
-	if *p.GroupConstrained != *o.GroupConstrained {
-		t.Fatalf("expected %v got %v", *p.GroupConstrained, *o.GroupConstrained)
-	}
 }
 
 func TestChannelIsValid(t *testing.T) {
 	o := Channel{}
 
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.Id = NewId()
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.CreateAt = GetMillis()
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.UpdateAt = GetMillis()
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.DisplayName = strings.Repeat("01234567890", 20)
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.DisplayName = "1234"
 	o.Name = "ZZZZZZZ"
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.Name = "zzzzz"
-
+	require.Error(t, o.IsValid())
-	if err := o.IsValid(); err == nil {
-		t.Fatal("should be invalid")
-	}
 
 	o.Type = "U"
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.Type = "P"
-
+	require.Error(t, o.IsValid())
-	if err := o.IsValid(); err != nil {
-		t.Fatal(err)
-	}
 
 	o.Header = strings.Repeat("01234567890", 100)
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.Header = "1234"
-	if err := o.IsValid(); err != nil {
+	require.Nil(t, o.IsValid())
-		t.Fatal(err)
-	}
 
 	o.Purpose = strings.Repeat("01234567890", 30)
-	if err := o.IsValid(); err == nil {
+	require.Error(t, o.IsValid())
-		t.Fatal("should be invalid")
-	}
 
 	o.Purpose = "1234"
-	if err := o.IsValid(); err != nil {
+	require.Nil(t, o.IsValid())
-		t.Fatal(err)
-	}
 
 	o.Purpose = strings.Repeat("0123456789", 25)
-	if err := o.IsValid(); err != nil {
+	require.Nil(t, o.IsValid())
-		t.Fatal(err)
-	}
 }
 
 func TestChannelPreSave(t *testing.T) {
@@ -159,15 +115,11 @@ func TestGetGroupDisplayNameFromUsers(t *testing.T) {
 	users[3] = &User{Username: NewId()}
 
 	name := GetGroupDisplayNameFromUsers(users, true)
-	if len(name) > CHANNEL_NAME_MAX_LENGTH {
+	require.LessOrEqual(t, len(name), CHANNEL_NAME_MAX_LENGTH)
-		t.Fatal("name too long")
-	}
 }
 
 func TestGetGroupNameFromUserIds(t *testing.T) {
 	name := GetGroupNameFromUserIds([]string{NewId(), NewId(), NewId(), NewId(), NewId()})
 
-	if len(name) > CHANNEL_NAME_MAX_LENGTH {
+	require.LessOrEqual(t, len(name), CHANNEL_NAME_MAX_LENGTH)
-		t.Fatal("name too long")
-	}
 }

model/cluster_stats_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestClusterStatsJson(t *testing.T) {
@@ -13,7 +15,5 @@ func TestClusterStatsJson(t *testing.T) {
 	json := cluster.ToJson()
 	result := ClusterStatsFromJson(strings.NewReader(json))
 
-	if cluster.Id != result.Id {
+	require.Equal(t, cluster.Id, result.Id, "Ids do not match")
-		t.Fatal("Ids do not match")
-	}
 }

model/config.go

@@ -1687,6 +1687,7 @@ type LdapSettings struct {
 	// Filtering
 	UserFilter  *string
 	GroupFilter *string
+	GuestFilter *string
 
 	// Group Mapping
 	GroupDisplayNameAttribute *string
@@ -1758,6 +1759,10 @@ func (s *LdapSettings) SetDefaults() {
 		s.UserFilter = NewString("")
 	}
 
+	if s.GuestFilter == nil {
+		s.GuestFilter = NewString("")
+	}
+
 	if s.GroupFilter == nil {
 		s.GroupFilter = NewString("")
 	}
@@ -2784,6 +2789,12 @@ func (ls *LdapSettings) isValid() *AppError {
 				return NewAppError("ValidateFilter", "ent.ldap.validate_filter.app_error", nil, err.Error(), http.StatusBadRequest)
 			}
 		}
+
+		if *ls.GuestFilter != "" {
+			if _, err := ldap.CompileFilter(*ls.GuestFilter); err != nil {
+				return NewAppError("LdapSettings.isValid", "ent.ldap.validate_guest_filter.app_error", nil, err.Error(), http.StatusBadRequest)
+			}
+		}
 	}
 
 	return nil

model/config_test.go

@@ -1018,6 +1018,105 @@ func TestLdapSettingsIsValid(t *testing.T) {
 			},
 			ExpectError: true,
 		},
+
+		{
+			Name: "valid guest filter #1",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("(property=value)"),
+			},
+			ExpectError: false,
+		},
+		{
+			Name: "invalid guest filter #1",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("("),
+			},
+			ExpectError: true,
+		},
+		{
+			Name: "invalid guest filter #2",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("()"),
+			},
+			ExpectError: true,
+		},
+		{
+			Name: "valid guest filter #2",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("(&(property=value)(otherthing=othervalue))"),
+			},
+			ExpectError: false,
+		},
+		{
+			Name: "valid guest filter #3",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("(&(property=value)(|(otherthing=othervalue)(other=thing)))"),
+			},
+			ExpectError: false,
+		},
+		{
+			Name: "invalid guest filter #3",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("(&(property=value)(|(otherthing=othervalue)(other=thing))"),
+			},
+			ExpectError: true,
+		},
+		{
+			Name: "invalid guest filter #4",
+			LdapSettings: LdapSettings{
+				Enable:            NewBool(true),
+				LdapServer:        NewString("server"),
+				BaseDN:            NewString("basedn"),
+				EmailAttribute:    NewString("email"),
+				UsernameAttribute: NewString("username"),
+				IdAttribute:       NewString("id"),
+				LoginIdAttribute:  NewString("loginid"),
+				GuestFilter:       NewString("(&(property=value)((otherthing=othervalue)(other=thing)))"),
+			},
+			ExpectError: true,
+		},
 	} {
 		t.Run(test.Name, func(t *testing.T) {
 			test.LdapSettings.SetDefaults()

model/outgoing_webhook_test.go

@@ -5,9 +5,10 @@ package model
 
 import (
 	"net/url"
-	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestOutgoingWebhookJson(t *testing.T) {
@@ -15,132 +16,81 @@ func TestOutgoingWebhookJson(t *testing.T) {
 	json := o.ToJson()
 	ro := OutgoingWebhookFromJson(strings.NewReader(json))
 
-	if o.Id != ro.Id {
+	assert.Equal(t, o.Id, ro.Id, "Ids do not match")
-		t.Fatal("Ids do not match")
-	}
 }
 
 func TestOutgoingWebhookIsValid(t *testing.T) {
 	o := OutgoingWebhook{}
-
+	assert.NotNil(t, o.IsValid(), "empty declaration should be invalid")
-	if err := o.IsValid(); err == nil {
-		t.Fatal("should be invalid")
-	}
 
 	o.Id = NewId()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "Id = NewId; %s should be invalid", o.Id)
-		t.Fatal("should be invalid")
-	}
 
 	o.CreateAt = GetMillis()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "CreateAt = GetMillis; %d should be invalid", o.CreateAt)
-		t.Fatal("should be invalid")
-	}
 
 	o.UpdateAt = GetMillis()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "UpdateAt = GetMillis; %d should be invalid", o.UpdateAt)
-		t.Fatal("should be invalid")
-	}
 
 	o.CreatorId = "123"
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "CreatorId %s should be invalid", o.CreatorId)
-		t.Fatal("should be invalid")
-	}
 
 	o.CreatorId = NewId()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "CreatorId = NewId; %s should be invalid", o.CreatorId)
-		t.Fatal("should be invalid")
-	}
 
 	o.Token = "123"
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "Token %s should be invalid", o.Token)
-		t.Fatal("should be invalid")
-	}
 
 	o.Token = NewId()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "Token = NewId; %s should be invalid", o.Token)
-		t.Fatal("should be invalid")
-	}
 
 	o.ChannelId = "123"
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "ChannelId %s should be invalid", o.ChannelId)
-		t.Fatal("should be invalid")
-	}
 
 	o.ChannelId = NewId()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "ChannelId = NewId; %s should be invalid", o.ChannelId)
-		t.Fatal("should be invalid")
-	}
 
 	o.TeamId = "123"
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "TeamId %s should be invalid", o.TeamId)
-		t.Fatal("should be invalid")
-	}
 
 	o.TeamId = NewId()
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "TeamId = NewId; %s should be invalid", o.TeamId)
-		t.Fatal("should be invalid")
-	}
 
 	o.CallbackURLs = []string{"nowhere.com/"}
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "%v for CallbackURLs should be invalid", o.CallbackURLs)
-		t.Fatal("should be invalid")
-	}
 
 	o.CallbackURLs = []string{"http://nowhere.com/"}
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "%v for CallbackURLs should be valid", o.CallbackURLs)
-		t.Fatal(err)
-	}
 
 	o.DisplayName = strings.Repeat("1", 65)
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "DisplayName length %d invalid, max length 64", len(o.DisplayName))
-		t.Fatal("should be invalid")
-	}
 
 	o.DisplayName = strings.Repeat("1", 64)
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "DisplayName length %d should be valid, max length 64", len(o.DisplayName))
-		t.Fatal(err)
-	}
 
 	o.Description = strings.Repeat("1", 501)
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "Description length %d should be invalid, max length 500", len(o.Description))
-		t.Fatal("should be invalid")
-	}
 
 	o.Description = strings.Repeat("1", 500)
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "Description length %d should be valid, max length 500", len(o.Description))
-		t.Fatal(err)
-	}
 
 	o.ContentType = strings.Repeat("1", 129)
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "ContentType length %d should be invalid, max length 128", len(o.ContentType))
-		t.Fatal(err)
-	}
 
 	o.ContentType = strings.Repeat("1", 128)
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "ContentType length %d should be valid", len(o.ContentType))
-		t.Fatal(err)
-	}
 
 	o.Username = strings.Repeat("1", 65)
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "Username length %d should be invalid, max length 64", len(o.Username))
-		t.Fatal("should be invalid")
-	}
 
 	o.Username = strings.Repeat("1", 64)
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "Username length %d should be valid", len(o.Username))
-		t.Fatal("should be invalid")
-	}
 
 	o.IconURL = strings.Repeat("1", 1025)
-	if err := o.IsValid(); err == nil {
+	assert.NotNilf(t, o.IsValid(), "IconURL length %d should be invalid, max length 1024", len(o.IconURL))
-		t.Fatal(err)
-	}
 
 	o.IconURL = strings.Repeat("1", 1024)
-	if err := o.IsValid(); err != nil {
+	assert.Nilf(t, o.IsValid(), "IconURL length %d should be valid", len(o.IconURL))
-		t.Fatal(err)
-	}
 }
 
 func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {
@@ -171,9 +121,9 @@ func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {
 	v.Set("text", "Text")
 	v.Set("trigger_word", "TriggerWord")
 	v.Set("file_ids", "FileIds")
-	if got, want := p.ToFormValues(), v.Encode(); !reflect.DeepEqual(got, want) {
+	got := p.ToFormValues()
-		t.Fatalf("Got %+v, wanted %+v", got, want)
+	want := v.Encode()
-	}
+	assert.Equalf(t, got, want, "Got %+v, wanted %+v", got, want)
 }
 
 func TestOutgoingWebhookPreSave(t *testing.T) {
@@ -189,12 +139,8 @@ func TestOutgoingWebhookPreUpdate(t *testing.T) {
 func TestOutgoingWebhookTriggerWordStartsWith(t *testing.T) {
 	o := OutgoingWebhook{Id: NewId()}
 	o.TriggerWords = append(o.TriggerWords, "foo")
-	if !o.TriggerWordStartsWith("foobar") {
+	assert.True(t, o.TriggerWordStartsWith("foobar"), "Should return true")
-		t.Fatal("Should return true")
+	assert.False(t, o.TriggerWordStartsWith("barfoo"), "Should return false")
-	}
-	if o.TriggerWordStartsWith("barfoo") {
-		t.Fatal("Should return false")
-	}
 }
 
 func TestOutgoingWebhookResponseJson(t *testing.T) {
@@ -204,7 +150,5 @@ func TestOutgoingWebhookResponseJson(t *testing.T) {
 	json := o.ToJson()
 	ro, _ := OutgoingWebhookResponseFromJson(strings.NewReader(json))
 
-	if *o.Text != *ro.Text {
+	assert.Equal(t, *o.Text, *ro.Text, "Text does not match")
-		t.Fatal("Text does not match")
-	}
 }

model/user_access_token_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserAccessTokenJson(t *testing.T) {
@@ -16,43 +18,33 @@ func TestUserAccessTokenJson(t *testing.T) {
 	json := a1.ToJson()
 	ra1 := UserAccessTokenFromJson(strings.NewReader(json))
 
-	if a1.Token != ra1.Token {
+	require.Equal(t, a1.Token, ra1.Token, "tokens didn't match")
-		t.Fatal("tokens didn't match")
-	}
 
 	tokens := []*UserAccessToken{&a1}
 	json = UserAccessTokenListToJson(tokens)
 	tokens = UserAccessTokenListFromJson(strings.NewReader(json))
 
-	if tokens[0].Token != a1.Token {
+	require.Equal(t, tokens[0].Token, ra1.Token, "tokens didn't match")
-		t.Fatal("tokens didn't match")
-	}
 }
 
 func TestUserAccessTokenIsValid(t *testing.T) {
 	ad := UserAccessToken{}
 
-	if err := ad.IsValid(); err == nil || err.Id != "model.user_access_token.is_valid.id.app_error" {
+	err := ad.IsValid()
-		t.Fatal(err)
+	require.False(t, err == nil || err.Id != "model.user_access_token.is_valid.id.app_error")
-	}
 
 	ad.Id = NewRandomString(26)
-	if err := ad.IsValid(); err == nil || err.Id != "model.user_access_token.is_valid.token.app_error" {
+	err = ad.IsValid()
-		t.Fatal(err)
+	require.False(t, err == nil || err.Id != "model.user_access_token.is_valid.token.app_error")
-	}
 
 	ad.Token = NewRandomString(26)
-	if err := ad.IsValid(); err == nil || err.Id != "model.user_access_token.is_valid.user_id.app_error" {
+	err = ad.IsValid()
-		t.Fatal(err)
+	require.False(t, err == nil || err.Id != "model.user_access_token.is_valid.user_id.app_error")
-	}
 
 	ad.UserId = NewRandomString(26)
-	if err := ad.IsValid(); err != nil {
+	require.Nil(t, ad.IsValid())
-		t.Fatal(err)
-	}
 
 	ad.Description = NewRandomString(256)
-	if err := ad.IsValid(); err == nil || err.Id != "model.user_access_token.is_valid.description.app_error" {
+	err = ad.IsValid()
-		t.Fatal(err)
+	require.False(t, err == nil || err.Id != "model.user_access_token.is_valid.description.app_error")
-	}
 }

model/websocket_request_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestWebSocketRequest(t *testing.T) {
@@ -13,13 +15,9 @@ func TestWebSocketRequest(t *testing.T) {
 	json := m.ToJson()
 	result := WebSocketRequestFromJson(strings.NewReader(json))
 
-	if result == nil {
+	require.NotNil(t, result)
-		t.Fatal("should not be nil")
-	}
 
 	badresult := WebSocketRequestFromJson(strings.NewReader("junk"))
 
-	if badresult != nil {
+	require.Nil(t, badresult)
-		t.Fatal("should have been nil")
-	}
 }

plugin/health_check.go

@@ -4,7 +4,6 @@
 package plugin
 
 import (
-	"fmt"
 	"sync"
 	"time"
 
@@ -88,7 +87,7 @@ func (job *PluginHealthCheckJob) checkPlugin(id string) {
 	pluginErr := sup.PerformHealthCheck()
 
 	if pluginErr != nil {
-		mlog.Error(fmt.Sprintf("Health check failed for plugin %s, error: %s", id, pluginErr.Error()))
+		mlog.Error("Health check failed for plugin", mlog.String("id", id), mlog.Err(pluginErr))
 		job.handleHealthCheckFail(id, pluginErr)
 	}
 }
@@ -107,13 +106,13 @@ func (job *PluginHealthCheckJob) handleHealthCheckFail(id string, err error) {
 
 	if shouldDeactivatePlugin(p) {
 		p.failTimeStamps = []time.Time{}
-		mlog.Debug(fmt.Sprintf("Deactivating plugin due to multiple crashes `%s`", id))
+		mlog.Debug("Deactivating plugin due to multiple crashes", mlog.String("id", id))
 		job.env.Deactivate(id)
 		job.env.SetPluginState(id, model.PluginStateFailedToStayRunning)
 	} else {
-		mlog.Debug(fmt.Sprintf("Restarting plugin due to failed health check `%s`", id))
+		mlog.Debug("Restarting plugin due to failed health check", mlog.String("id", id))
 		if err := job.env.RestartPlugin(id); err != nil {
-			mlog.Error(fmt.Sprintf("Failed to restart plugin `%s`: %s", id, err.Error()))
+			mlog.Error("Failed to restart plugin", mlog.String("id", id), mlog.Err(err))
 		}
 	}
 }

plugin/helpers_bots_test.go

@@ -136,7 +136,7 @@ func TestEnsureBot(t *testing.T) {
 			assert.Nil(t, err)
 		})
 
-		t.Run("shoudl fail if create bot fails", func(t *testing.T) {
+		t.Run("should fail if create bot fails", func(t *testing.T) {
 			api := setupAPI()
 			api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil)
 			api.On("GetUserByUsername", testbot.Username).Return(nil, nil)

services/filesstore/s3store_test.go

@@ -7,26 +7,23 @@ import (
 	"testing"
 
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCheckMandatoryS3Fields(t *testing.T) {
 	cfg := model.FileSettings{}
 
 	err := CheckMandatoryS3Fields(&cfg)
-	if err == nil || err.Message != "api.admin.test_s3.missing_s3_bucket" {
+	require.NotNil(t, err)
-		t.Fatal("should've failed with missing s3 bucket")
+	require.Equal(t, err.Message, "api.admin.test_s3.missing_s3_bucket", "should've failed with missing s3 bucket")
-	}
 
 	cfg.AmazonS3Bucket = model.NewString("test-mm")
 	err = CheckMandatoryS3Fields(&cfg)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal("should've not failed")
-	}
 
 	cfg.AmazonS3Endpoint = model.NewString("")
 	err = CheckMandatoryS3Fields(&cfg)
-	if err != nil || *cfg.AmazonS3Endpoint != "s3.amazonaws.com" {
-		t.Fatal("should've not failed because it should set the endpoint to the default")
-	}
 
+	require.Nil(t, err)
+	require.Equal(t, *cfg.AmazonS3Endpoint, "s3.amazonaws.com", "should've set the endpoint to the default")
 }

store/sqlstore/upgrade.go

@@ -6,7 +6,6 @@ package sqlstore
 import (
 	"database/sql"
 	"encoding/json"
-	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -106,7 +105,7 @@ func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error
 		}
 
 		currentSchemaVersion = &currentModelVersion
-		mlog.Info(fmt.Sprintf("The database schema has been set to version %s", *currentSchemaVersion))
+		mlog.Info("The database schema version has been set", mlog.String("version", currentSchemaVersion.String()))
 		return nil
 	}
 
@@ -119,7 +118,7 @@ func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error
 	if currentSchemaVersion.GTE(nextUnsupportedMajorVersion) {
 		return errors.Errorf("Database schema version %s is not supported. This Mattermost server supports only >=%s, <%s. Please upgrade to at least version %s before continuing.", *currentSchemaVersion, currentModelVersion, nextUnsupportedMajorVersion, nextUnsupportedMajorVersion)
 	} else if currentSchemaVersion.GT(currentModelVersion) {
-		mlog.Warn(fmt.Sprintf("The database schema with version %s is newer than Mattermost version %s.", currentSchemaVersion, currentModelVersion))
+		mlog.Warn("The database schema version and model versions do not match", mlog.String("schema_version", currentSchemaVersion.String()), mlog.String("model_version", currentModelVersion.String()))
 	}
 
 	// Otherwise, apply any necessary migrations. Note that these methods currently invoke
@@ -176,12 +175,12 @@ func saveSchemaVersion(sqlStore SqlStore, version string) {
 		os.Exit(EXIT_VERSION_SAVE)
 	}
 
-	mlog.Warn(fmt.Sprintf("The database schema has been upgraded to version %v", version))
+	mlog.Warn("The database schema version has been upgraded", mlog.String("version", version))
 }
 
 func shouldPerformUpgrade(sqlStore SqlStore, currentSchemaVersion string, expectedSchemaVersion string) bool {
 	if sqlStore.GetCurrentSchemaVersion() == currentSchemaVersion {
-		mlog.Warn(fmt.Sprintf("Attempting to upgrade the database schema version from %s to %v", currentSchemaVersion, expectedSchemaVersion))
+		mlog.Warn("Attempting to upgrade the database schema version", mlog.String("current_version", currentSchemaVersion), mlog.String("new_version", expectedSchemaVersion))
 
 		return true
 	}
@@ -205,7 +204,7 @@ func UpgradeDatabaseToVersion32(sqlStore SqlStore) {
 }
 
 func themeMigrationFailed(err error) {
-	mlog.Critical(fmt.Sprintf("Failed to migrate User.ThemeProps to Preferences table %v", err))
+	mlog.Critical("Failed to migrate User.ThemeProps to Preferences table", mlog.Err(err))
 	time.Sleep(time.Second)
 	os.Exit(EXIT_THEME_MIGRATION)
 }
@@ -479,7 +478,7 @@ func UpgradeDatabaseToVersion49(sqlStore SqlStore) {
 		defaultTimezone := timezones.DefaultUserTimezone()
 		defaultTimezoneValue, err := json.Marshal(defaultTimezone)
 		if err != nil {
-			mlog.Critical(fmt.Sprint(err))
+			mlog.Critical(err.Error())
 		}
 		sqlStore.CreateColumnIfNotExists("Users", "Timezone", "varchar(256)", "varchar(256)", string(defaultTimezoneValue))
 		sqlStore.RemoveIndexIfExists("idx_channels_displayname", "Channels")
@@ -673,12 +672,12 @@ func UpgradeDatabaseToVersion511(sqlStore SqlStore) {
 		// Enforce all teams have an InviteID set
 		var teams []*model.Team
 		if _, err := sqlStore.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE InviteId = ''"); err != nil {
-			mlog.Error("Error fetching Teams without InviteID: " + err.Error())
+			mlog.Error("Error fetching Teams without InviteID", mlog.Err(err))
 		} else {
 			for _, team := range teams {
 				team.InviteId = model.NewId()
 				if _, err := sqlStore.Team().Update(team); err != nil {
-					mlog.Error("Error updating Team InviteIDs: " + err.Error())
+					mlog.Error("Error updating Team InviteIDs", mlog.String("team_id", team.Id), mlog.Err(err))
 				}
 			}
 		}

store/storetest/audit_store.go

@@ -45,9 +45,7 @@ func testAuditStore(t *testing.T, ss store.Store) {
 
 	audits, err = ss.Audit().Get("", 0, 100)
 	require.Nil(t, err)
-	if len(audits) < 4 {
+	require.Len(t, audits, 4, "Failed to save and retrieve 4 audit logs")
-		t.Fatal("Failed to save and retrieve 4 audit logs")
-	}
 
 	require.Nil(t, ss.Audit().PermanentDeleteByUser(audit.UserId))
 }

store/storetest/cluster_discovery_store.go

@@ -29,13 +29,11 @@ func testClusterDiscoveryStore(t *testing.T, ss store.Store) {
 		Type:        "test_test",
 	}
 
-	if err := ss.ClusterDiscovery().Save(discovery); err != nil {
+	err := ss.ClusterDiscovery().Save(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if err := ss.ClusterDiscovery().Cleanup(); err != nil {
+	err = ss.ClusterDiscovery().Cleanup()
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testClusterDiscoveryStoreDelete(t *testing.T, ss store.Store) {
@@ -45,13 +43,11 @@ func testClusterDiscoveryStoreDelete(t *testing.T, ss store.Store) {
 		Type:        "test_test",
 	}
 
-	if err := ss.ClusterDiscovery().Save(discovery); err != nil {
+	err := ss.ClusterDiscovery().Save(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if _, err := ss.ClusterDiscovery().Delete(discovery); err != nil {
+	_, err = ss.ClusterDiscovery().Delete(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testClusterDiscoveryStoreLastPing(t *testing.T, ss store.Store) {
@@ -61,29 +57,24 @@ func testClusterDiscoveryStoreLastPing(t *testing.T, ss store.Store) {
 		Type:        "test_test_lastPing" + model.NewId(),
 	}
 
-	if err := ss.ClusterDiscovery().Save(discovery); err != nil {
+	err := ss.ClusterDiscovery().Save(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if err := ss.ClusterDiscovery().SetLastPingAt(discovery); err != nil {
+	err = ss.ClusterDiscovery().SetLastPingAt(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	ttime := model.GetMillis()
 
 	time.Sleep(1 * time.Second)
 
-	if err := ss.ClusterDiscovery().SetLastPingAt(discovery); err != nil {
+	err = ss.ClusterDiscovery().SetLastPingAt(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	list, err := ss.ClusterDiscovery().GetAll(discovery.Type, "cluster_name_lastPing")
 	require.Nil(t, err)
 	assert.Len(t, list, 1)
 
-	if list[0].LastPingAt-ttime < 500 {
+	require.Less(t, int64(500), list[0].LastPingAt-ttime)
-		t.Fatal("failed to set time")
-	}
 
 	discovery2 := &model.ClusterDiscovery{
 		ClusterName: "cluster_name_missing",
@@ -91,9 +82,8 @@ func testClusterDiscoveryStoreLastPing(t *testing.T, ss store.Store) {
 		Type:        "test_test_missing",
 	}
 
-	if err := ss.ClusterDiscovery().SetLastPingAt(discovery2); err != nil {
+	err = ss.ClusterDiscovery().SetLastPingAt(discovery2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testClusterDiscoveryStoreExists(t *testing.T, ss store.Store) {
@@ -103,9 +93,8 @@ func testClusterDiscoveryStoreExists(t *testing.T, ss store.Store) {
 		Type:        "test_test_Exists" + model.NewId(),
 	}
 
-	if err := ss.ClusterDiscovery().Save(discovery); err != nil {
+	err := ss.ClusterDiscovery().Save(discovery)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	val, err := ss.ClusterDiscovery().Exists(discovery)
 	require.Nil(t, err)