Merge branch 'master' into mark-as-unread

.circleci/config.yml

@@ -25,7 +25,9 @@ jobs:
           git checkout $CIRCLE_BRANCH || git checkout master
           export WEBAPP_GIT_COMMIT=$(git rev-parse HEAD)
           echo "$WEBAPP_GIT_COMMIT"
-          curl -f -o ./dist.tar.gz https://releases.mattermost.com/mattermost-webapp/commit/${WEBAPP_GIT_COMMIT}/mattermost-webapp.tar.gz && mkdir ./dist && tar -xvf ./dist.tar.gz -C ./dist --strip-components=1 || echo "curl failed" && export CURL_FAILED=1
+          export CURL_FAILED=0
+          curl -f -o ./dist.tar.gz https://releases.mattermost.com/mattermost-webapp/commit/${WEBAPP_GIT_COMMIT}/mattermost-webapp.tar.gz && mkdir ./dist && tar -xvf ./dist.tar.gz -C ./dist --strip-components=1 || export CURL_FAILED=1
+
           if [ $CURL_FAILED -eq 1 ]
           then
             npm ci && cd node_modules/mattermost-redux && npm install && npm run build && cd ../.. && make build
@@ -54,7 +56,7 @@ jobs:
 
   build:
     docker:
-      - image: mattermost/mattermost-build-server:sep-17-2019
+      - image: mattermost/mattermost-build-server:oct-18-2019
     working_directory: /go/src/github.com/mattermost
     steps:
       - attach_workspace:
@@ -121,7 +123,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:sep-17-2019 \
+              mattermost/mattermost-build-server:oct-18-2019 \
               bash -c 'ulimit -n 8096; make test-server BUILD_NUMBER="$CIRCLE_BRANCH-$CIRCLE_PREVIOUS_BUILD_NUM" TESTFLAGS= TESTFLAGSEE='
           no_output_timeout: 1h
       - run:
@@ -194,7 +196,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:oct-18-2019 \
               bash -c 'ulimit -n 8096; make ARGS="version" run-cli && make MM_SQLSETTINGS_DATASOURCE="postgres://mmuser:mostest@postgres:5432/latest?sslmode=disable&connect_timeout=10" ARGS="version" run-cli'
             echo "Generating dump"
             docker-compose --no-ansi exec -T postgres pg_dump --schema-only -d migrated -U mmuser > migrated.sql
@@ -247,6 +249,7 @@ jobs:
             echo "Generating diff"
             diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (echo "Schema mismatch" && cat diff.txt && exit 1)
           no_output_timeout: 1h
+
   upload-s3-sha:
     docker:
       - image: 'circleci/python:2.7'

app/server_test.go

@@ -164,14 +164,15 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) {
 		TLSClientConfig: &tls.Config{
 			InsecureSkipVerify: true,
 			CipherSuites: []uint16{
-				tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+				tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
 			},
+			MaxVersion: tls.VersionTLS12,
 		},
 	}
 
 	client := &http.Client{Transport: tr}
 	err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound)
-
+	require.Error(t, err, "Expected error due to Cipher mismatch")
 	if !strings.Contains(err.Error(), "remote error: tls: handshake failure") {
 		t.Errorf("Expected protocol version error, got %s", err)
 	}
@@ -183,6 +184,7 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) {
 				tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
 				tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
 			},
+			MaxVersion: tls.VersionTLS12,
 		},
 	}
 

build/Dockerfile.buildenv

@@ -1,3 +1,3 @@
-FROM golang:1.12
+FROM golang:1.13
 
-RUN apt-get update && apt-get install -y make git apt-transport-https ca-certificates curl software-properties-common build-essential zip xmlsec1
+RUN apt-get update && apt-get install -y make git apt-transport-https ca-certificates curl software-properties-common build-essential zip xmlsec1 jq

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:sep-17-2019'
+def mmBuilderServer = 'mattermost/mattermost-build-server:oct-18-2019'
 def mmBuilderWebapp = 'mattermost/mattermost-build-webapp:oct-2-2018'
 
 pipeline {

build/Jenkinsfile.pr

@@ -96,7 +96,7 @@ pipeline {
 			}
 
 			steps {
-				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:sep-17-2019') {
+				withDockerContainer(args: '-u root --privileged -v ${WORKSPACE}/src:/go/src/', image: 'mattermost/mattermost-build-server:oct-18-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:sep-17-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:oct-18-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -328,7 +328,7 @@ pipeline {
 					}
 				}
 
-				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:sep-17-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:oct-18-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server
@@ -353,9 +353,9 @@ pipeline {
 				dir('src/github.com/mattermost/mattermost-server') {
 					ansiColor('xterm') {
 						sh """
-                                                echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest"
+						echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest"
-                                                /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+						/usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
-                                                /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
+						/usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;"
 
 						echo "Generating dump"
 						/usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml  exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > migrated.sql
@@ -375,7 +375,7 @@ pipeline {
 					}
 				}
 
-				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:sep-17-2019') {
+				withDockerContainer(args: "-u root --privileged --net ${COMPOSE_PROJECT_NAME}_mm-test -v ${WORKSPACE}/src:/go/src/", image: 'mattermost/mattermost-build-server:oct-18-2019') {
 					ansiColor('xterm') {
 						sh """
 							cd /go/src/github.com/mattermost/mattermost-server

store/storetest/plugin_store.go

@@ -29,36 +29,30 @@ func testPluginSaveGet(t *testing.T, ss store.Store) {
 		ExpireAt: 0,
 	}
 
-	if _, err := ss.Plugin().SaveOrUpdate(kv); err != nil {
+	_, err := ss.Plugin().SaveOrUpdate(kv)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	defer func() {
 		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
-	if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil {
+	received, err := ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	assert.Equal(t, kv.PluginId, received.PluginId)
-		assert.Equal(t, kv.PluginId, received.PluginId)
+	assert.Equal(t, kv.Key, received.Key)
-		assert.Equal(t, kv.Key, received.Key)
+	assert.Equal(t, kv.Value, received.Value)
-		assert.Equal(t, kv.Value, received.Value)
+	assert.Equal(t, kv.ExpireAt, received.ExpireAt)
-		assert.Equal(t, kv.ExpireAt, received.ExpireAt)
-	}
 
 	// Try inserting when already exists
 	kv.Value = []byte(model.NewId())
-	if _, err := ss.Plugin().SaveOrUpdate(kv); err != nil {
+	_, err = ss.Plugin().SaveOrUpdate(kv)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil {
+	received, err = ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	assert.Equal(t, kv.PluginId, received.PluginId)
-		assert.Equal(t, kv.PluginId, received.PluginId)
+	assert.Equal(t, kv.Key, received.Key)
-		assert.Equal(t, kv.Key, received.Key)
+	assert.Equal(t, kv.Value, received.Value)
-		assert.Equal(t, kv.Value, received.Value)
-	}
 }
 
 func testPluginSaveGetExpiry(t *testing.T, ss store.Store) {
@@ -69,22 +63,19 @@ func testPluginSaveGetExpiry(t *testing.T, ss store.Store) {
 		ExpireAt: model.GetMillis() + 30000,
 	}
 
-	if _, err := ss.Plugin().SaveOrUpdate(kv); err != nil {
+	_, err := ss.Plugin().SaveOrUpdate(kv)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	defer func() {
 		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
-	if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil {
+	received, err := ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	assert.Equal(t, kv.PluginId, received.PluginId)
-		assert.Equal(t, kv.PluginId, received.PluginId)
+	assert.Equal(t, kv.Key, received.Key)
-		assert.Equal(t, kv.Key, received.Key)
+	assert.Equal(t, kv.Value, received.Value)
-		assert.Equal(t, kv.Value, received.Value)
+	assert.Equal(t, kv.ExpireAt, received.ExpireAt)
-		assert.Equal(t, kv.ExpireAt, received.ExpireAt)
-	}
 
 	kv = &model.PluginKeyValue{
 		PluginId: model.NewId(),
@@ -93,17 +84,15 @@ func testPluginSaveGetExpiry(t *testing.T, ss store.Store) {
 		ExpireAt: model.GetMillis() - 5000,
 	}
 
-	if _, err := ss.Plugin().SaveOrUpdate(kv); err != nil {
+	_, err = ss.Plugin().SaveOrUpdate(kv)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	defer func() {
 		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
-	if _, err := ss.Plugin().Get(kv.PluginId, kv.Key); err == nil {
+	_, err = ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal("result.Err should not be nil")
+	require.NotNil(t, err)
-	}
 }
 
 func testPluginDelete(t *testing.T, ss store.Store) {
@@ -114,9 +103,8 @@ func testPluginDelete(t *testing.T, ss store.Store) {
 	})
 	require.Nil(t, err)
 
-	if err := ss.Plugin().Delete(kv.PluginId, kv.Key); err != nil {
+	err = ss.Plugin().Delete(kv.PluginId, kv.Key)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func testPluginDeleteAll(t *testing.T, ss store.Store) {
@@ -136,17 +124,14 @@ func testPluginDeleteAll(t *testing.T, ss store.Store) {
 	})
 	require.Nil(t, err)
 
-	if err := ss.Plugin().DeleteAllForPlugin(pluginId); err != nil {
+	err = ss.Plugin().DeleteAllForPlugin(pluginId)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if _, err := ss.Plugin().Get(pluginId, kv.Key); err == nil {
+	_, err = ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal("result.Err should not be nil")
+	require.NotNil(t, err)
-	}
 
-	if _, err := ss.Plugin().Get(pluginId, kv2.Key); err == nil {
+	_, err = ss.Plugin().Get(kv.PluginId, kv2.Key)
-		t.Fatal("result.Err should not be nil")
+	require.NotNil(t, err)
-	}
 }
 
 func testPluginDeleteExpired(t *testing.T, ss store.Store) {
@@ -168,20 +153,16 @@ func testPluginDeleteExpired(t *testing.T, ss store.Store) {
 	})
 	require.Nil(t, err)
 
-	if err := ss.Plugin().DeleteAllExpired(); err != nil {
+	err = ss.Plugin().DeleteAllExpired()
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
-	if _, err := ss.Plugin().Get(pluginId, kv.Key); err == nil {
+	_, err = ss.Plugin().Get(kv.PluginId, kv.Key)
-		t.Fatal("result.Err should not be nil")
+	require.NotNil(t, err)
-	}
 
-	if received, err := ss.Plugin().Get(kv2.PluginId, kv2.Key); err != nil {
+	received, err := ss.Plugin().Get(kv2.PluginId, kv2.Key)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+	assert.Equal(t, kv2.PluginId, received.PluginId)
-		assert.Equal(t, kv2.PluginId, received.PluginId)
+	assert.Equal(t, kv2.Key, received.Key)
-		assert.Equal(t, kv2.Key, received.Key)
+	assert.Equal(t, kv2.Value, received.Value)
-		assert.Equal(t, kv2.Value, received.Value)
+	assert.Equal(t, kv2.ExpireAt, received.ExpireAt)
-		assert.Equal(t, kv2.ExpireAt, received.ExpireAt)
-	}
 }

store/storetest/reaction_store.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -34,30 +35,26 @@ func testReactionSave(t *testing.T, ss store.Store) {
 		EmojiName: model.NewId(),
 	}
 	reaction, err := ss.Reaction().Save(reaction1)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+
-	} else if saved := reaction; saved.UserId != reaction1.UserId ||
+	saved := reaction
-		saved.PostId != reaction1.PostId || saved.EmojiName != reaction1.EmojiName {
+	assert.Equal(t, saved.UserId, reaction1.UserId, "should've saved reaction user_id and returned it")
-		t.Fatal("should've saved reaction and returned it")
+	assert.Equal(t, saved.PostId, reaction1.PostId, "should've saved reaction post_id and returned it")
-	}
+	assert.Equal(t, saved.EmojiName, reaction1.EmojiName, "should've saved reaction emoji_name and returned it")
 
 	var secondUpdateAt int64
 	postList, err := ss.Post().Get(reaction1.PostId, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+
-	}
+	assert.True(t, postList.Posts[post.Id].HasReactions, "should've set HasReactions = true on post")
-	if !postList.Posts[post.Id].HasReactions {
+	assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, firstUpdateAt, "should've marked post as updated when HasReactions changed")
-		t.Fatal("should've set HasReactions = true on post")
+
-	} else if postList.Posts[post.Id].UpdateAt == firstUpdateAt {
+	if postList.Posts[post.Id].HasReactions && postList.Posts[post.Id].UpdateAt != firstUpdateAt {
-		t.Fatal("should've marked post as updated when HasReactions changed")
-	} else {
 		secondUpdateAt = postList.Posts[post.Id].UpdateAt
 	}
 
-	if _, err = ss.Reaction().Save(reaction1); err != nil {
+	_, err = ss.Reaction().Save(reaction1)
-		t.Log(err)
+	assert.Nil(t, err, "should've allowed saving a duplicate reaction")
-		t.Fatal("should've allowed saving a duplicate reaction")
-	}
 
 	// different user
 	reaction2 := &model.Reaction{
@@ -65,18 +62,13 @@ func testReactionSave(t *testing.T, ss store.Store) {
 		PostId:    reaction1.PostId,
 		EmojiName: reaction1.EmojiName,
 	}
-	if _, err = ss.Reaction().Save(reaction2); err != nil {
+	_, err = ss.Reaction().Save(reaction2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	postList, err = ss.Post().Get(reaction2.PostId, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
 
-	if postList.Posts[post.Id].UpdateAt == secondUpdateAt {
+	assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, secondUpdateAt, "should've marked post as updated even if HasReactions doesn't change")
-		t.Fatal("should've marked post as updated even if HasReactions doesn't change")
-	}
 
 	// different post
 	reaction3 := &model.Reaction{
@@ -84,9 +76,8 @@ func testReactionSave(t *testing.T, ss store.Store) {
 		PostId:    model.NewId(),
 		EmojiName: reaction1.EmojiName,
 	}
-	if _, err := ss.Reaction().Save(reaction3); err != nil {
+	_, err = ss.Reaction().Save(reaction3)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	// different emoji
 	reaction4 := &model.Reaction{
@@ -94,18 +85,17 @@ func testReactionSave(t *testing.T, ss store.Store) {
 		PostId:    reaction1.PostId,
 		EmojiName: model.NewId(),
 	}
-	if _, err := ss.Reaction().Save(reaction4); err != nil {
+	_, err = ss.Reaction().Save(reaction4)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	// invalid reaction
 	reaction5 := &model.Reaction{
 		UserId: reaction1.UserId,
 		PostId: reaction1.PostId,
 	}
-	if _, err := ss.Reaction().Save(reaction5); err == nil {
+	_, err = ss.Reaction().Save(reaction5)
-		t.Fatal("should've failed for invalid reaction")
+	require.NotNil(t, err, "should've failed for invalid reaction")
-	}
+
 }
 
 func testReactionDelete(t *testing.T, ss store.Store) {
@@ -123,30 +113,25 @@ func testReactionDelete(t *testing.T, ss store.Store) {
 
 	_, err = ss.Reaction().Save(reaction)
 	require.Nil(t, err)
+
 	result, err := ss.Post().Get(reaction.PostId, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+
-	}
 	firstUpdateAt := result.Posts[post.Id].UpdateAt
 
-	if _, err = ss.Reaction().Delete(reaction); err != nil {
+	_, err = ss.Reaction().Delete(reaction)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
+
+	reactions, rErr := ss.Reaction().GetForPost(post.Id, false)
+	require.Nil(t, rErr)
+
+	assert.Len(t, reactions, 0, "should've deleted reaction")
 
-	if reactions, rErr := ss.Reaction().GetForPost(post.Id, false); rErr != nil {
-		t.Fatal(rErr)
-	} else if len(reactions) != 0 {
-		t.Fatal("should've deleted reaction")
-	}
 	postList, err := ss.Post().Get(post.Id, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+
-	}
+	assert.False(t, postList.Posts[post.Id].HasReactions, "should've set HasReactions = false on post")
-	if postList.Posts[post.Id].HasReactions {
+	assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, firstUpdateAt, "should mark post as updated after deleting reactions")
-		t.Fatal("should've set HasReactions = false on post")
-	} else if postList.Posts[post.Id].UpdateAt == firstUpdateAt {
-		t.Fatal("should mark post as updated after deleting reactions")
-	}
 }
 
 func testReactionGetForPost(t *testing.T, ss store.Store) {
@@ -182,53 +167,49 @@ func testReactionGetForPost(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 	}
 
-	if returned, err := ss.Reaction().GetForPost(postId, false); err != nil {
+	returned, err := ss.Reaction().GetForPost(postId, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 3 {
+	require.Len(t, returned, 3, "should've returned 3 reactions")
-		t.Fatal("should've returned 3 reactions")
-	} else {
-		for _, reaction := range reactions {
-			found := false
 
-			for _, returnedReaction := range returned {
+	for _, reaction := range reactions {
-				if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId &&
+		found := false
-					returnedReaction.EmojiName == reaction.EmojiName {
-					found = true
-					break
-				}
-			}
 
-			if !found && reaction.PostId == postId {
+		for _, returnedReaction := range returned {
-				t.Fatalf("should've returned reaction for post %v", reaction)
+			if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId &&
-			} else if found && reaction.PostId != postId {
+				returnedReaction.EmojiName == reaction.EmojiName {
-				t.Fatal("shouldn't have returned reaction for another post")
+				found = true
+				break
 			}
 		}
+
+		if !found {
+			assert.NotEqual(t, reaction.PostId, postId, "should've returned reaction for post %v", reaction)
+		} else if found {
+			assert.Equal(t, reaction.PostId, postId, "shouldn't have returned reaction for another post")
+		}
 	}
 
 	// Should return cached item
-	if returned, err := ss.Reaction().GetForPost(postId, true); err != nil {
+	returned, err = ss.Reaction().GetForPost(postId, true)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 3 {
+	require.Len(t, returned, 3, "should've returned 3 reactions")
-		t.Fatal("should've returned 3 reactions")
-	} else {
-		for _, reaction := range reactions {
-			found := false
 
-			for _, returnedReaction := range returned {
+	for _, reaction := range reactions {
-				if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId &&
+		found := false
-					returnedReaction.EmojiName == reaction.EmojiName {
-					found = true
-					break
-				}
-			}
 
-			if !found && reaction.PostId == postId {
+		for _, returnedReaction := range returned {
-				t.Fatalf("should've returned reaction for post %v", reaction)
+			if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId &&
-			} else if found && reaction.PostId != postId {
+				returnedReaction.EmojiName == reaction.EmojiName {
-				t.Fatal("shouldn't have returned reaction for another post")
+				found = true
+				break
 			}
 		}
+
+		if !found {
+			assert.NotEqual(t, reaction.PostId, postId, "should've returned reaction for post %v", reaction)
+		} else if found {
+			assert.Equal(t, reaction.PostId, postId, "shouldn't have returned reaction for another post")
+		}
 	}
 }
 
@@ -286,60 +267,39 @@ func testReactionDeleteAllWithEmojiName(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 	}
 
-	if err := ss.Reaction().DeleteAllWithEmojiName(emojiToDelete); err != nil {
+	err := ss.Reaction().DeleteAllWithEmojiName(emojiToDelete)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 
 	// check that the reactions were deleted
-	if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil {
+	returned, err := ss.Reaction().GetForPost(post.Id, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 1 {
+	require.Len(t, returned, 1, "should've only removed reactions with emoji name")
-		t.Fatal("should've only removed reactions with emoji name")
+
-	} else {
+	for _, reaction := range returned {
-		for _, reaction := range returned {
+		assert.NotEqual(t, reaction.EmojiName, "smile", "should've removed reaction with emoji name")
-			if reaction.EmojiName == "smile" {
-				t.Fatal("should've removed reaction with emoji name")
-			}
-		}
 	}
 
-	if returned, err := ss.Reaction().GetForPost(post2.Id, false); err != nil {
+	returned, err = ss.Reaction().GetForPost(post2.Id, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 1 {
+	assert.Len(t, returned, 1, "should've only removed reactions with emoji name")
-		t.Fatal("should've only removed reactions with emoji name")
-	}
 
-	if returned, err := ss.Reaction().GetForPost(post3.Id, false); err != nil {
+	returned, err = ss.Reaction().GetForPost(post3.Id, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 0 {
+	assert.Len(t, returned, 0, "should've only removed reactions with emoji name")
-		t.Fatal("should've only removed reactions with emoji name")
-	}
 
 	// check that the posts are updated
 	postList, err := ss.Post().Get(post.Id, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+	assert.True(t, postList.Posts[post.Id].HasReactions, "post should still have reactions")
-	}
-	if !postList.Posts[post.Id].HasReactions {
-		t.Fatal("post should still have reactions")
-	}
 
 	postList, err = ss.Post().Get(post2.Id, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+	assert.True(t, postList.Posts[post2.Id].HasReactions, "post should still have reactions")
-	}
-	if !postList.Posts[post2.Id].HasReactions {
-		t.Fatal("post should still have reactions")
-	}
 
 	postList, err = ss.Post().Get(post3.Id, false)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+	assert.False(t, postList.Posts[post3.Id].HasReactions, "post shouldn't have reactions any more")
-	}
 
-	if postList.Posts[post3.Id].HasReactions {
-		t.Fatal("post shouldn't have reactions any more")
-	}
 }
 
 func testReactionStorePermanentDeleteBatch(t *testing.T, ss store.Store) {
@@ -384,24 +344,20 @@ func testReactionStorePermanentDeleteBatch(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 	}
 
-	if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil {
+	returned, err := ss.Reaction().GetForPost(post.Id, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 4 {
+	require.Len(t, returned, 4, "expected 4 reactions")
-		t.Fatal("expected 4 reactions")
-	}
 
-	_, err := ss.Reaction().PermanentDeleteBatch(1800, 1000)
+	_, err = ss.Reaction().PermanentDeleteBatch(1800, 1000)
 	require.Nil(t, err)
 
 	// This is to force a clear of the cache.
 	_, err = ss.Reaction().Delete(lastReaction)
 	require.Nil(t, err)
 
-	if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil {
+	returned, err = ss.Reaction().GetForPost(post.Id, false)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 1 {
+	require.Len(t, returned, 1, "expected 1 reaction. Got: %v", len(returned))
-		t.Fatalf("expected 1 reaction. Got: %v", len(returned))
-	}
 }
 
 func testReactionBulkGetForPosts(t *testing.T, ss store.Store) {
@@ -451,22 +407,18 @@ func testReactionBulkGetForPosts(t *testing.T, ss store.Store) {
 	}
 
 	postIds := []string{postId, post2Id, post3Id}
-	if returned, err := ss.Reaction().BulkGetForPosts(postIds); err != nil {
+	returned, err := ss.Reaction().BulkGetForPosts(postIds)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(returned) != 5 {
+	require.Len(t, returned, 5, "should've returned 5 reactions")
-		t.Fatal("should've returned 5 reactions")
-	} else {
-		post4IdFound := false
-		for _, reaction := range returned {
-			if reaction.PostId == post4Id {
-				post4IdFound = true
-				break
-			}
-		}
 
-		if post4IdFound {
+	post4IdFound := false
-			t.Fatal("Wrong reaction returned")
+	for _, reaction := range returned {
+		if reaction.PostId == post4Id {
+			post4IdFound = true
+			break
 		}
 	}
 
+	require.False(t, post4IdFound, "Wrong reaction returned")
+
 }

web/context_test.go

@@ -3,6 +3,8 @@ package web
 import (
 	"net/http"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestRequireHookId(t *testing.T) {
@@ -11,21 +13,14 @@ func TestRequireHookId(t *testing.T) {
 		c.Params = &Params{HookId: "abcdefghijklmnopqrstuvwxyz"}
 		c.RequireHookId()
 
-		if c.Err != nil {
+		require.Nil(t, c.Err, "Hook Id is Valid. Should not have set error in context")
-			t.Fatal("Hook Id is Valid. Should not have set error in context")
-		}
 	})
 
 	t.Run("WhenHookIdIsInvalid", func(t *testing.T) {
 		c.Params = &Params{HookId: "abc"}
 		c.RequireHookId()
 
-		if c.Err == nil {
+		require.Error(t, c.Err, "Should have set Error in context")
-			t.Fatal("Should have set Error in context")
+		require.Equal(t, http.StatusBadRequest, c.Err.StatusCode, "Should have set status as 400")
-		}
-
-		if c.Err.StatusCode != http.StatusBadRequest {
-			t.Fatal("Should have set status as 400")
-		}
 	})
 }

web/web_test.go

@@ -220,11 +220,8 @@ func TestStatic(t *testing.T) {
 
 	resp, err := http.Get(URL + "/static/root.html")
 
-	if err != nil {
+	assert.NoErrorf(t, err, "got error while trying to get static files %v", err)
-		t.Fatalf("got error while trying to get static files %v", err)
+	assert.Equalf(t, resp.StatusCode, http.StatusOK, "couldn't get static files %v", resp.StatusCode)
-	} else if resp.StatusCode != http.StatusOK {
-		t.Fatalf("couldn't get static files %v", resp.StatusCode)
-	}
 }
 */
 
@@ -254,9 +251,8 @@ func TestCheckClientCompatability(t *testing.T) {
 	}
 	for _, browser := range uaTestParameters {
 		t.Run(browser.Name, func(t *testing.T) {
-			if result := CheckClientCompatability(browser.UserAgent); result != browser.Result {
+			result := CheckClientCompatability(browser.UserAgent)
-				t.Fatalf("%s User Agent Test failed!", browser.Name)
+			require.Equalf(t, result, browser.Result, "user agent test failed for %s", browser.Name)
-			}
 		})
 	}
 }