Merge branch 'master' into mark-as-unread

.circleci/config.yml

@@ -52,7 +52,7 @@ jobs:
 
   build:
     docker:
-      - image: mattermost/mattermost-build-server:feb-28-2019
+      - image: mattermost/mattermost-build-server:sep-17-2019
     working_directory: /go/src/github.com/mattermost
     steps:
       - attach_workspace:
@@ -119,7 +119,7 @@ jobs:
               --env MM_ELASTICSEARCHSETTINGS_CONNECTIONURL=http://elasticsearch:9200 \
               -v ~/go/src:/go/src \
               -w /go/src/github.com/mattermost/mattermost-server \
-              mattermost/mattermost-build-server:feb-28-2019 \
+              mattermost/mattermost-build-server:sep-17-2019 \
               bash -c 'ulimit -n 8096; make test-server BUILD_NUMBER="$CIRCLE_BRANCH-$CIRCLE_PREVIOUS_BUILD_NUM" TESTFLAGS= TESTFLAGSEE='
           no_output_timeout: 1h
       - run:

Makefile

@@ -45,8 +45,9 @@ else
 endif
 
 # Golang Flags
+export GO111MODULE=on
 GOPATH ?= $(shell go env GOPATH)
-GOFLAGS ?= $(GOFLAGS:)
+GOFLAGS ?= $(GOFLAGS:) -mod=vendor
 GO=go
 DELVE=dlv
 LDFLAGS += -X "github.com/mattermost/mattermost-server/model.BuildNumber=$(BUILD_NUMBER)"
@@ -141,7 +142,7 @@ clean-docker: ## Deletes the docker containers for local development.
 
 govet: ## Runs govet against all packages.
 	@echo Running GOVET
-	$(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
+	env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
 	$(GO) vet $(GOFLAGS) $(ALL_PACKAGES) || exit 1
 	$(GO) vet -vettool=$(GOPATH)/bin/shadow $(GOFLAGS) $(ALL_PACKAGES) || exit 1
 	$(GO) run plugin/checker/main.go
@@ -151,7 +152,7 @@ gofmt: ## Runs gofmt against all packages.
 
 	@for package in $(TE_PACKAGES) $(EE_PACKAGES); do \
 		echo "Checking "$$package; \
-		files=$$(go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \
+		files=$$($(GO) list $(GOFLAGS) -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \
 		if [ "$$files" ]; then \
 			gofmt_output=$$(gofmt -d -s $$files 2>&1); \
 			if [ "$$gofmt_output" ]; then \
@@ -180,7 +181,7 @@ store-mocks: ## Creates mock files.
 	$(GOPATH)/bin/mockery -dir store -all -output store/storetest/mocks -note 'Regenerate this file using `make store-mocks`.'
 
 store-layers: ## Generate layers for the store
-	go generate ./store
+	$(GO) generate $(GOFLAGS) ./store
 
 filesstore-mocks: ## Creates mock files.
 	env GO111MODULE=off go get -u github.com/vektra/mockery/...
@@ -201,7 +202,7 @@ einterfaces-mocks: ## Creates mock files for einterfaces.
 	$(GOPATH)/bin/mockery -dir einterfaces -all -output einterfaces/mocks -note 'Regenerate this file using `make einterfaces-mocks`.'
 
 pluginapi: ## Generates api and hooks glue code for plugins
-	go generate ./plugin
+	$(GO) generate $(GOFLAGS) ./plugin
 
 check-licenses: ## Checks license status.
 	./scripts/license-check.sh $(TE_PACKAGES) $(EE_PACKAGES)
@@ -409,7 +410,7 @@ config-ldap: ## Configures LDAP.
 config-reset: ## Resets the config/config.json file to the default.
 	@echo Resetting configuration to default
 	rm -f config/config.json
-	OUTPUT_CONFIG=$(PWD)/config/config.json go generate ./config
+	OUTPUT_CONFIG=$(PWD)/config/config.json $(GO) generate $(GOFLAGS) ./config
 
 diff-config: ## Compares default configuration between two mattermost versions
 	@./scripts/diff-config.sh

app/channel_test.go

@@ -25,22 +25,20 @@ func TestPermanentDeleteChannel(t *testing.T) {
 	})
 
 	channel, err := th.App.CreateChannel(&model.Channel{DisplayName: "deletion-test", Name: "deletion-test", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false)
-	if err != nil {
+	require.NotNil(t, channel, "Channel shouldn't be nil")
-		t.Fatal(err.Error())
+	require.Nil(t, err)
-	}
 	defer func() {
 		th.App.PermanentDeleteChannel(channel)
 	}()
 
 	incoming, err := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, channel, &model.IncomingWebhook{ChannelId: channel.Id})
-	if err != nil {
+	require.NotNil(t, incoming, "incoming webhook should not be nil")
-		t.Fatal(err.Error())
+	require.Nil(t, err, "Unable to create Incoming Webhook for Channel")
-	}
 	defer th.App.DeleteIncomingWebhook(incoming.Id)
 
-	if incoming, err = th.App.GetIncomingWebhook(incoming.Id); incoming == nil || err != nil {
+	incoming, err = th.App.GetIncomingWebhook(incoming.Id)
-		t.Fatal("unable to get new incoming webhook")
+	require.NotNil(t, incoming, "incoming webhook should not be nil")
-	}
+	require.Nil(t, err, "Unable to get new incoming webhook")
 
 	outgoing, err := th.App.CreateOutgoingWebhook(&model.OutgoingWebhook{
 		ChannelId:    channel.Id,
@@ -48,30 +46,29 @@ func TestPermanentDeleteChannel(t *testing.T) {
 		CreatorId:    th.BasicUser.Id,
 		CallbackURLs: []string{"http://foo"},
 	})
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err.Error())
-	}
 	defer th.App.DeleteOutgoingWebhook(outgoing.Id)
 
-	if outgoing, err = th.App.GetOutgoingWebhook(outgoing.Id); outgoing == nil || err != nil {
+	outgoing, err = th.App.GetOutgoingWebhook(outgoing.Id)
-		t.Fatal("unable to get new outgoing webhook")
+	require.NotNil(t, outgoing, "Outgoing webhook should not be nil")
-	}
+	require.Nil(t, err, "Unable to get new outgoing webhook")
 
 	err = th.App.PermanentDeleteChannel(channel)
 	require.Nil(t, err)
 
-	if incoming, err = th.App.GetIncomingWebhook(incoming.Id); incoming != nil || err == nil {
+	incoming, err = th.App.GetIncomingWebhook(incoming.Id)
-		t.Error("incoming webhook wasn't deleted")
+	require.Nil(t, incoming, "Incoming webhook should be nil")
-	}
+	require.NotNil(t, err, "Incoming webhook wasn't deleted")
 
-	if outgoing, err = th.App.GetOutgoingWebhook(outgoing.Id); outgoing != nil || err == nil {
+	outgoing, err = th.App.GetOutgoingWebhook(outgoing.Id)
-		t.Error("outgoing webhook wasn't deleted")
+	require.Nil(t, outgoing, "Outgoing webhook should be nil")
-	}
+	require.NotNil(t, err, "Outgoing webhook wasn't deleted")
 }
 
 func TestMoveChannel(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
+	var err *model.AppError
 
 	sourceTeam := th.CreateTeam()
 	targetTeam := th.CreateTeam()
@@ -82,62 +79,51 @@ func TestMoveChannel(t *testing.T) {
 		th.App.PermanentDeleteTeam(targetTeam)
 	}()
 
-	if _, err := th.App.AddUserToTeam(sourceTeam.Id, th.BasicUser.Id, ""); err != nil {
+	_, err = th.App.AddUserToTeam(sourceTeam.Id, th.BasicUser.Id, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
-	if _, err := th.App.AddUserToTeam(sourceTeam.Id, th.BasicUser2.Id, ""); err != nil {
-		t.Fatal(err)
-	}
 
-	if _, err := th.App.AddUserToTeam(targetTeam.Id, th.BasicUser.Id, ""); err != nil {
+	_, err = th.App.AddUserToTeam(sourceTeam.Id, th.BasicUser2.Id, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if _, err := th.App.AddUserToChannel(th.BasicUser, channel1); err != nil {
+	_, err = th.App.AddUserToTeam(targetTeam.Id, th.BasicUser.Id, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
-	if _, err := th.App.AddUserToChannel(th.BasicUser2, channel1); err != nil {
-		t.Fatal(err)
-	}
 
-	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false); err == nil {
+	_, err = th.App.AddUserToChannel(th.BasicUser, channel1)
-		t.Fatal("Should have failed due to mismatched members.")
+	require.Nil(t, err)
-	}
 
-	if _, err := th.App.AddUserToTeam(targetTeam.Id, th.BasicUser2.Id, ""); err != nil {
+	_, err = th.App.AddUserToChannel(th.BasicUser2, channel1)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false); err != nil {
+	err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false)
-		t.Fatal(err)
+	require.NotNil(t, err, "Should have failed due to mismatched members.")
-	}
+
+	_, err = th.App.AddUserToTeam(targetTeam.Id, th.BasicUser2.Id, "")
+	require.Nil(t, err)
+
+	err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false)
+	require.Nil(t, err)
 
 	// Test moving a channel with a deactivated user who isn't in the destination team.
 	// It should fail, unless removeDeactivatedMembers is true.
 	deacivatedUser := th.CreateUser()
 	channel2 := th.CreateChannel(sourceTeam)
 
-	if _, err := th.App.AddUserToTeam(sourceTeam.Id, deacivatedUser.Id, ""); err != nil {
+	_, err = th.App.AddUserToTeam(sourceTeam.Id, deacivatedUser.Id, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
+	_, err = th.App.AddUserToChannel(th.BasicUser, channel2)
-	if _, err := th.App.AddUserToChannel(th.BasicUser, channel2); err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
-	if _, err := th.App.AddUserToChannel(deacivatedUser, channel2); err != nil {
-		t.Fatal(err)
-	}
 
-	if _, err := th.App.UpdateActive(deacivatedUser, false); err != nil {
+	_, err = th.App.AddUserToChannel(deacivatedUser, channel2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if err := th.App.MoveChannel(targetTeam, channel2, th.BasicUser, false); err == nil {
+	_, err = th.App.UpdateActive(deacivatedUser, false)
-		t.Fatal("Should have failed due to mismatched deacivated member.")
+	require.Nil(t, err)
-	}
 
-	if err := th.App.MoveChannel(targetTeam, channel2, th.BasicUser, true); err != nil {
+	err = th.App.MoveChannel(targetTeam, channel2, th.BasicUser, false)
-		t.Fatal(err)
+	require.NotNil(t, err, "Should have failed due to mismatched deacivated member.")
-	}
+
+	err = th.App.MoveChannel(targetTeam, channel2, th.BasicUser, true)
+	require.Nil(t, err)
 
 	// Test moving a channel with no members.
 	channel3 := &model.Channel{
@@ -148,7 +134,6 @@ func TestMoveChannel(t *testing.T) {
 		CreatorId:   th.BasicUser.Id,
 	}
 
-	var err *model.AppError
 	channel3, err = th.App.CreateChannel(channel3, false)
 	require.Nil(t, err)
 
@@ -231,20 +216,12 @@ func TestJoinDefaultChannelsExperimentalDefaultChannels(t *testing.T) {
 
 	for _, channelName := range defaultChannelList {
 		channel, err := th.App.GetChannelByName(channelName, th.BasicTeam.Id, false)
-
+		require.Nil(t, err, "Expected nil, didn't receive nil")
-		if err != nil {
-			t.Errorf("Expected nil, got %s", err)
-		}
 
 		member, err := th.App.GetChannelMember(channel.Id, user.Id)
 
-		if member == nil {
+		require.NotNil(t, member, "Expected member object, got nil")
-			t.Errorf("Expected member object, got nil")
+		require.Nil(t, err, "Expected nil object, didn't receive nil")
-		}
-
-		if err != nil {
-			t.Errorf("Expected nil object, got %s", err)
-		}
 	}
 }
 
@@ -285,12 +262,10 @@ func TestUpdateChannelPrivacy(t *testing.T) {
 	privateChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE)
 	privateChannel.Type = model.CHANNEL_OPEN
 
-	if publicChannel, err := th.App.UpdateChannelPrivacy(privateChannel, th.BasicUser); err != nil {
+	publicChannel, err := th.App.UpdateChannelPrivacy(privateChannel, th.BasicUser)
-		t.Fatal("Failed to update channel privacy. Error: " + err.Error())
+	require.Nil(t, err, "Failed to update channel privacy.")
-	} else {
+	assert.Equal(t, publicChannel.Id, privateChannel.Id)
-		assert.Equal(t, publicChannel.Id, privateChannel.Id)
+	assert.Equal(t, publicChannel.Type, model.CHANNEL_OPEN)
-		assert.Equal(t, publicChannel.Type, model.CHANNEL_OPEN)
-	}
 }
 
 func TestCreateGroupChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
@@ -305,24 +280,22 @@ func TestCreateGroupChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
 	groupUserIds = append(groupUserIds, user2.Id)
 	groupUserIds = append(groupUserIds, th.BasicUser.Id)
 
-	if channel, err := th.App.CreateGroupChannel(groupUserIds, th.BasicUser.Id); err != nil {
+	channel, err := th.App.CreateGroupChannel(groupUserIds, th.BasicUser.Id)
-		t.Fatal("Failed to create group channel. Error: " + err.Message)
-	} else {
-		// there should be a ChannelMemberHistory record for each user
-		histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-		require.Nil(t, err)
-		assert.Len(t, histories, 3)
 
-		channelMemberHistoryUserIds := make([]string, 0)
+	require.Nil(t, err, "Failed to create group channel.")
-		for _, history := range histories {
+	histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-			assert.Equal(t, channel.Id, history.ChannelId)
+	require.Nil(t, err)
-			channelMemberHistoryUserIds = append(channelMemberHistoryUserIds, history.UserId)
+	assert.Len(t, histories, 3)
-		}
 
-		sort.Strings(groupUserIds)
+	channelMemberHistoryUserIds := make([]string, 0)
-		sort.Strings(channelMemberHistoryUserIds)
+	for _, history := range histories {
-		assert.Equal(t, groupUserIds, channelMemberHistoryUserIds)
+		assert.Equal(t, channel.Id, history.ChannelId)
+		channelMemberHistoryUserIds = append(channelMemberHistoryUserIds, history.UserId)
 	}
+
+	sort.Strings(groupUserIds)
+	sort.Strings(channelMemberHistoryUserIds)
+	assert.Equal(t, groupUserIds, channelMemberHistoryUserIds)
 }
 
 func TestCreateDirectChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
@@ -332,24 +305,22 @@ func TestCreateDirectChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
 	user1 := th.CreateUser()
 	user2 := th.CreateUser()
 
-	if channel, err := th.App.GetOrCreateDirectChannel(user1.Id, user2.Id); err != nil {
+	channel, err := th.App.GetOrCreateDirectChannel(user1.Id, user2.Id)
-		t.Fatal("Failed to create direct channel. Error: " + err.Message)
+	require.Nil(t, err, "Failed to create direct channel.")
-	} else {
-		// there should be a ChannelMemberHistory record for both users
-		histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-		require.Nil(t, err)
-		assert.Len(t, histories, 2)
 
-		historyId0 := histories[0].UserId
+	histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-		historyId1 := histories[1].UserId
+	require.Nil(t, err)
-		switch historyId0 {
+	assert.Len(t, histories, 2)
-		case user1.Id:
+
-			assert.Equal(t, user2.Id, historyId1)
+	historyId0 := histories[0].UserId
-		case user2.Id:
+	historyId1 := histories[1].UserId
-			assert.Equal(t, user1.Id, historyId1)
+	switch historyId0 {
-		default:
+	case user1.Id:
-			t.Fatal("Unexpected user id " + historyId0 + " in ChannelMemberHistory table")
+		assert.Equal(t, user2.Id, historyId1)
-		}
+	case user2.Id:
+		assert.Equal(t, user1.Id, historyId1)
+	default:
+		require.Fail(t, "Unexpected user id in ChannelMemberHistory table", historyId0)
 	}
 }
 
@@ -361,24 +332,23 @@ func TestGetDirectChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
 	user2 := th.CreateUser()
 
 	// this function call implicitly creates a direct channel between the two users if one doesn't already exist
-	if channel, err := th.App.GetOrCreateDirectChannel(user1.Id, user2.Id); err != nil {
+	channel, err := th.App.GetOrCreateDirectChannel(user1.Id, user2.Id)
-		t.Fatal("Failed to create direct channel. Error: " + err.Message)
+	require.Nil(t, err, "Failed to create direct channel.")
-	} else {
-		// there should be a ChannelMemberHistory record for both users
-		histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-		require.Nil(t, err)
-		assert.Len(t, histories, 2)
 
-		historyId0 := histories[0].UserId
+	// there should be a ChannelMemberHistory record for both users
-		historyId1 := histories[1].UserId
+	histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
-		switch historyId0 {
+	require.Nil(t, err)
-		case user1.Id:
+	assert.Len(t, histories, 2)
-			assert.Equal(t, user2.Id, historyId1)
+
-		case user2.Id:
+	historyId0 := histories[0].UserId
-			assert.Equal(t, user1.Id, historyId1)
+	historyId1 := histories[1].UserId
-		default:
+	switch historyId0 {
-			t.Fatal("Unexpected user id " + historyId0 + " in ChannelMemberHistory table")
+	case user1.Id:
-		}
+		assert.Equal(t, user2.Id, historyId1)
+	case user2.Id:
+		assert.Equal(t, user1.Id, historyId1)
+	default:
+		require.Fail(t, "Unexpected user id in ChannelMemberHistory table", historyId0)
 	}
 }
 
@@ -388,18 +358,17 @@ func TestAddUserToChannelCreatesChannelMemberHistoryRecord(t *testing.T) {
 
 	// create a user and add it to a channel
 	user := th.CreateUser()
-	if _, err := th.App.AddTeamMember(th.BasicTeam.Id, user.Id); err != nil {
+	_, err := th.App.AddTeamMember(th.BasicTeam.Id, user.Id)
-		t.Fatal("Failed to add user to team. Error: " + err.Message)
+	require.Nil(t, err, "Failed to add user to team.")
-	}
 
 	groupUserIds := make([]string, 0)
 	groupUserIds = append(groupUserIds, th.BasicUser.Id)
 	groupUserIds = append(groupUserIds, user.Id)
 
 	channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN)
-	if _, err := th.App.AddUserToChannel(user, channel); err != nil {
+
-		t.Fatal("Failed to add user to channel. Error: " + err.Message)
+	_, err = th.App.AddUserToChannel(user, channel)
-	}
+	require.Nil(t, err, "Failed to add user to channel.")
 
 	// there should be a ChannelMemberHistory record for the user
 	histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
@@ -511,9 +480,8 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) {
 	channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN)
 	userRequestorId := ""
 	postRootId := ""
-	if _, err := th.App.AddChannelMember(user.Id, channel, userRequestorId, postRootId); err != nil {
+	_, err := th.App.AddChannelMember(user.Id, channel, userRequestorId, postRootId)
-		t.Fatal("Failed to add user to channel. Error: " + err.Message)
+	require.Nil(t, err, "Failed to add user to channel.")
-	}
 
 	// there should be a ChannelMemberHistory record for the user
 	histories, err := th.App.Srv.Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, channel.Id)
@@ -547,12 +515,10 @@ func TestAppUpdateChannelScheme(t *testing.T) {
 	channel.SchemeId = mockID
 
 	updatedChannel, err := th.App.UpdateChannelScheme(channel)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
 
 	if updatedChannel.SchemeId != mockID {
-		t.Fatal("Wrong Channel SchemeId")
+		require.Fail(t, "Wrong Channel SchemeId")
 	}
 }
 
@@ -814,9 +780,8 @@ func TestGetChannelMembersTimezones(t *testing.T) {
 
 	userRequestorId := ""
 	postRootId := ""
-	if _, err := th.App.AddChannelMember(th.BasicUser2.Id, th.BasicChannel, userRequestorId, postRootId); err != nil {
+	_, err := th.App.AddChannelMember(th.BasicUser2.Id, th.BasicChannel, userRequestorId, postRootId)
-		t.Fatal("Failed to add user to channel. Error: " + err.Message)
+	require.Nil(t, err, "Failed to add user to channel.")
-	}
 
 	user := th.BasicUser
 	user.Timezone["useAutomaticTimezone"] = "false"
@@ -839,9 +804,8 @@ func TestGetChannelMembersTimezones(t *testing.T) {
 	th.App.AddUserToChannel(ruser, th.BasicChannel)
 
 	timezones, err := th.App.GetChannelMembersTimezones(th.BasicChannel.Id)
-	if err != nil {
+	require.Nil(t, err, "Failed to get the timezones for a channel.")
-		t.Fatal("Failed to get the timezones for a channel. Error: " + err.Error())
+
-	}
 	assert.Equal(t, 2, len(timezones))
 }
 
@@ -903,9 +867,8 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.AddUserToChannel(ruser, th.BasicChannel)
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user"); err == nil {
+		_, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user")
-			t.Fatal("Should fail when try to modify the guest role")
+		require.NotNil(t, err, "Should fail when try to modify the guest role")
-		}
 	})
 
 	t.Run("from user to guest", func(t *testing.T) {
@@ -918,9 +881,8 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.AddUserToChannel(ruser, th.BasicChannel)
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest"); err == nil {
+		_, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest")
-			t.Fatal("Should fail when try to modify the guest role")
+		require.NotNil(t, err, "Should fail when try to modify the guest role")
-		}
 	})
 
 	t.Run("from user to admin", func(t *testing.T) {
@@ -933,9 +895,8 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.AddUserToChannel(ruser, th.BasicChannel)
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user channel_admin"); err != nil {
+		_, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user channel_admin")
-			t.Fatal("Should work when you not modify guest role")
+		require.Nil(t, err, "Should work when you not modify guest role")
-		}
 	})
 
 	t.Run("from guest to guest plus custom", func(t *testing.T) {
@@ -951,9 +912,8 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.CreateRole(&model.Role{Name: "custom", DisplayName: "custom", Description: "custom"})
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest custom"); err != nil {
+		_, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest custom")
-			t.Fatal("Should work when you not modify guest role")
+		require.Nil(t, err, "Should work when you not modify guest role")
-		}
 	})
 
 	t.Run("a guest cant have user role", func(t *testing.T) {
@@ -966,9 +926,8 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.AddUserToChannel(ruser, th.BasicChannel)
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest channel_user"); err == nil {
+		_, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest channel_user")
-			t.Fatal("Should work when you not modify guest role")
+		require.NotNil(t, err, "Should work when you not modify guest role")
-		}
 	})
 }
 

app/command.go

@@ -158,7 +158,7 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *
 
 	clientTriggerId, triggerId, appErr := model.GenerateTriggerId(args.UserId, a.AsymmetricSigningKey())
 	if appErr != nil {
-		mlog.Error(appErr.Error())
+		mlog.Error("error occurred in generating trigger Id for a user ", mlog.Err(appErr))
 	}
 
 	args.TriggerId = triggerId
@@ -362,7 +362,7 @@ func (a *App) HandleCommandResponse(command *model.Command, args *model.CommandA
 	_, err := a.HandleCommandResponsePost(command, args, response, builtIn)
 
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("error occurred in handling command response post", mlog.Err(err))
 		lastError = err
 	}
 
@@ -371,7 +371,7 @@ func (a *App) HandleCommandResponse(command *model.Command, args *model.CommandA
 			_, err := a.HandleCommandResponsePost(command, args, resp, builtIn)
 
 			if err != nil {
-				mlog.Error(err.Error())
+				mlog.Error("error occurred in handling command response post", mlog.Err(err))
 				lastError = err
 			}
 		}

app/command_groupmsg_test.go

@@ -10,35 +10,44 @@ import (
 )
 
 func TestGroupMsgUsernames(t *testing.T) {
-	if users, parsedMessage := groupMsgUsernames(""); len(users) != 0 || parsedMessage != "" {
+	assert := assert.New(t)
-		t.Fatal("error parsing empty message")
-	}
-	if users, parsedMessage := groupMsgUsernames("test"); len(users) != 1 || parsedMessage != "" {
-		t.Fatal("error parsing simple user")
-	}
-	if users, parsedMessage := groupMsgUsernames("test1, test2, test3 , test4"); len(users) != 4 || parsedMessage != "" {
-		t.Fatal("error parsing various users")
-	}
 
-	if users, parsedMessage := groupMsgUsernames("test1, test2 message with spaces"); len(users) != 2 || parsedMessage != "message with spaces" {
+	users, parsedMessage := groupMsgUsernames("")
-		t.Fatal("error parsing message")
+	assert.Len(users, 0)
-	}
+	assert.Empty(parsedMessage)
 
-	if users, parsedMessage := groupMsgUsernames("test1, test2 message with, comma"); len(users) != 2 || parsedMessage != "message with, comma" {
+	users, parsedMessage = groupMsgUsernames("test")
-		t.Fatal("error parsing messages with comma")
+	assert.Len(users, 1)
-	}
+	assert.Empty(parsedMessage)
 
-	if users, parsedMessage := groupMsgUsernames("test1,,,test2"); len(users) != 2 || parsedMessage != "" {
+	users, parsedMessage = groupMsgUsernames("test1, test2, test3 , test4")
-		t.Fatal("error parsing multiple commas in username ")
+	assert.Len(users, 4)
-	}
+	assert.Empty(parsedMessage)
 
-	if users, parsedMessage := groupMsgUsernames("    test1,       test2     other message         "); len(users) != 2 || parsedMessage != "other message" {
+	users, parsedMessage = groupMsgUsernames("test1, test2 message with spaces")
-		t.Fatal("error parsing strange usage of spaces")
+	assert.Len(users, 2)
-	}
+	assert.Equal(parsedMessage, "message with spaces", "error parsing message")
 
-	if users, _ := groupMsgUsernames("    test1,       test2,,123,@321,+123"); len(users) != 5 || users[0] != "test1" || users[1] != "test2" || users[2] != "123" || users[3] != "321" || users[4] != "+123" {
+	users, parsedMessage = groupMsgUsernames("test1, test2 message with, comma")
-		t.Fatal("error parsing different types of users")
+	assert.Len(users, 2)
-	}
+	assert.Equal(parsedMessage, "message with, comma", "error parsing messages with comma")
+
+	users, parsedMessage = groupMsgUsernames("test1,,,test2")
+	assert.Len(users, 2)
+	assert.Empty(parsedMessage)
+
+	users, parsedMessage = groupMsgUsernames("    test1,       test2     other message         ")
+	assert.Len(users, 2)
+	assert.Equal(parsedMessage, "other message", "error parsing strange usage of spaces")
+
+	users, _ = groupMsgUsernames("    test1,       test2,,123,@321,+123")
+	assert.Len(users, 5)
+	assert.Equal(users[0], "test1")
+	assert.Equal(users[1], "test2")
+	assert.Equal(users[2], "123")
+	assert.Equal(users[3], "321")
+	assert.Equal(users[4], "+123")
+	assert.Equal(parsedMessage, "other message", "error parsing different types of users")
 }
 
 func TestGroupMsgProvider(t *testing.T) {

app/config_test.go

@@ -68,12 +68,10 @@ func TestClientConfigWithComputed(t *testing.T) {
 	defer th.TearDown()
 
 	config := th.App.ClientConfigWithComputed()
-	if _, ok := config["NoAccounts"]; !ok {
+	_, ok := config["NoAccounts"]
-		t.Fatal("expected NoAccounts in returned config")
+	assert.True(t, ok, "expected NoAccounts in returned config")
-	}
+	_, ok = config["MaxPostSize"]
-	if _, ok := config["MaxPostSize"]; !ok {
+	assert.True(t, ok, "expected MaxPostSize in returned config")
-		t.Fatal("expected MaxPostSize in returned config")
-	}
 }
 
 func TestEnsureInstallationDate(t *testing.T) {

app/email_batching_test.go

@@ -4,7 +4,7 @@
 package app
 
 import (
-	"strings"
+	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
 
@@ -25,54 +25,35 @@ func TestHandleNewNotifications(t *testing.T) {
 
 	job.handleNewNotifications()
 
-	if len(job.pendingNotifications) != 0 {
+	require.Len(t, job.pendingNotifications, 0, "shouldn't have added any pending notifications")
-		t.Fatal("shouldn't have added any pending notifications")
-	}
 
 	job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
-	if len(job.pendingNotifications) != 0 {
+	require.Len(t, job.pendingNotifications, 0, "shouldn't have added any pending notifications")
-		t.Fatal("shouldn't have added any pending notifications")
-	}
 
 	job.handleNewNotifications()
-	if len(job.pendingNotifications) != 1 {
+	require.Len(t, job.pendingNotifications, 1, "should have received posts for 1 user")
-		t.Fatal("should have received posts for 1 user")
+	require.Len(t, job.pendingNotifications[id1], 1, "should have received 1 post for user")
-	} else if len(job.pendingNotifications[id1]) != 1 {
-		t.Fatal("should have received 1 post for user")
-	}
 
 	job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
 	job.handleNewNotifications()
-	if len(job.pendingNotifications) != 1 {
+	require.Len(t, job.pendingNotifications, 1, "should have received posts for 1 user")
-		t.Fatal("should have received posts for 1 user")
+	require.Len(t, job.pendingNotifications[id1], 2, "should have received 2 posts for user1")
-	} else if len(job.pendingNotifications[id1]) != 2 {
-		t.Fatal("should have received 2 posts for user1", job.pendingNotifications[id1])
-	}
 
 	job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
 	job.handleNewNotifications()
-	if len(job.pendingNotifications) != 2 {
+	require.Len(t, job.pendingNotifications, 2, "should have received posts for 2 users")
-		t.Fatal("should have received posts for 2 users")
+	require.Len(t, job.pendingNotifications[id1], 2, "should have received 2 posts for user1")
-	} else if len(job.pendingNotifications[id1]) != 2 {
+	require.Len(t, job.pendingNotifications[id2], 1, "should have received 1 post for user2")
-		t.Fatal("should have received 2 posts for user1")
-	} else if len(job.pendingNotifications[id2]) != 1 {
-		t.Fatal("should have received 1 post for user2")
-	}
 
 	job.Add(&model.User{Id: id2}, &model.Post{UserId: id2, Message: "test"}, &model.Team{Name: "team"})
 	job.Add(&model.User{Id: id1}, &model.Post{UserId: id3, Message: "test"}, &model.Team{Name: "team"})
 	job.Add(&model.User{Id: id3}, &model.Post{UserId: id3, Message: "test"}, &model.Team{Name: "team"})
 	job.Add(&model.User{Id: id2}, &model.Post{UserId: id2, Message: "test"}, &model.Team{Name: "team"})
 	job.handleNewNotifications()
-	if len(job.pendingNotifications) != 3 {
+	require.Len(t, job.pendingNotifications, 3, "should have received posts for 3 users")
-		t.Fatal("should have received posts for 3 users")
+	require.Len(t, job.pendingNotifications[id1], 3, "should have received 3 posts for user1")
-	} else if len(job.pendingNotifications[id1]) != 3 {
+	require.Len(t, job.pendingNotifications[id2], 3, "should have received 3 posts for user2")
-		t.Fatal("should have received 3 posts for user1")
+	require.Len(t, job.pendingNotifications[id3], 1, "should have received 1 post for user3")
-	} else if len(job.pendingNotifications[id2]) != 3 {
-		t.Fatal("should have received 3 posts for user2")
-	} else if len(job.pendingNotifications[id3]) != 1 {
-		t.Fatal("should have received 1 post for user3")
-	}
 
 	// test ordering of received posts
 	job = NewEmailBatchingJob(th.Server, 128)
@@ -83,14 +64,11 @@ func TestHandleNewNotifications(t *testing.T) {
 	job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test4"}, &model.Team{Name: "team"})
 	job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test5"}, &model.Team{Name: "team"})
 	job.handleNewNotifications()
-	if job.pendingNotifications[id1][0].post.Message != "test1" ||
+	assert.Equal(t, job.pendingNotifications[id1][0].post.Message, "test1", "incorrect order of received posts for user1");
-		job.pendingNotifications[id1][1].post.Message != "test2" ||
+	assert.Equal(t, job.pendingNotifications[id1][1].post.Message, "test2", "incorrect order of received posts for user1");
-		job.pendingNotifications[id1][2].post.Message != "test4" {
+	assert.Equal(t, job.pendingNotifications[id1][2].post.Message, "test4", "incorrect order of received posts for user1");
-		t.Fatal("incorrect order of received posts for user1")
+	assert.Equal(t, job.pendingNotifications[id2][0].post.Message, "test3", "incorrect order of received posts for user2");
-	} else if job.pendingNotifications[id2][0].post.Message != "test3" ||
+	assert.Equal(t, job.pendingNotifications[id2][1].post.Message, "test5", "incorrect order of received posts for user2");
-		job.pendingNotifications[id2][1].post.Message != "test5" {
-		t.Fatal("incorrect order of received posts for user2")
-	}
 }
 
 func TestCheckPendingNotifications(t *testing.T) {
@@ -126,9 +104,8 @@ func TestCheckPendingNotifications(t *testing.T) {
 	// test that notifications aren't sent before interval
 	job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
 
-	if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 {
+	require.NotNil(t, job.pendingNotifications[th.BasicUser.Id])
-		t.Fatal("shouldn't have sent queued post")
+	require.Len(t, job.pendingNotifications[th.BasicUser.Id], 1, "shouldn't have sent queued post")
-	}
 
 	// test that notifications are cleared if the user has acted
 	channelMember, err = th.App.Srv.Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id)
@@ -139,9 +116,8 @@ func TestCheckPendingNotifications(t *testing.T) {
 
 	job.checkPendingNotifications(time.Unix(10002, 0), func(string, []*batchedNotification) {})
 
-	if job.pendingNotifications[th.BasicUser.Id] != nil && len(job.pendingNotifications[th.BasicUser.Id]) != 0 {
+	require.Nil(t, job.pendingNotifications[th.BasicUser.Id])
-		t.Fatal("should've remove queued post since user acted")
+	require.Len(t, job.pendingNotifications[th.BasicUser.Id], 0, "should've remove queued post since user acted")
-	}
 
 	// test that notifications are sent if enough time passes since the first message
 	job.pendingNotifications[th.BasicUser.Id] = []*batchedNotification{
@@ -180,26 +156,22 @@ func TestCheckPendingNotifications(t *testing.T) {
 		timeout <- true
 	}()
 
-	if job.pendingNotifications[th.BasicUser.Id] != nil && len(job.pendingNotifications[th.BasicUser.Id]) != 0 {
+	require.Nil(t, job.pendingNotifications[th.BasicUser.Id], "shouldn't have sent queued post")
-		t.Fatal("should've remove queued posts when sending messages")
+
+	select {
+	case post := <-received:
+		require.Equal(t, post.Message, "post1", "should've received post1 first")
+
+	case <-timeout:
+		require.Fail(t, "timed out waiting for first post notification")
 	}
 
 	select {
 	case post := <-received:
-		if post.Message != "post1" {
+		require.Equal(t, post.Message, "post2", "should've received post2 second")
-			t.Fatal("should've received post1 first")
-		}
-	case <-timeout:
-		t.Fatal("timed out waiting for first post notification")
-	}
 
-	select {
-	case post := <-received:
-		if post.Message != "post2" {
-			t.Fatal("should've received post2 second")
-		}
 	case <-timeout:
-		t.Fatal("timed out waiting for second post notification")
+		require.Fail(t, "timed out waiting for second post notification")
 	}
 }
 
@@ -232,15 +204,13 @@ func TestCheckPendingNotificationsDefaultInterval(t *testing.T) {
 
 	// notifications should not be sent 1s after post was created, because default batch interval is 15mins
 	job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
-	if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 {
+	require.NotNil(t, job.pendingNotifications[th.BasicUser.Id])
-		t.Fatal("shouldn't have sent queued post")
+	require.Len(t, job.pendingNotifications[th.BasicUser.Id], 1, "shouldn't have sent queued post")
-	}
 
 	// notifications should be sent 901s after post was created, because default batch interval is 15mins
 	job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {})
-	if job.pendingNotifications[th.BasicUser.Id] != nil || len(job.pendingNotifications[th.BasicUser.Id]) != 0 {
+	require.Nil(t, job.pendingNotifications[th.BasicUser.Id])
-		t.Fatal("should have sent queued post")
+	require.Len(t, job.pendingNotifications[th.BasicUser.Id], 0, "should have sent queued post")
-	}
 }
 
 /**
@@ -281,15 +251,13 @@ func TestCheckPendingNotificationsCantParseInterval(t *testing.T) {
 
 	// notifications should not be sent 1s after post was created, because default batch interval is 15mins
 	job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
-	if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 {
+	require.NotNil(t, job.pendingNotifications[th.BasicUser.Id])
-		t.Fatal("shouldn't have sent queued post")
+	require.Len(t, job.pendingNotifications[th.BasicUser.Id], 1, "shouldn't have sent queued post")
-	}
 
 	// notifications should be sent 901s after post was created, because default batch interval is 15mins
 	job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {})
-	if job.pendingNotifications[th.BasicUser.Id] != nil || len(job.pendingNotifications[th.BasicUser.Id]) != 0 {
+
-		t.Fatal("should have sent queued post")
+	require.Nil(t, job.pendingNotifications[th.BasicUser.Id], "should have sent queued post")
-	}
 }
 
 /*
@@ -314,9 +282,7 @@ func TestRenderBatchedPostGeneric(t *testing.T) {
 	}
 
 	var rendered = th.Server.renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_GENERIC)
-	if strings.Contains(rendered, post.Message) {
+	require.NotContains(t, rendered, post.Message, "Rendered email should not contain post contents when email notification contents type is set to Generic.")
-		t.Fatal("Rendered email should not contain post contents when email notification contents type is set to Generic.")
-	}
 }
 
 /*
@@ -341,7 +307,5 @@ func TestRenderBatchedPostFull(t *testing.T) {
 	}
 
 	var rendered = th.Server.renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_FULL)
-	if !strings.Contains(rendered, post.Message) {
+	require.Contains(t, rendered, post.Message, "Rendered email should contain post contents when email notification contents type is set to Full.")
-		t.Fatal("Rendered email should contain post contents when email notification contents type is set to Full.")
-	}
 }

app/import_functions.go

@@ -6,7 +6,6 @@ package app
 import (
 	"bytes"
 	"crypto/sha1"
-	"fmt"
 	"io"
 	"net/http"
 	"os"
@@ -935,7 +934,7 @@ func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, tea
 					oldHash := sha1.Sum(oldFileData)
 
 					if bytes.Equal(oldHash[:], newHash[:]) {
-						mlog.Info(fmt.Sprintf("Skipping uploading of file with name %s, already exists", file.Name()))
+						mlog.Info("Skipping uploading of file because name already exists", mlog.Any("file_name", file.Name()))
 						return nil, nil
 					}
 
@@ -947,13 +946,13 @@ func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, tea
 		fileInfo, err := a.DoUploadFile(timestamp, teamId, post.ChannelId, post.UserId, file.Name(), buf.Bytes())
 
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Failed to upload file: %s", err.Error()))
+			mlog.Error("Failed to upload file:", mlog.Err(err))
 			return nil, err
 		}
 
 		a.HandleImages([]string{fileInfo.PreviewPath}, []string{fileInfo.ThumbnailPath}, [][]byte{buf.Bytes()})
 
-		mlog.Info(fmt.Sprintf("uploading file with name %s", file.Name()))
+		mlog.Info("Uploading file with name", mlog.String("file_name", file.Name()))
 		return fileInfo, nil
 	}
 	return nil, model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest)
@@ -1091,7 +1090,7 @@ func (a *App) uploadAttachments(attachments *[]AttachmentImportData, post *model
 func (a *App) UpdateFileInfoWithPostId(post *model.Post) {
 	for _, fileId := range post.FileIds {
 		if err := a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); err != nil {
-			mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, err), mlog.String("post_id", post.Id))
+			mlog.Error("Error attaching files to post.", mlog.String("post_id", post.Id), mlog.Any("post_file_ids", post.FileIds), mlog.Err(err))
 		}
 	}
 }

app/login_test.go

@@ -6,6 +6,8 @@ package app
 import (
 	"net/http"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestCheckForClientSideCert(t *testing.T) {
@@ -30,8 +32,6 @@ func TestCheckForClientSideCert(t *testing.T) {
 
 		_, _, actualEmail := th.App.CheckForClientSideCert(r)
 
-		if actualEmail != tt.expectedEmail {
+		require.Equal(t, actualEmail, tt.expectedEmail, "CheckForClientSideCert(%v): expected %v, actual %v", tt.subject, tt.expectedEmail, actualEmail)
-			t.Fatalf("CheckForClientSideCert(%v): expected %v, actual %v", tt.subject, tt.expectedEmail, actualEmail)
-		}
 	}
 }

app/notification_email_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/services/timezones"
@@ -28,9 +29,7 @@ func TestGetDirectMessageNotificationEmailSubject(t *testing.T) {
 	}
 	translateFunc := utils.GetUserTranslations("en")
 	subject := getDirectMessageNotificationEmailSubject(user, post, translateFunc, "http://localhost:8065", "@sender", true)
-	if !strings.HasPrefix(subject, expectedPrefix) {
+	require.Regexp(t, regexp.MustCompile("^"+regexp.QuoteMeta(expectedPrefix)), subject, fmt.Sprintf("Expected subject line prefix '%s', got %s", expectedPrefix, subject))
-		t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject)
-	}
 }
 
 func TestGetGroupMessageNotificationEmailSubjectFull(t *testing.T) {
@@ -45,9 +44,7 @@ func TestGetGroupMessageNotificationEmailSubjectFull(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 	emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL
 	subject := getGroupMessageNotificationEmailSubject(user, post, translateFunc, "http://localhost:8065", "sender", emailNotificationContentsType, true)
-	if !strings.HasPrefix(subject, expectedPrefix) {
+	require.Regexp(t, regexp.MustCompile("^"+regexp.QuoteMeta(expectedPrefix)), subject, fmt.Sprintf("Expected subject line prefix '%s', got %s", expectedPrefix, subject))
-		t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject)
-	}
 }
 
 func TestGetGroupMessageNotificationEmailSubjectGeneric(t *testing.T) {
@@ -62,9 +59,7 @@ func TestGetGroupMessageNotificationEmailSubjectGeneric(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 	emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC
 	subject := getGroupMessageNotificationEmailSubject(user, post, translateFunc, "http://localhost:8065", "sender", emailNotificationContentsType, true)
-	if !strings.HasPrefix(subject, expectedPrefix) {
+	require.Regexp(t, regexp.MustCompile("^"+regexp.QuoteMeta(expectedPrefix)), subject, fmt.Sprintf("Expected subject line prefix '%s', got %s", expectedPrefix, subject))
-		t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject)
-	}
 }
 
 func TestGetNotificationEmailSubject(t *testing.T) {
@@ -78,9 +73,7 @@ func TestGetNotificationEmailSubject(t *testing.T) {
 	}
 	translateFunc := utils.GetUserTranslations("en")
 	subject := getNotificationEmailSubject(user, post, translateFunc, "http://localhost:8065", "team", true)
-	if !strings.HasPrefix(subject, expectedPrefix) {
+	require.Regexp(t, regexp.MustCompile("^"+regexp.QuoteMeta(expectedPrefix)), subject, fmt.Sprintf("Expected subject line prefix '%s', got %s", expectedPrefix, subject))
-		t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationPublicChannel(t *testing.T) {
@@ -103,21 +96,11 @@ func TestGetNotificationEmailBodyFullNotificationPublicChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification.") {
+	require.Contains(t, body, "You have a new notification.", fmt.Sprintf("Expected email text 'You have a new notification. Got %s", body))
-		t.Fatal("Expected email text 'You have a new notification. Got " + body)
+	require.Contains(t, body, "Channel: "+channel.DisplayName, "Expected email text 'Channel: %s'. Got %s", channel.DisplayName, body)
-	}
+	require.Contains(t, body, senderName+" - ", fmt.Sprintf("Expected email text '%s - '. Got %s", senderName, body))
-	if !strings.Contains(body, "Channel: "+channel.DisplayName) {
+	require.Contains(t, body, post.Message, fmt.Sprintf("Expected email text '%s'. Got %s", post.Message, body))
-		t.Fatal("Expected email text 'Channel: " + channel.DisplayName + "'. Got " + body)
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-	}
-	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
-	}
-	if !strings.Contains(body, post.Message) {
-		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationGroupChannel(t *testing.T) {
@@ -140,21 +123,11 @@ func TestGetNotificationEmailBodyFullNotificationGroupChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Group Message.") {
+	require.Contains(t, body, "You have a new Group Message.", fmt.Sprintf("Expected email text 'You have a new Group Message. Got "+body))
-		t.Fatal("Expected email text 'You have a new Group Message. Got " + body)
+	require.Contains(t, body, "Channel: ChannelName", fmt.Sprintf("Expected email text 'Channel: ChannelName'. Got %s", body))
-	}
+	require.Contains(t, body, senderName+" - ", fmt.Sprintf("Expected email text '%s - '. Got %s", senderName, body))
-	if !strings.Contains(body, "Channel: ChannelName") {
+	require.Contains(t, body, post.Message, fmt.Sprintf("Expected email text '%s'. Got %s", post.Message, body))
-		t.Fatal("Expected email text 'Channel: ChannelName'. Got " + body)
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-	}
-	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
-	}
-	if !strings.Contains(body, post.Message) {
-		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationPrivateChannel(t *testing.T) {
@@ -177,21 +150,11 @@ func TestGetNotificationEmailBodyFullNotificationPrivateChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification.") {
+	require.Contains(t, body, "You have a new notification.", fmt.Sprintf("Expected email text 'You have a new notification. Got "+body))
-		t.Fatal("Expected email text 'You have a new notification. Got " + body)
+	require.Contains(t, body, "Channel: "+channel.DisplayName, fmt.Sprintf("Expected email text 'Channel: "+channel.DisplayName+"'. Got "+body))
-	}
+	require.Contains(t, body, senderName+" - ", fmt.Sprintf("Expected email text '%s - '. Got %s", senderName, body))
-	if !strings.Contains(body, "Channel: "+channel.DisplayName) {
+	require.Contains(t, body, post.Message, fmt.Sprintf("Expected email text '%s'. Got %s", post.Message, body))
-		t.Fatal("Expected email text 'Channel: " + channel.DisplayName + "'. Got " + body)
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-	}
-	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
-	}
-	if !strings.Contains(body, post.Message) {
-		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationDirectChannel(t *testing.T) {
@@ -214,18 +177,10 @@ func TestGetNotificationEmailBodyFullNotificationDirectChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Direct Message.") {
+	require.Contains(t, body, "You have a new Direct Message.", fmt.Sprintf("Expected email text 'You have a new Direct Message. Got "+body))
-		t.Fatal("Expected email text 'You have a new Direct Message. Got " + body)
+	require.Contains(t, body, senderName+" - ", fmt.Sprintf("Expected email text '%s - '. Got %s", senderName, body))
-	}
+	require.Contains(t, body, post.Message, fmt.Sprintf("Expected email text '%s'. Got %s", post.Message, body))
-	if !strings.Contains(body, senderName+" - ") {
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
-	}
-	if !strings.Contains(body, post.Message) {
-		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationLocaleTimeWithTimezone(t *testing.T) {
@@ -254,9 +209,7 @@ func TestGetNotificationEmailBodyFullNotificationLocaleTimeWithTimezone(t *testi
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, false, translateFunc)
 	r, _ := regexp.Compile("E([S|D]+)T")
 	zone := r.FindString(body)
-	if !strings.Contains(body, "sender - 9:43 AM "+zone+", April 25") {
+	require.Contains(t, body, "sender - 9:43 AM "+zone+", April 25", fmt.Sprintf("Expected email text 'sender - 9:43 AM %s, April 25'. Got %s", zone, body))
-		t.Fatal("Expected email text 'sender - 9:43 AM " + zone + ", April 25'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationLocaleTimeNoTimezone(t *testing.T) {
@@ -296,9 +249,7 @@ func TestGetNotificationEmailBodyFullNotificationLocaleTimeNoTimezone(t *testing
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
 	postTimeLine := fmt.Sprintf("sender - %s:%s %s, %s %s", formattedTime.Hour, formattedTime.Minute, formattedTime.TimeZone, formattedTime.Month, formattedTime.Day)
-	if !strings.Contains(body, postTimeLine) {
+	require.Contains(t, body, postTimeLine, fmt.Sprintf("Expected email text '%s'. Got %s", postTimeLine, body))
-		t.Fatal("Expected email text '" + postTimeLine + " '. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationLocaleTime12Hour(t *testing.T) {
@@ -325,12 +276,8 @@ func TestGetNotificationEmailBodyFullNotificationLocaleTime12Hour(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, false, translateFunc)
-	if !strings.Contains(body, "sender - 2:30 PM") {
+	require.Contains(t, body, "sender - 2:30 PM", fmt.Sprintf("Expected email text 'sender - 2:30 PM'. Got %s", body))
-		t.Fatal("Expected email text 'sender - 2:30 PM'. Got " + body)
+	require.Contains(t, body, "April 25", fmt.Sprintf("Expected email text 'April 25'. Got %s", body))
-	}
-	if !strings.Contains(body, "April 25") {
-		t.Fatal("Expected email text 'April 25'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyFullNotificationLocaleTime24Hour(t *testing.T) {
@@ -357,12 +304,8 @@ func TestGetNotificationEmailBodyFullNotificationLocaleTime24Hour(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "sender - 14:30") {
+	require.Contains(t, body, "sender - 14:30", fmt.Sprintf("Expected email text 'sender - 14:30'. Got %s", body))
-		t.Fatal("Expected email text 'sender - 14:30'. Got " + body)
+	require.Contains(t, body, "April 25", fmt.Sprintf("Expected email text 'April 25'. Got %s", body))
-	}
-	if !strings.Contains(body, "April 25") {
-		t.Fatal("Expected email text 'April 25'. Got " + body)
-	}
 }
 
 // from here
@@ -386,18 +329,10 @@ func TestGetNotificationEmailBodyGenericNotificationPublicChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification from "+senderName) {
+	require.Contains(t, body, "You have a new notification from "+senderName, fmt.Sprintf("Expected email text 'You have a new notification from %s'. Got %s", senderName, body))
-		t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body)
+	require.False(t, strings.Contains(body, "Channel: "+channel.DisplayName), fmt.Sprintf("Did not expect email text 'CHANNEL: %s'. Got %s", channel.DisplayName, body))
-	}
+	require.False(t, strings.Contains(body, post.Message), fmt.Sprintf("Did not expect email text '%s'. Got %s", post.Message, body))
-	if strings.Contains(body, "Channel: "+channel.DisplayName) {
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-		t.Fatal("Did not expect email text 'Channel: " + channel.DisplayName + "'. Got " + body)
-	}
-	if strings.Contains(body, post.Message) {
-		t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyGenericNotificationGroupChannel(t *testing.T) {
@@ -420,18 +355,10 @@ func TestGetNotificationEmailBodyGenericNotificationGroupChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Group Message from "+senderName) {
+	require.Contains(t, body, "You have a new Group Message from "+senderName, fmt.Sprintf("Expected email text 'You have a new Group Message from %s'. Got %s", senderName, body))
-		t.Fatal("Expected email text 'You have a new Group Message from " + senderName + "'. Got " + body)
+	require.False(t, strings.Contains(body, "CHANNEL: "+channel.DisplayName), fmt.Sprintf("Did not expect email text 'CHANNEL: %s'. Got %s", channel.DisplayName, body))
-	}
+	require.False(t, strings.Contains(body, post.Message), fmt.Sprintf("Did not expect email text '%s'. Got %s", post.Message, body))
-	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)
-	}
-	if strings.Contains(body, post.Message) {
-		t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyGenericNotificationPrivateChannel(t *testing.T) {
@@ -454,18 +381,10 @@ func TestGetNotificationEmailBodyGenericNotificationPrivateChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification from "+senderName) {
+	require.Contains(t, body, "You have a new notification from "+senderName, fmt.Sprintf("Expected email text 'You have a new notification from %s'. Got %s", senderName, body))
-		t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body)
+	require.False(t, strings.Contains(body, "CHANNEL: "+channel.DisplayName), fmt.Sprintf("Did not expect email text 'CHANNEL: %s'. Got %s", channel.DisplayName, body))
-	}
+	require.False(t, strings.Contains(body, post.Message), fmt.Sprintf("Did not expect email text '%s'. Got %s", post.Message, body))
-	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)
-	}
-	if strings.Contains(body, post.Message) {
-		t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T) {
@@ -488,18 +407,10 @@ func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Direct Message from "+senderName) {
+	require.Contains(t, body, "You have a new Direct Message from "+senderName, fmt.Sprintf("Expected email text 'You have a new Direct Message from "+senderName+"'. Got "+body))
-		t.Fatal("Expected email text 'You have a new Direct Message from " + senderName + "'. Got " + body)
+	require.False(t, strings.Contains(body, "CHANNEL: "+channel.DisplayName), fmt.Sprintf("Did not expect email text 'CHANNEL: %s'. Got %s", channel.DisplayName, body))
-	}
+	require.False(t, strings.Contains(body, post.Message), fmt.Sprintf("Did not expect email text '%s'. Got %s", post.Message, body))
-	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
+	require.Contains(t, body, teamURL, fmt.Sprintf("Expected email text '%s'. Got %s", teamURL, body))
-		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)
-	}
-	if strings.Contains(body, post.Message) {
-		t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body)
-	}
-	if !strings.Contains(body, teamURL) {
-		t.Fatal("Expected email text '" + teamURL + "'. Got " + body)
-	}
 }
 
 func TestGetNotificationEmailEscapingChars(t *testing.T) {

app/notification_push.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"hash/fnv"
 	"net/http"
 	"strings"
@@ -174,7 +173,7 @@ func (a *App) getPushNotificationMessage(postMessage string, explicitMention, ch
 func (a *App) ClearPushNotificationSync(currentSessionId, userId, channelId string) {
 	sessions, err := a.getMobileAppSessions(userId)
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("error getting mobile app sessions", mlog.Err(err))
 		return
 	}
 
@@ -187,7 +186,7 @@ func (a *App) ClearPushNotificationSync(currentSessionId, userId, channelId stri
 
 	if unreadCount, err := a.Srv.Store.User().GetUnreadCount(userId); err != nil {
 		msg.Badge = 0
-		mlog.Error(fmt.Sprint("We could not get the unread message count for the user", userId, err), mlog.String("user_id", userId))
+		mlog.Error("We could not get the unread message count for", mlog.String("user_id", userId), mlog.Err(err))
 	} else {
 		msg.Badge = int(unreadCount)
 	}
@@ -267,7 +266,7 @@ func (a *App) pushNotificationWorker(notifications chan PushNotification) {
 				notification.replyToThreadType,
 			)
 		default:
-			mlog.Error(fmt.Sprintf("Invalid notification type %v", notification.notificationType))
+			mlog.Error("Invalid notification type", mlog.String("notification_type", string(notification.notificationType)))
 		}
 	}
 }
@@ -446,14 +445,14 @@ func (a *App) BuildPushNotificationMessage(post *model.Post, user *model.User, c
 	if user.NotifyProps["push"] == "all" {
 		if unreadCount, err := a.Srv.Store.User().GetAnyUnreadPostCountForChannel(user.Id, channel.Id); err != nil {
 			msg.Badge = 1
-			mlog.Error(fmt.Sprint("We could not get the unread message count for the user", user.Id, err), mlog.String("user_id", user.Id))
+			mlog.Error("We could not get the unread message count for the user", mlog.String("user_id", user.Id), mlog.Err(err))
 		} else {
 			msg.Badge = int(unreadCount)
 		}
 	} else {
 		if unreadCount, err := a.Srv.Store.User().GetUnreadCount(user.Id); err != nil {
 			msg.Badge = 1
-			mlog.Error(fmt.Sprint("We could not get the unread message count for the user", user.Id, err), mlog.String("user_id", user.Id))
+			mlog.Error("We could not get the unread message count for the user", mlog.String("user_id", user.Id), mlog.Err(err))
 		} else {
 			msg.Badge = int(unreadCount)
 		}

app/oauth_test.go

@@ -69,9 +69,8 @@ func TestOAuthRevokeAccessToken(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()
 
-	if err := th.App.RevokeAccessToken(model.NewRandomString(16)); err == nil {
+	err := th.App.RevokeAccessToken(model.NewRandomString(16))
-		t.Fatal("Should have failed bad token")
+	require.NotNil(t, err, "Should have failed bad token")
-	}
 
 	session := &model.Session{}
 	session.CreateAt = model.GetMillis()
@@ -81,9 +80,8 @@ func TestOAuthRevokeAccessToken(t *testing.T) {
 	session.SetExpireInDays(1)
 
 	session, _ = th.App.CreateSession(session)
-	if err := th.App.RevokeAccessToken(session.Token); err == nil {
+	err = th.App.RevokeAccessToken(session.Token)
-		t.Fatal("Should have failed does not have an access token")
+	require.NotNil(t, err, "Should have failed does not have an access token")
-	}
 
 	accessData := &model.AccessData{}
 	accessData.Token = session.Token
@@ -92,12 +90,11 @@ func TestOAuthRevokeAccessToken(t *testing.T) {
 	accessData.ClientId = model.NewId()
 	accessData.ExpiresAt = session.ExpiresAt
 
-	_, err := th.App.Srv.Store.OAuth().SaveAccessData(accessData)
+	_, err = th.App.Srv.Store.OAuth().SaveAccessData(accessData)
 	require.Nil(t, err)
 
-	if err = th.App.RevokeAccessToken(accessData.Token); err != nil {
+	err = th.App.RevokeAccessToken(accessData.Token)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func TestOAuthDeleteApp(t *testing.T) {
@@ -114,9 +111,7 @@ func TestOAuthDeleteApp(t *testing.T) {
 
 	var err *model.AppError
 	a1, err = th.App.CreateOAuthApp(a1)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
 
 	session := &model.Session{}
 	session.CreateAt = model.GetMillis()
@@ -138,13 +133,11 @@ func TestOAuthDeleteApp(t *testing.T) {
 	_, err = th.App.Srv.Store.OAuth().SaveAccessData(accessData)
 	require.Nil(t, err)
 
-	if err = th.App.DeleteOAuthApp(a1.Id); err != nil {
+	err = th.App.DeleteOAuthApp(a1.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if _, err = th.App.GetSession(session.Token); err == nil {
+	_, err = th.App.GetSession(session.Token)
-		t.Fatal("should not get session from cache or db")
+	require.NotNil(t, err, "should not get session from cache or db")
-	}
 }
 
 func TestAuthorizeOAuthUser(t *testing.T) {

app/post_test.go

@@ -259,22 +259,17 @@ func TestUpdatePostEditAt(t *testing.T) {
 	*post = *th.BasicPost
 
 	post.IsPinned = true
-	if saved, err := th.App.UpdatePost(post, true); err != nil {
+	saved, err := th.App.UpdatePost(post, true)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if saved.EditAt != post.EditAt {
+	assert.Equal(t, saved.EditAt, post.EditAt, "shouldn't have updated post.EditAt when pinning post")
-		t.Fatal("shouldn't have updated post.EditAt when pinning post")
+	*post = *saved
-
-		*post = *saved
-	}
 
 	time.Sleep(time.Millisecond * 100)
 
 	post.Message = model.NewId()
-	if saved, err := th.App.UpdatePost(post, true); err != nil {
+	saved, err = th.App.UpdatePost(post, true)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if saved.EditAt == post.EditAt {
+	assert.NotEqual(t, saved.EditAt, post.EditAt, "should have updated post.EditAt when updating post message")
-		t.Fatal("should have updated post.EditAt when updating post message")
-	}
 
 	time.Sleep(time.Millisecond * 200)
 }
@@ -291,25 +286,23 @@ func TestUpdatePostTimeLimit(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.ServiceSettings.PostEditTimeLimit = -1
 	})
-	if _, err := th.App.UpdatePost(post, true); err != nil {
+	_, err := th.App.UpdatePost(post, true)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.ServiceSettings.PostEditTimeLimit = 1000000000
 	})
 	post.Message = model.NewId()
-	if _, err := th.App.UpdatePost(post, true); err != nil {
+
-		t.Fatal("should allow you to edit the post")
+	_, err = th.App.UpdatePost(post, true)
-	}
+	require.Nil(t, err, "should allow you to edit the post")
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.ServiceSettings.PostEditTimeLimit = 1
 	})
 	post.Message = model.NewId()
-	if _, err := th.App.UpdatePost(post, true); err == nil {
+	_, err = th.App.UpdatePost(post, true)
-		t.Fatal("should fail on update old post")
+	require.NotNil(t, err, "should fail on update old post")
-	}
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.ServiceSettings.PostEditTimeLimit = -1
@@ -340,14 +333,11 @@ func TestPostReplyToPostWhereRootPosterLeftChannel(t *testing.T) {
 	userNotInChannel := th.BasicUser
 	rootPost := th.BasicPost
 
-	if _, err := th.App.AddUserToChannel(userInChannel, channel); err != nil {
+	_, err := th.App.AddUserToChannel(userInChannel, channel)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
-
-	if err := th.App.RemoveUserFromChannel(userNotInChannel.Id, "", channel); err != nil {
-		t.Fatal(err)
-	}
 
+	err = th.App.RemoveUserFromChannel(userNotInChannel.Id, "", channel)
+	require.Nil(t, err)
 	replyPost := model.Post{
 		Message:       "asd",
 		ChannelId:     channel.Id,
@@ -358,9 +348,8 @@ func TestPostReplyToPostWhereRootPosterLeftChannel(t *testing.T) {
 		CreateAt:      0,
 	}
 
-	if _, err := th.App.CreatePostAsUser(&replyPost, ""); err != nil {
+	_, err = th.App.CreatePostAsUser(&replyPost, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func TestPostAttachPostToChildPost(t *testing.T) {
@@ -382,9 +371,7 @@ func TestPostAttachPostToChildPost(t *testing.T) {
 	}
 
 	res1, err := th.App.CreatePostAsUser(&replyPost1, "")
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
 
 	replyPost2 := model.Post{
 		Message:       "reply two",
@@ -397,9 +384,7 @@ func TestPostAttachPostToChildPost(t *testing.T) {
 	}
 
 	_, err = th.App.CreatePostAsUser(&replyPost2, "")
-	if err.StatusCode != http.StatusBadRequest {
+	assert.Equalf(t, err.StatusCode, http.StatusBadRequest, "Expected BadRequest error, got %v", err)
-		t.Fatal(fmt.Sprintf("Expected BadRequest error, got %v", err))
-	}
 
 	replyPost3 := model.Post{
 		Message:       "reply three",
@@ -411,9 +396,8 @@ func TestPostAttachPostToChildPost(t *testing.T) {
 		CreateAt:      0,
 	}
 
-	if _, err := th.App.CreatePostAsUser(&replyPost3, ""); err != nil {
+	_, err = th.App.CreatePostAsUser(&replyPost3, "")
-		t.Fatal(err)
+	assert.Nil(t, err)
-	}
 }
 
 func TestPostChannelMentions(t *testing.T) {
@@ -429,9 +413,7 @@ func TestPostChannelMentions(t *testing.T) {
 		Type:        model.CHANNEL_OPEN,
 		TeamId:      th.BasicTeam.Id,
 	}, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err.Error())
-	}
 	defer th.App.PermanentDeleteChannel(channelToMention)
 
 	_, err = th.App.AddUserToChannel(user, channel)
@@ -618,14 +600,11 @@ func TestDeletePostWithFileAttachments(t *testing.T) {
 	data := []byte("abcd")
 
 	info1, err := th.App.DoUploadFile(time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamId, channelId, userId, filename, data)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
-		defer func() {
+		th.App.RemoveFile(info1.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
+	}()
-			th.App.RemoveFile(info1.Path)
-		}()
-	}
 
 	post := &model.Post{
 		Message:       "asd",

app/user_agent_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/avct/uasurfer"
+	"github.com/stretchr/testify/assert"
 )
 
 type testUserAgent struct {
@@ -53,9 +54,8 @@ func TestGetPlatformName(t *testing.T) {
 		t.Run(fmt.Sprintf("GetPlatformName_%v", i), func(t *testing.T) {
 			ua := uasurfer.Parse(userAgent.UserAgent)
 
-			if actual := getPlatformName(ua); actual != expected[i] {
+			actual := getPlatformName(ua)
-				t.Fatalf("%v Got %v, expected %v", userAgent.Name, actual, expected[i])
+			assert.Equal(t, expected[i], actual)
-			}
 		})
 	}
 }
@@ -83,9 +83,8 @@ func TestGetOSName(t *testing.T) {
 		t.Run(fmt.Sprintf("GetOSName_%v", i), func(t *testing.T) {
 			ua := uasurfer.Parse(userAgent.UserAgent)
 
-			if actual := getOSName(ua); actual != expected[i] {
+			actual := getOSName(ua)
-				t.Fatalf("Got %v, expected %v", actual, expected[i])
+			assert.Equal(t, expected[i], actual)
-			}
 		})
 	}
 }
@@ -113,9 +112,8 @@ func TestGetBrowserName(t *testing.T) {
 		t.Run(fmt.Sprintf("GetBrowserName_%v", i), func(t *testing.T) {
 			ua := uasurfer.Parse(userAgent.UserAgent)
 
-			if actual := getBrowserName(ua, userAgent.UserAgent); actual != expected[i] {
+			actual := getBrowserName(ua, userAgent.UserAgent)
-				t.Fatalf("Got %v, expected %v", actual, expected[i])
+			assert.Equal(t, expected[i], actual)
-			}
 		})
 	}
 }
@@ -143,9 +141,8 @@ func TestGetBrowserVersion(t *testing.T) {
 		t.Run(fmt.Sprintf("GetBrowserVersion_%v", i), func(t *testing.T) {
 			ua := uasurfer.Parse(userAgent.UserAgent)
 
-			if actual := getBrowserVersion(ua, userAgent.UserAgent); actual != expected[i] {
+			actual := getBrowserVersion(ua, userAgent.UserAgent)
-				t.Fatalf("Got %v, expected %v", actual, expected[i])
+			assert.Equal(t, expected[i], actual)
-			}
 		})
 	}
 }

app/web_hub.go

@@ -72,7 +72,7 @@ func (a *App) TotalWebsocketConnections() int {
 func (a *App) HubStart() {
 	// Total number of hubs is twice the number of CPUs.
 	numberOfHubs := runtime.NumCPU() * 2
-	mlog.Info(fmt.Sprintf("Starting %v websocket hubs", numberOfHubs))
+	mlog.Info("Starting websocket hubs", mlog.Int("number_of_hubs", numberOfHubs))
 
 	a.Srv.Hubs = make([]*Hub, numberOfHubs)
 	a.Srv.HubsStopCheckingForDeadlock = make(chan bool, 1)
@@ -95,7 +95,12 @@ func (a *App) HubStart() {
 			case <-ticker.C:
 				for _, hub := range a.Srv.Hubs {
 					if len(hub.broadcast) >= DEADLOCK_WARN {
-						mlog.Error(fmt.Sprintf("Hub processing might be deadlock on hub %v goroutine %v with %v events in the buffer", hub.connectionIndex, hub.goroutineId, len(hub.broadcast)))
+						mlog.Error(
+							"Hub processing might be deadlock with events in the buffer",
+							mlog.Int("hub", hub.connectionIndex),
+							mlog.Int("goroutine", hub.goroutineId),
+							mlog.Int("events", len(hub.broadcast)),
+						)
 						buf := make([]byte, 1<<16)
 						runtime.Stack(buf, true)
 						output := fmt.Sprintf("%s", buf)
@@ -103,7 +108,7 @@ func (a *App) HubStart() {
 
 						for _, part := range splits {
 							if strings.Contains(part, fmt.Sprintf("%v", hub.goroutineId)) {
-								mlog.Error(fmt.Sprintf("Trace for possible deadlock goroutine %v", part))
+								mlog.Error("Trace for possible deadlock goroutine", mlog.String("trace", part))
 							}
 						}
 					}
@@ -433,7 +438,7 @@ func (h *Hub) Start() {
 
 	doStart = func() {
 		h.goroutineId = getGoroutineId()
-		mlog.Debug(fmt.Sprintf("Hub for index %v is starting with goroutine %v", h.connectionIndex, h.goroutineId))
+		mlog.Debug("Hub for index is starting with goroutine", mlog.Int("index", h.connectionIndex), mlog.Int("goroutine", h.goroutineId))
 
 		connections := newHubConnectionIndex()
 
@@ -489,7 +494,7 @@ func (h *Hub) Start() {
 						select {
 						case webCon.Send <- msg:
 						default:
-							mlog.Error(fmt.Sprintf("webhub.broadcast: cannot send, closing websocket for userId=%v", webCon.UserId))
+							mlog.Error("webhub.broadcast: cannot send, closing websocket for user", mlog.String("user_id", webCon.UserId))
 							close(webCon.Send)
 							connections.Remove(webCon)
 						}
@@ -523,7 +528,7 @@ func (h *Hub) Start() {
 	doRecover = func() {
 		if !h.ExplicitStop {
 			if r := recover(); r != nil {
-				mlog.Error(fmt.Sprintf("Recovering from Hub panic. Panic was: %v", r))
+				mlog.Error("Recovering from Hub panic.", mlog.Any("panic", r))
 			} else {
 				mlog.Error("Webhub stopped unexpectedly. Recovering.")
 			}

app/web_hub_test.go

@@ -100,6 +100,6 @@ func TestHubStopRaceCondition(t *testing.T) {
 	select {
 	case <-done:
 	case <-time.After(15 * time.Second):
-		t.Fatalf("hub call did not return within 15 seconds after stop")
+		require.FailNow(t, "hub call did not return within 15 seconds after stop")
 	}
 }

build/Jenkinsfile.branch

@@ -9,7 +9,7 @@ def platformStages = new org.mattermost.PlatformStages()
 
 def rndEE = UUID.randomUUID().toString()
 def rndTE = UUID.randomUUID().toString()
-def mmBuilderServer = 'mattermost/mattermost-build-server:feb-28-2019'
+def mmBuilderServer = 'mattermost/mattermost-build-server:sep-17-2019'
 def mmBuilderWebapp = 'mattermost/mattermost-build-webapp:oct-2-2018'
 
 pipeline {

build/Jenkinsfile.pr

@@ -77,7 +77,7 @@ pipeline {
 			}
 
 			steps {
-				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:feb-28-2019') {
+				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:sep-17-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -96,7 +96,7 @@ pipeline {
 			}
 
 			steps {
-				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:feb-28-2019') {
+				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:sep-17-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -272,7 +272,7 @@ pipeline {
 					}
 				}
 
-				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:feb-28-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:sep-17-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -326,7 +326,7 @@ pipeline {
 					}
 				}
 
-				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:feb-28-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:sep-17-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -369,7 +369,7 @@ pipeline {
 					}
 				}
 
-				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:feb-28-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:sep-17-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server

build/README.md

@@ -14,6 +14,6 @@ We have a docker image to build `mattermost-server` and it is based on Go docker
 
 In our Docker Hub Repository we have the following images:
 
-- `mattermost/mattermost-build-server:dec-7-2018` which is based on Go 1.11 you can use for MM versions <= `5.9.0`
+- `mattermost/mattermost-build-server:dec-7-2018` which is based on Go 1.11 you can use for MM versions <= `5.8.0`
-- `mattermost/mattermost-build-server:feb-28-2019` which is based on Go 1.12
+- `mattermost/mattermost-build-server:feb-28-2019` which is based on Go 1.12 you can use for MM versions >= `5.9.0` <= `5.15.0`
-
+- `mattermost/mattermost-build-server:sep-17-2019` which is based on Go 1.12.9 you can use for MM versions >= `5.16.0`

build/local-test-env.sh

@@ -43,7 +43,7 @@ up()
         -e MM_EMAILSETTINGS_SMTPSERVER="inbucket" \
         -e MM_EMAILSETTINGS_SMTPPORT="10025" \
         -e MM_ELASTICSEARCHSETTINGS_CONNECTIONURL="http://elasticsearch:9200" \
-        mattermost/mattermost-build-server:dec-7-2018 /bin/bash
+        mattermost/mattermost-build-server:sep-17-2019 /bin/bash
 }
 
 down()

config/default.go

@@ -1,4 +1,4 @@
-//go:generate go run config_generator/main.go
+//go:generate go run -mod=vendor config_generator/main.go
 
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See License.txt for license information.

config/file.go

@@ -92,6 +92,16 @@ func resolveConfigFilePath(path string) (string, error) {
 	return "", fmt.Errorf("failed to find config file %s", path)
 }
 
+// resolveFilePath uses the name if name is absolute path.
+// otherwise returns the combined path/name
+func (fs *FileStore) resolveFilePath(name string) string {
+	// Absolute paths are explicit and require no resolution.
+	if filepath.IsAbs(name) {
+		return name
+	}
+	return filepath.Join(filepath.Dir(fs.path), name)
+}
+
 // Set replaces the current configuration in its entirety and updates the backing store.
 func (fs *FileStore) Set(newCfg *model.Config) (*model.Config, error) {
 	return fs.commonStore.set(newCfg, true, func(cfg *model.Config) error {
@@ -160,7 +170,7 @@ func (fs *FileStore) Load() (err error) {
 
 // GetFile fetches the contents of a previously persisted configuration file.
 func (fs *FileStore) GetFile(name string) ([]byte, error) {
-	resolvedPath := filepath.Join(filepath.Dir(fs.path), name)
+	resolvedPath := fs.resolveFilePath(name)
 
 	data, err := ioutil.ReadFile(resolvedPath)
 	if err != nil {
@@ -172,7 +182,7 @@ func (fs *FileStore) GetFile(name string) ([]byte, error) {
 
 // SetFile sets or replaces the contents of a configuration file.
 func (fs *FileStore) SetFile(name string, data []byte) error {
-	resolvedPath := filepath.Join(filepath.Dir(fs.path), name)
+	resolvedPath := fs.resolveFilePath(name)
 
 	err := ioutil.WriteFile(resolvedPath, data, 0777)
 	if err != nil {
@@ -188,7 +198,7 @@ func (fs *FileStore) HasFile(name string) (bool, error) {
 		return false, nil
 	}
 
-	resolvedPath := filepath.Join(filepath.Dir(fs.path), name)
+	resolvedPath := fs.resolveFilePath(name)
 
 	_, err := os.Stat(resolvedPath)
 	if err != nil && os.IsNotExist(err) {
@@ -202,6 +212,11 @@ func (fs *FileStore) HasFile(name string) (bool, error) {
 
 // RemoveFile removes a previously persisted configuration file.
 func (fs *FileStore) RemoveFile(name string) error {
+	if filepath.IsAbs(name) {
+		// Don't delete absolute filenames, as may be mounted drive, etc.
+		mlog.Debug("Skipping removal of configuration file with absolute path", mlog.String("filename", name))
+		return nil
+	}
 	resolvedPath := filepath.Join(filepath.Dir(fs.path), name)
 
 	err := os.Remove(resolvedPath)

config/file_test.go

@@ -890,6 +890,17 @@ func TestFileGetFile(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, []byte("test"), data)
 	})
+
+	t.Run("get via absolute path", func(t *testing.T) {
+		err := fs.SetFile("new", []byte("new file"))
+		require.NoError(t, err)
+
+		data, err := fs.GetFile(filepath.Join(filepath.Dir(path), "new"))
+
+		require.NoError(t, err)
+		require.Equal(t, []byte("new file"), data)
+	})
+
 }
 
 func TestFileSetFile(t *testing.T) {
@@ -920,6 +931,18 @@ func TestFileSetFile(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, []byte("overwritten file"), data)
 	})
+
+	t.Run("set via absolute path", func(t *testing.T) {
+		absolutePath := filepath.Join(filepath.Dir(path), "new")
+		err := fs.SetFile(absolutePath, []byte("new file"))
+		require.NoError(t, err)
+
+		data, err := fs.GetFile("new")
+
+		require.NoError(t, err)
+		require.Equal(t, []byte("new file"), data)
+	})
+
 }
 
 func TestFileHasFile(t *testing.T) {
@@ -987,6 +1010,23 @@ func TestFileHasFile(t *testing.T) {
 		require.NoError(t, err)
 		require.False(t, has)
 	})
+
+	t.Run("has via absolute path", func(t *testing.T) {
+		path, tearDown := setupConfigFile(t, minimalConfig)
+		defer tearDown()
+
+		fs, err := config.NewFileStore(path, true)
+		require.NoError(t, err)
+		defer fs.Close()
+
+		err = fs.SetFile("existing", []byte("existing file"))
+		require.NoError(t, err)
+
+		has, err := fs.HasFile(filepath.Join(filepath.Dir(path), "existing"))
+		require.NoError(t, err)
+		require.True(t, has)
+	})
+
 }
 
 func TestFileRemoveFile(t *testing.T) {
@@ -1052,6 +1092,27 @@ func TestFileRemoveFile(t *testing.T) {
 		_, err = fs.GetFile("existing")
 		require.Error(t, err)
 	})
+
+	t.Run("don't remove via absolute path", func(t *testing.T) {
+		path, tearDown := setupConfigFile(t, minimalConfig)
+		defer tearDown()
+
+		fs, err := config.NewFileStore(path, true)
+		require.NoError(t, err)
+		defer fs.Close()
+
+		err = fs.SetFile("existing", []byte("existing file"))
+		require.NoError(t, err)
+
+		filename := filepath.Join(filepath.Dir(path), "existing")
+		err = fs.RemoveFile(filename)
+		require.NoError(t, err)
+
+		has, err := fs.HasFile(filename)
+		require.NoError(t, err)
+		require.True(t, has)
+
+	})
 }
 
 func TestFileStoreString(t *testing.T) {

go.mod

@@ -16,7 +16,6 @@ require (
 	github.com/fortytw2/leaktest v1.3.0 // indirect
 	github.com/fsnotify/fsnotify v1.4.7
 	github.com/go-gorp/gorp v2.0.0+incompatible // indirect
-	github.com/go-ldap/ldap v3.0.3+incompatible // indirect
 	github.com/go-redis/redis v6.15.2+incompatible
 	github.com/go-sql-driver/mysql v1.4.1
 	github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0

go.sum

@@ -3,8 +3,6 @@ cloud.google.com/go v0.31.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cloud.google.com/go v0.37.1/go.mod h1:SAbnLi6YTSPKSI0dTUEOVLCkyPfKXK8n4ibqiMoj4ok=
 contrib.go.opencensus.io/exporter/ocagent v0.4.9/go.mod h1:ueLzZcP7LPhPulEBukGn4aLh7Mx9YJwpVJ9nL2FYltw=
-git.apache.org/thrift.git v0.12.0/go.mod h1:fPE2ZNJGynbRyZ4dJvy6G277gSllfV2HJqblrnkyeyg=
-github.com/apache/thrift v0.0.0-20180902110319-2566ecd5d999/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/Azure/azure-sdk-for-go v26.5.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/go-autorest v11.5.2+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
@@ -25,6 +23,7 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuy
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c=
 github.com/apache/thrift v0.0.0-20180902110319-2566ecd5d999/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
+github.com/apache/thrift v0.0.0-20180902110319-2566ecd5d999/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da h1:8GUt8eRujhVEGZFFEjBj46YV4rDjvGrNxb0KMWYkL2I=
@@ -88,8 +87,6 @@ github.com/gliderlabs/ssh v0.1.1/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aev
 github.com/go-gorp/gorp v2.0.0+incompatible h1:dIQPsBtl6/H1MjVseWuWPXa7ET4p6Dve4j3Hg+UjqYw=
 github.com/go-gorp/gorp v2.0.0+incompatible/go.mod h1:7IfkAQnO7jfT/9IQ3R9wL1dFhukN6aQxzKTHnkxzA/E=
 github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
-github.com/go-ldap/ldap v3.0.3+incompatible h1:HTeSZO8hWMS1Rgb2Ziku6b8a7qRIZZMHjsvuZyatzwk=
-github.com/go-ldap/ldap v3.0.3+incompatible/go.mod h1:qfd9rJvER9Q0/D/Sqn1DfHRoBp40uXYvFoEVrNEPqRc=
 github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-redis/redis v6.15.2+incompatible h1:9SpNVG76gr6InJGxoZ6IuuxaCOQwDAhzyXg+Bs+0Sb4=

i18n/en.json

@@ -4192,7 +4192,7 @@
   },
   {
     "id": "mfa.validate_token.authenticate.app_error",
-    "translation": "Error trying to authenticate MFA token"
+    "translation": "Invalid MFA token."
   },
   {
     "id": "migrations.worker.run_advanced_permissions_phase_2_migration.invalid_progress",

model/version.go

@@ -13,6 +13,8 @@ import (
 // It should be maintained in chronological order with most current
 // release at the front of the list.
 var versions = []string{
+	"5.18.0",
+	"5.17.0",
 	"5.16.0",
 	"5.15.0",
 	"5.14.0",

plugin/client.go

@@ -10,7 +10,6 @@ import (
 const (
 	INTERNAL_KEY_PREFIX = "mmi_"
 	BOT_USER_KEY        = INTERNAL_KEY_PREFIX + "botid"
-	CHANNEL_KEY         = INTERNAL_KEY_PREFIX + "channelid"
 )
 
 // Starts the serving of a Mattermost plugin over net/rpc. gRPC is not yet supported.

plugin/helpers_channels.go

@@ -1,111 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-package plugin
-
-import (
-	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/utils"
-	"github.com/pkg/errors"
-)
-
-func (p *HelpersImpl) EnsureChannel(channel *model.Channel) (retChannelId string, retErr error) {
-	// Must provide a channel with a name and teadId
-	if channel == nil || len(channel.Name) < 1 || len(channel.TeamId) < 1 {
-		return "", errors.New("passed a bad channel, nil or no name or no team id")
-	}
-
-	// If we fail for any reason, this could be a race between creation of channel and
-	// retrieval from another EnsureChannel. Just try the basic retrieve existing again.
-	defer func() {
-		if retChannelId == "" || retErr != nil {
-			var err error
-			var channelIdBytes []byte
-
-			err = utils.ProgressiveRetry(func() error {
-				channelIdBytes, err = p.API.KVGet(CHANNEL_KEY)
-				if err != nil {
-					return err
-				}
-				return nil
-			})
-
-			if err == nil && channelIdBytes != nil {
-				retChannelId = string(channelIdBytes)
-				retErr = nil
-			}
-		}
-	}()
-
-	// Fetch channel ID from key value store
-	channelIdBytes, kvGetErr := p.API.KVGet(CHANNEL_KEY)
-	if kvGetErr != nil {
-		// Failed to retrive the value of channel
-		return "", errors.Wrap(kvGetErr, "failed to get channel ID")
-	}
-
-	var existingChannel *model.Channel
-	var channelGetErr *model.AppError
-
-	// If channel ID exists, get existing channel by ID else get it by Name
-	if channelIdBytes != nil {
-		existingChannel, channelGetErr = p.API.GetChannel(string(channelIdBytes))
-		if channelGetErr != nil {
-			return "", errors.Wrap(channelGetErr, "failed to get channel by ID")
-		}
-	} else {
-		existingChannel, channelGetErr = p.API.GetChannelByName(channel.TeamId, channel.Name, false)
-		if channelGetErr != nil {
-			return "", errors.Wrap(channelGetErr, "failed to get channel by name")
-		}
-	}
-
-	// If channel exists, update the metadata
-	if existingChannel != nil {
-		return updateChannel(p, existingChannel, channel)
-	}
-
-	// Create a new channel
-	createdChannel, createChannelErr := p.API.CreateChannel(channel)
-	if createChannelErr != nil {
-		return "", errors.Wrap(createChannelErr, "failed to create channel")
-	}
-
-	// Set the new channel id in key value store
-	if kvSetErr := p.API.KVSet(CHANNEL_KEY, []byte(createdChannel.Id)); kvSetErr != nil {
-		p.API.LogWarn("Failed to set created channel id.", "channelid", createdChannel.Id, "err", kvSetErr)
-	}
-
-	return createdChannel.Id, nil
-}
-
-func updateChannel(p *HelpersImpl, existing *model.Channel, new *model.Channel) (string, error) {
-	// Update metadata of the channel
-	if updateErr := updateChannelMeta(existing, new); updateErr != nil {
-		return "", errors.Wrap(updateErr, "Failed to update the metadata of existing channel")
-	}
-
-	// Send the updates to API
-	updatedChannel, channelUpdateErr := p.API.UpdateChannel(existing)
-	if channelUpdateErr != nil {
-		return "", errors.Wrap(channelUpdateErr, "Failed to update the existing channel")
-	}
-
-	// Channel exists!
-	return updatedChannel.Id, nil
-}
-
-func updateChannelMeta(existing *model.Channel, new *model.Channel) error {
-	// Check if channels are of different types
-	if existing.Type != new.Type {
-		return errors.New("Channel type cannot be updated")
-	}
-
-	// Update metadata of channel
-	existing.Name = new.Name
-	existing.DisplayName = new.DisplayName
-	existing.Purpose = new.Purpose
-	existing.Header = new.Header
-
-	return nil
-}

plugin/helpers_channels_test.go

@@ -1,297 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-package plugin_test
-
-import (
-	"testing"
-
-	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/plugin"
-	"github.com/mattermost/mattermost-server/plugin/plugintest"
-	"github.com/stretchr/testify/assert"
-)
-
-func TestEnsureChannel(t *testing.T) {
-	setupAPI := func() *plugintest.API {
-		return &plugintest.API{}
-	}
-
-	testChannel := &model.Channel{
-		Id:          model.NewId(),
-		TeamId:      model.NewId(),
-		Type:        "public",
-		Name:        "test_channel",
-		DisplayName: "Test Channel",
-		Purpose:     "Testing EnsureChannel",
-		Header:      "Testing EnsureChannel",
-	}
-
-	t.Run("bad parameters", func(t *testing.T) {
-		t.Run("no channel", func(t *testing.T) {
-			p := &plugin.HelpersImpl{}
-			channelId, err := p.EnsureChannel(nil)
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("empty name", func(t *testing.T) {
-			p := &plugin.HelpersImpl{}
-			channelId, err := p.EnsureChannel(&model.Channel{
-				Name: "",
-			})
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("name without teamId", func(t *testing.T) {
-			p := &plugin.HelpersImpl{}
-			channelId, err := p.EnsureChannel(&model.Channel{
-				Name: "test_channel",
-			})
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("teamId without name", func(t *testing.T) {
-			p := &plugin.HelpersImpl{}
-			channelId, err := p.EnsureChannel(&model.Channel{
-				TeamId: model.NewId(),
-			})
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("teamId with empty name", func(t *testing.T) {
-			p := &plugin.HelpersImpl{}
-			channelId, err := p.EnsureChannel(&model.Channel{
-				TeamId: model.NewId(),
-			})
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-	})
-
-	t.Run("if channel already exists in Key Value store", func(t *testing.T) {
-		t.Run("should return an error if unable to get channel id", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return an error if unable to get channel", func(t *testing.T) {
-			expectedChannelId := model.NewId()
-
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return([]byte(expectedChannelId), nil)
-			api.On("GetChannel", expectedChannelId).Return(nil, &model.AppError{})
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return an error if unable to update channel", func(t *testing.T) {
-			expectedChannelId := model.NewId()
-
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return([]byte(expectedChannelId), nil)
-			api.On("GetChannel", expectedChannelId).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(nil, &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return the Id of existing channel if metadata is same", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return([]byte(testChannel.Id), nil)
-			api.On("GetChannel", testChannel.Id).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(testChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, testChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-		t.Run("should return error if channel type is different from existing one", func(t *testing.T) {
-			privChannel := &model.Channel{
-				Id:     model.NewId(),
-				Type:   "private",
-				TeamId: testChannel.TeamId,
-				Name:   testChannel.Name,
-			}
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return([]byte(testChannel.Id), nil)
-			api.On("GetChannel", testChannel.Id).Return(privChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return the Id of updated channel if metadata is different", func(t *testing.T) {
-			updatedChannel := &model.Channel{
-				Id:     model.NewId(),
-				TeamId: testChannel.TeamId,
-				Name:   testChannel.Name,
-			}
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return([]byte(testChannel.Id), nil)
-			api.On("GetChannel", testChannel.Id).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(updatedChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, updatedChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-	})
-
-	t.Run("if channel is not in Key Value store but already exists", func(t *testing.T) {
-		t.Run("should return an error if unable to get channel", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(nil, &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return the Id of existing channel if metadata is same", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(testChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, testChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-		t.Run("should return error if failed to update the channel", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(nil, &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should return the Id of updated channel if metadata is different", func(t *testing.T) {
-			updatedChannel := &model.Channel{
-				Id:     model.NewId(),
-				TeamId: testChannel.TeamId,
-				Name:   testChannel.Name,
-			}
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(testChannel, nil)
-			api.On("UpdateChannel", testChannel).Return(updatedChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, updatedChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-		t.Run("should return error if channel type is different from existing one", func(t *testing.T) {
-			privChannel := &model.Channel{
-				Id:     model.NewId(),
-				Type:   "private",
-				TeamId: testChannel.TeamId,
-				Name:   testChannel.Name,
-			}
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(privChannel, nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-	})
-
-	t.Run("if channel does not exist", func(t *testing.T) {
-		t.Run("should create new channel and return the Id", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(nil, nil)
-			api.On("CreateChannel", testChannel).Return(testChannel, nil)
-			api.On("KVSet", plugin.CHANNEL_KEY, []byte(testChannel.Id)).Return(nil)
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, testChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-		t.Run("should return error if unable to create new channel", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(nil, nil)
-			api.On("CreateChannel", testChannel).Return(nil, &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, "", channelId)
-			assert.NotNil(t, err)
-		})
-		t.Run("should log and return id if unable to write to Key Value store", func(t *testing.T) {
-			api := setupAPI()
-			api.On("KVGet", plugin.CHANNEL_KEY).Return(nil, nil)
-			api.On("GetChannelByName", testChannel.TeamId, testChannel.Name, false).Return(nil, nil)
-			api.On("CreateChannel", testChannel).Return(testChannel, nil)
-			api.On("KVSet", plugin.CHANNEL_KEY, []byte(testChannel.Id)).Return(&model.AppError{})
-			api.On("LogWarn", "Failed to set created channel id.", "channelid", testChannel.Id, "err", &model.AppError{})
-			defer api.AssertExpectations(t)
-
-			p := &plugin.HelpersImpl{API: api}
-
-			channelId, err := p.EnsureChannel(testChannel)
-
-			assert.Equal(t, testChannel.Id, channelId)
-			assert.Nil(t, err)
-		})
-	})
-}

scripts/license-check.sh

@@ -3,7 +3,7 @@ set -e
 IFS=$'\n'
 count=0
 for fileType in GoFiles; do
-    for file in `go list -f $'{{range .GoFiles}}{{$.Dir}}/{{.}}\n{{end}}' "$@"`; do
+    for file in `go list -mod=vendor -f $'{{range .GoFiles}}{{$.Dir}}/{{.}}\n{{end}}' "$@"`; do
         case $file in
             */utils/lru.go|*/utils/imgutils/gif.go|*/store/storetest/mocks/*|*/services/*/mocks/*|*/app/plugin/jira/plugin_*|*/plugin/plugintest/*|*/app/plugin/zoom/plugin_*|*/einterfaces/mocks/*)
             # Third-party, doesn't require a header.

store/layer_generators/main.go

@@ -246,14 +246,13 @@ func (s *{{$.Name}}{{$substoreName}}Store) {{$index}}({{$element.Params | joinPa
 	{{ else }}
 	{{$element.Results | genResultsVars}} := s.{{$substoreName}}Store.{{$index}}({{$element.Params | joinParams}})
 	{{ end }}
-	t := timemodule.Now()
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
-	elapsed := t.Sub(start)
 	if s.Root.Metrics != nil {
 		success := "false"
 		if {{$element.Results | errorToBoolean}} {
 			success = "true"
 		}
-		s.Root.Metrics.ObserveStoreMethodDuration("{{$substoreName}}Store.{{$index}}", success, float64(elapsed))
+		s.Root.Metrics.ObserveStoreMethodDuration("{{$substoreName}}Store.{{$index}}", success, elapsed)
 	}
 	return {{$element.Results | genResultsVars}}
 }

store/sqlstore/channel_store.go

@@ -1419,7 +1419,7 @@ func (s SqlChannelStore) IsUserInChannelUseCache(userId string, channelId string
 
 	ids, err := s.GetAllChannelMembersForUser(userId, true, false)
 	if err != nil {
-		mlog.Error("SqlChannelStore.IsUserInChannelUseCache: " + err.Error())
+		mlog.Error("Error getting all channel members for user", mlog.Err(err))
 		return false
 	}
 
@@ -2008,7 +2008,7 @@ func (s SqlChannelStore) GetChannelsByIds(channelIds []string) ([]*model.Channel
 	_, err := s.GetReplica().Select(&channels, query, params)
 
 	if err != nil {
-		mlog.Error(fmt.Sprint(err))
+		mlog.Error("Query error getting channels by ids", mlog.Err(err))
 		return nil, model.NewAppError("SqlChannelStore.GetChannelsByIds", "store.sql_channel.get_channels_by_ids.app_error", nil, "", http.StatusInternalServerError)
 	}
 	return channels, nil

utils/license.go

@@ -10,7 +10,6 @@ import (
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/pem"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -37,7 +36,7 @@ func ValidateLicense(signed []byte) (bool, string) {
 
 	_, err := base64.StdEncoding.Decode(decoded, signed)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Encountered error decoding license, err=%v", err.Error()))
+		mlog.Error("Encountered error decoding license", mlog.Err(err))
 		return false, ""
 	}
 
@@ -58,7 +57,7 @@ func ValidateLicense(signed []byte) (bool, string) {
 
 	public, err := x509.ParsePKIXPublicKey(block.Bytes)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Encountered error signing license, err=%v", err.Error()))
+		mlog.Error("Encountered error signing license", mlog.Err(err))
 		return false, ""
 	}
 
@@ -70,7 +69,7 @@ func ValidateLicense(signed []byte) (bool, string) {
 
 	err = rsa.VerifyPKCS1v15(rsaPublic, crypto.SHA512, d, signature)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Invalid signature, err=%v", err.Error()))
+		mlog.Error("Invalid signature", mlog.Err(err))
 		return false, ""
 	}
 
@@ -81,15 +80,15 @@ func GetAndValidateLicenseFileFromDisk(location string) (*model.License, []byte)
 	fileName := GetLicenseFileLocation(location)
 
 	if _, err := os.Stat(fileName); err != nil {
-		mlog.Debug(fmt.Sprintf("We could not find the license key in the database or on disk at %v", fileName))
+		mlog.Debug("We could not find the license key in the database or on disk at", mlog.String("filename", fileName))
 		return nil, nil
 	}
 
-	mlog.Info(fmt.Sprintf("License key has not been uploaded.  Loading license key from disk at %v", fileName))
+	mlog.Info("License key has not been uploaded.  Loading license key from disk at", mlog.String("filename", fileName))
 	licenseBytes := GetLicenseFileFromDisk(fileName)
 
 	if success, licenseStr := ValidateLicense(licenseBytes); !success {
-		mlog.Error(fmt.Sprintf("Found license key at %v but it appears to be invalid.", fileName))
+		mlog.Error("Found license key at %v but it appears to be invalid.", mlog.String("filename", fileName))
 		return nil, nil
 	} else {
 		return model.LicenseFromJson(strings.NewReader(licenseStr)), licenseBytes
@@ -99,14 +98,14 @@ func GetAndValidateLicenseFileFromDisk(location string) (*model.License, []byte)
 func GetLicenseFileFromDisk(fileName string) []byte {
 	file, err := os.Open(fileName)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Failed to open license key from disk at %v err=%v", fileName, err.Error()))
+		mlog.Error("Failed to open license key from disk at", mlog.String("filename", fileName), mlog.Err(err))
 		return nil
 	}
 	defer file.Close()
 
 	licenseBytes, err := ioutil.ReadAll(file)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Failed to read license key from disk at %v err=%v", fileName, err.Error()))
+		mlog.Error("Failed to read license key from disk at", mlog.String("filename", fileName), mlog.Err(err))
 		return nil
 	}
 

utils/test_files_compiler.go

@@ -5,11 +5,11 @@ package utils
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -38,23 +38,13 @@ func CompileGo(t *testing.T, sourceCode, outputPath string) {
 	err = ioutil.WriteFile(main, []byte(sourceCode), 0600)
 	require.NoError(t, err)
 
-	if os.Getenv("GO111MODULE") != "off" {
+	_, sourceFile, _, ok := runtime.Caller(0)
-		var mattermostServerPath string
+	require.True(t, ok)
-
+	serverPath := filepath.Dir(filepath.Dir(sourceFile))
-		// Generate a go.mod file relying on the local copy of the mattermost-server.
-		// testlib linked mattermost-server into the general temporary directory for this test.
-		mattermostServerPath, err = filepath.Abs("mattermost-server")
-		require.NoError(t, err)
-
-		goMod(t, dir, "init", "mattermost.com/test")
-		goMod(t, dir, "edit", "-require", "github.com/mattermost/mattermost-server@v0.0.0")
-		goMod(t, dir, "edit", "-replace", fmt.Sprintf("github.com/mattermost/mattermost-server@v0.0.0=%s", mattermostServerPath))
-		goMod(t, dir, "edit", "-replace", fmt.Sprintf("git.apache.org/thrift.git=%s", "github.com/apache/thrift@v0.0.0-20180902110319-2566ecd5d999"))
-	}
 
 	out := &bytes.Buffer{}
-	cmd := exec.Command("go", "build", "-o", outputPath, "main.go")
+	cmd := exec.Command("go", "build", "-mod=vendor", "-o", outputPath, main)
-	cmd.Dir = dir
+	cmd.Dir = serverPath
 	cmd.Stdout = out
 	cmd.Stderr = out
 	err = cmd.Run()