From d10c524d34af9d32f660ccd0595ff7da593f4596 Mon Sep 17 00:00:00 2001 From: Ruslan Abelharisov Date: Mon, 21 Oct 2019 12:12:25 +0300 Subject: [PATCH 1/6] Replace t.fatal with testify package functions (#12842) --- web/context_test.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/web/context_test.go b/web/context_test.go index 3fa6ebf22d..dfd779992c 100644 --- a/web/context_test.go +++ b/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 { - t.Fatal("Hook Id is Valid. Should not have set error in context") - } + require.Nil(t, c.Err, "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 { - t.Fatal("Should have set Error in context") - } - - if c.Err.StatusCode != http.StatusBadRequest { - t.Fatal("Should have set status as 400") - } + require.Error(t, c.Err, "Should have set Error in context") + require.Equal(t, http.StatusBadRequest, c.Err.StatusCode, "Should have set status as 400") }) } From 43161731fa66f63598ef3ed44da97f18bb33c0a4 Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Mon, 21 Oct 2019 15:14:23 +0200 Subject: [PATCH 2/6] fix bash to not build webapp when it is already built (#12834) --- .circleci/config.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e32e9e70a8..3481f0519a 100644 --- a/.circleci/config.yml +++ b/.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 From 709f407d331e850593ef5f6b5d4f3dc621c5db7d Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Mon, 21 Oct 2019 15:31:29 +0200 Subject: [PATCH 3/6] update build image to go 1.13 (#12833) * update build image to go 1.13 * Define TLS Max Version for Clients since TLS1.3 does not allow cipher suites to be configured --- .circleci/config.yml | 7 ++++--- app/server_test.go | 6 ++++-- build/Dockerfile.buildenv | 4 ++-- build/Jenkinsfile.branch | 2 +- build/Jenkinsfile.pr | 14 +++++++------- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3481f0519a..d2c96cf9d8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -56,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: @@ -123,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: @@ -196,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 @@ -249,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' diff --git a/app/server_test.go b/app/server_test.go index 0991988e9b..0036e14496 100644 --- a/app/server_test.go +++ b/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, }, } diff --git a/build/Dockerfile.buildenv b/build/Dockerfile.buildenv index b68dbf0684..12f9f54b79 100644 --- a/build/Dockerfile.buildenv +++ b/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 diff --git a/build/Jenkinsfile.branch b/build/Jenkinsfile.branch index 79c61da673..2446270ae2 100644 --- a/build/Jenkinsfile.branch +++ b/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 { diff --git a/build/Jenkinsfile.pr b/build/Jenkinsfile.pr index 21af2e7d34..8a0fa14567 100644 --- a/build/Jenkinsfile.pr +++ b/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" - /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;" + 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 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 From 4b127cd8775351ab0239337fb97610bfbdc11e0b Mon Sep 17 00:00:00 2001 From: Ben Sooraj Date: Mon, 21 Oct 2019 19:49:47 +0530 Subject: [PATCH 4/6] Migrate tests from "store/storetest/reaction_store.go" to use testify (#12752) * testReactionDelete * testReactionSave * testReactionGetForPost * testReactionDeleteAllWithEmojiName * testReactionStorePermanentDeleteBatch * testReactionBulkGetForPosts * `assert` to follow convention with first parameter as `t` * using semantic assertions instead * removing unnecessary empty lines --- store/storetest/reaction_store.go | 266 ++++++++++++------------------ 1 file changed, 109 insertions(+), 157 deletions(-) diff --git a/store/storetest/reaction_store.go b/store/storetest/reaction_store.go index 06cf857d05..96394e43de 100644 --- a/store/storetest/reaction_store.go +++ b/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 { - t.Fatal(err) - } else if saved := reaction; saved.UserId != reaction1.UserId || - saved.PostId != reaction1.PostId || saved.EmojiName != reaction1.EmojiName { - t.Fatal("should've saved reaction and returned it") - } + require.Nil(t, err) + + saved := reaction + assert.Equal(t, saved.UserId, reaction1.UserId, "should've saved reaction user_id 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 { - t.Fatal(err) - } - if !postList.Posts[post.Id].HasReactions { - t.Fatal("should've set HasReactions = true on post") - } else if postList.Posts[post.Id].UpdateAt == firstUpdateAt { - t.Fatal("should've marked post as updated when HasReactions changed") - } else { + require.Nil(t, err) + + assert.True(t, postList.Posts[post.Id].HasReactions, "should've set HasReactions = true on post") + assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, firstUpdateAt, "should've marked post as updated when HasReactions changed") + + if postList.Posts[post.Id].HasReactions && postList.Posts[post.Id].UpdateAt != firstUpdateAt { secondUpdateAt = postList.Posts[post.Id].UpdateAt } - if _, err = ss.Reaction().Save(reaction1); err != nil { - t.Log(err) - t.Fatal("should've allowed saving a duplicate reaction") - } + _, err = ss.Reaction().Save(reaction1) + assert.Nil(t, err, "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 { - t.Fatal(err) - } + _, err = ss.Reaction().Save(reaction2) + require.Nil(t, err) postList, err = ss.Post().Get(reaction2.PostId, false) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) - if postList.Posts[post.Id].UpdateAt == secondUpdateAt { - t.Fatal("should've marked post as updated even if HasReactions doesn't change") - } + assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, secondUpdateAt, "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 { - t.Fatal(err) - } + _, err = ss.Reaction().Save(reaction3) + 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 { - t.Fatal(err) - } + _, err = ss.Reaction().Save(reaction4) + require.Nil(t, err) // invalid reaction reaction5 := &model.Reaction{ UserId: reaction1.UserId, PostId: reaction1.PostId, } - if _, err := ss.Reaction().Save(reaction5); err == nil { - t.Fatal("should've failed for invalid reaction") - } + _, err = ss.Reaction().Save(reaction5) + 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 { - t.Fatal(err) - } + require.Nil(t, err) + firstUpdateAt := result.Posts[post.Id].UpdateAt - if _, err = ss.Reaction().Delete(reaction); err != nil { - t.Fatal(err) - } + _, err = ss.Reaction().Delete(reaction) + 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 { - t.Fatal(err) - } - if postList.Posts[post.Id].HasReactions { - 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") - } + require.Nil(t, err) + + assert.False(t, postList.Posts[post.Id].HasReactions, "should've set HasReactions = false on post") + assert.NotEqual(t, postList.Posts[post.Id].UpdateAt, firstUpdateAt, "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 { - t.Fatal(err) - } else if len(returned) != 3 { - t.Fatal("should've returned 3 reactions") - } else { - for _, reaction := range reactions { - found := false + returned, err := ss.Reaction().GetForPost(postId, false) + require.Nil(t, err) + require.Len(t, returned, 3, "should've returned 3 reactions") - for _, returnedReaction := range returned { - if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId && - returnedReaction.EmojiName == reaction.EmojiName { - found = true - break - } - } + for _, reaction := range reactions { + found := false - if !found && reaction.PostId == postId { - t.Fatalf("should've returned reaction for post %v", reaction) - } else if found && reaction.PostId != postId { - t.Fatal("shouldn't have returned reaction for another post") + for _, returnedReaction := range returned { + if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId && + returnedReaction.EmojiName == reaction.EmojiName { + 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 { - t.Fatal(err) - } else if len(returned) != 3 { - t.Fatal("should've returned 3 reactions") - } else { - for _, reaction := range reactions { - found := false + returned, err = ss.Reaction().GetForPost(postId, true) + require.Nil(t, err) + require.Len(t, returned, 3, "should've returned 3 reactions") - for _, returnedReaction := range returned { - if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId && - returnedReaction.EmojiName == reaction.EmojiName { - found = true - break - } - } + for _, reaction := range reactions { + found := false - if !found && reaction.PostId == postId { - t.Fatalf("should've returned reaction for post %v", reaction) - } else if found && reaction.PostId != postId { - t.Fatal("shouldn't have returned reaction for another post") + for _, returnedReaction := range returned { + if returnedReaction.UserId == reaction.UserId && returnedReaction.PostId == reaction.PostId && + returnedReaction.EmojiName == reaction.EmojiName { + 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 { - t.Fatal(err) - } + err := ss.Reaction().DeleteAllWithEmojiName(emojiToDelete) + require.Nil(t, err) // check that the reactions were deleted - if returned, err := ss.Reaction().GetForPost(post.Id, false); err != nil { - t.Fatal(err) - } else if len(returned) != 1 { - t.Fatal("should've only removed reactions with emoji name") - } else { - for _, reaction := range returned { - if reaction.EmojiName == "smile" { - t.Fatal("should've removed reaction with emoji name") - } - } + returned, err := ss.Reaction().GetForPost(post.Id, false) + require.Nil(t, err) + require.Len(t, returned, 1, "should've only removed reactions with emoji name") + + for _, reaction := range returned { + assert.NotEqual(t, reaction.EmojiName, "smile", "should've removed reaction with emoji name") } - if returned, err := ss.Reaction().GetForPost(post2.Id, false); err != nil { - t.Fatal(err) - } else if len(returned) != 1 { - t.Fatal("should've only removed reactions with emoji name") - } + returned, err = ss.Reaction().GetForPost(post2.Id, false) + require.Nil(t, err) + assert.Len(t, returned, 1, "should've only removed reactions with emoji name") - if returned, err := ss.Reaction().GetForPost(post3.Id, false); err != nil { - t.Fatal(err) - } else if len(returned) != 0 { - t.Fatal("should've only removed reactions with emoji name") - } + returned, err = ss.Reaction().GetForPost(post3.Id, false) + require.Nil(t, err) + assert.Len(t, returned, 0, "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 { - t.Fatal(err) - } - if !postList.Posts[post.Id].HasReactions { - t.Fatal("post should still have reactions") - } + require.Nil(t, err) + assert.True(t, postList.Posts[post.Id].HasReactions, "post should still have reactions") postList, err = ss.Post().Get(post2.Id, false) - if err != nil { - t.Fatal(err) - } - if !postList.Posts[post2.Id].HasReactions { - t.Fatal("post should still have reactions") - } + require.Nil(t, err) + assert.True(t, postList.Posts[post2.Id].HasReactions, "post should still have reactions") postList, err = ss.Post().Get(post3.Id, false) - if err != nil { - t.Fatal(err) - } + require.Nil(t, 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 { - t.Fatal(err) - } else if len(returned) != 4 { - t.Fatal("expected 4 reactions") - } + returned, err := ss.Reaction().GetForPost(post.Id, false) + require.Nil(t, err) + require.Len(t, returned, 4, "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 { - t.Fatal(err) - } else if len(returned) != 1 { - t.Fatalf("expected 1 reaction. Got: %v", len(returned)) - } + returned, err = ss.Reaction().GetForPost(post.Id, false) + require.Nil(t, err) + require.Len(t, returned, 1, "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 { - t.Fatal(err) - } else if len(returned) != 5 { - t.Fatal("should've returned 5 reactions") - } else { - post4IdFound := false - for _, reaction := range returned { - if reaction.PostId == post4Id { - post4IdFound = true - break - } - } + returned, err := ss.Reaction().BulkGetForPosts(postIds) + require.Nil(t, err) + require.Len(t, returned, 5, "should've returned 5 reactions") - if post4IdFound { - t.Fatal("Wrong reaction returned") + post4IdFound := false + for _, reaction := range returned { + if reaction.PostId == post4Id { + post4IdFound = true + break } } + require.False(t, post4IdFound, "Wrong reaction returned") + } From 4c7220d69a724af10c0e6bc4c7296bef9dd02f9d Mon Sep 17 00:00:00 2001 From: Will Andrews Date: Mon, 21 Oct 2019 15:27:42 +0100 Subject: [PATCH 5/6] changed from using t.Fatal to Require (#12774) --- store/storetest/plugin_store.go | 109 +++++++++++++------------------- 1 file changed, 45 insertions(+), 64 deletions(-) diff --git a/store/storetest/plugin_store.go b/store/storetest/plugin_store.go index 2b14446273..e98af1e460 100644 --- a/store/storetest/plugin_store.go +++ b/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 { - t.Fatal(err) - } + _, err := ss.Plugin().SaveOrUpdate(kv) + 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 { - t.Fatal(err) - } else { - assert.Equal(t, kv.PluginId, received.PluginId) - assert.Equal(t, kv.Key, received.Key) - assert.Equal(t, kv.Value, received.Value) - assert.Equal(t, kv.ExpireAt, received.ExpireAt) - } + received, err := ss.Plugin().Get(kv.PluginId, kv.Key) + require.Nil(t, err) + assert.Equal(t, kv.PluginId, received.PluginId) + assert.Equal(t, kv.Key, received.Key) + assert.Equal(t, kv.Value, received.Value) + 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 { - t.Fatal(err) - } + _, err = ss.Plugin().SaveOrUpdate(kv) + require.Nil(t, err) - if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil { - t.Fatal(err) - } else { - assert.Equal(t, kv.PluginId, received.PluginId) - assert.Equal(t, kv.Key, received.Key) - assert.Equal(t, kv.Value, received.Value) - } + received, err = ss.Plugin().Get(kv.PluginId, kv.Key) + require.Nil(t, err) + assert.Equal(t, kv.PluginId, received.PluginId) + assert.Equal(t, kv.Key, received.Key) + 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 { - t.Fatal(err) - } + _, err := ss.Plugin().SaveOrUpdate(kv) + 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 { - t.Fatal(err) - } else { - assert.Equal(t, kv.PluginId, received.PluginId) - assert.Equal(t, kv.Key, received.Key) - assert.Equal(t, kv.Value, received.Value) - assert.Equal(t, kv.ExpireAt, received.ExpireAt) - } + received, err := ss.Plugin().Get(kv.PluginId, kv.Key) + require.Nil(t, err) + assert.Equal(t, kv.PluginId, received.PluginId) + assert.Equal(t, kv.Key, received.Key) + assert.Equal(t, kv.Value, received.Value) + 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 { - t.Fatal(err) - } + _, err = ss.Plugin().SaveOrUpdate(kv) + require.Nil(t, err) defer func() { _ = ss.Plugin().Delete(kv.PluginId, kv.Key) }() - if _, err := ss.Plugin().Get(kv.PluginId, kv.Key); err == nil { - t.Fatal("result.Err should not be nil") - } + _, err = ss.Plugin().Get(kv.PluginId, kv.Key) + 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 { - t.Fatal(err) - } + err = ss.Plugin().Delete(kv.PluginId, kv.Key) + 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 { - t.Fatal(err) - } + err = ss.Plugin().DeleteAllForPlugin(pluginId) + require.Nil(t, err) - if _, err := ss.Plugin().Get(pluginId, kv.Key); err == nil { - t.Fatal("result.Err should not be nil") - } + _, err = ss.Plugin().Get(kv.PluginId, kv.Key) + require.NotNil(t, err) - if _, err := ss.Plugin().Get(pluginId, kv2.Key); err == nil { - t.Fatal("result.Err should not be nil") - } + _, err = ss.Plugin().Get(kv.PluginId, kv2.Key) + 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 { - t.Fatal(err) - } + err = ss.Plugin().DeleteAllExpired() + require.Nil(t, err) - if _, err := ss.Plugin().Get(pluginId, kv.Key); err == nil { - t.Fatal("result.Err should not be nil") - } + _, err = ss.Plugin().Get(kv.PluginId, kv.Key) + require.NotNil(t, err) - if received, err := ss.Plugin().Get(kv2.PluginId, kv2.Key); err != nil { - t.Fatal(err) - } else { - assert.Equal(t, kv2.PluginId, received.PluginId) - assert.Equal(t, kv2.Key, received.Key) - assert.Equal(t, kv2.Value, received.Value) - assert.Equal(t, kv2.ExpireAt, received.ExpireAt) - } + received, err := ss.Plugin().Get(kv2.PluginId, kv2.Key) + require.Nil(t, err) + assert.Equal(t, kv2.PluginId, received.PluginId) + assert.Equal(t, kv2.Key, received.Key) + assert.Equal(t, kv2.Value, received.Value) + assert.Equal(t, kv2.ExpireAt, received.ExpireAt) } From e83f83fa856fe88bec24d56dd7fa352e5def1073 Mon Sep 17 00:00:00 2001 From: Sascha Andres Date: Mon, 21 Oct 2019 17:00:21 +0200 Subject: [PATCH 6/6] MM-1946 Migrate tests from "web/web_test.go" to use testify #12794 (#12802) * MM-1946 Migrate tests from "web/web_test.go" to use testify #12794 Also changed call in commented out test for client. * refactor: re-introduce t.Run * Update web/web_test.go Co-Authored-By: Harrison Healey --- web/web_test.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/web/web_test.go b/web/web_test.go index f9c663ec81..6995ab1914 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -165,7 +165,7 @@ func TestPublicFilesRequest(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - + ` // Compile and write the plugin backend := filepath.Join(pluginDir, pluginID, "backend.exe") @@ -220,11 +220,8 @@ func TestStatic(t *testing.T) { resp, err := http.Get(URL + "/static/root.html") - if err != nil { - t.Fatalf("got error while trying to get static files %v", err) - } else if resp.StatusCode != http.StatusOK { - t.Fatalf("couldn't get static files %v", resp.StatusCode) - } + assert.NoErrorf(t, err, "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) } */ @@ -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 { - t.Fatalf("%s User Agent Test failed!", browser.Name) - } + result := CheckClientCompatability(browser.UserAgent) + require.Equalf(t, result, browser.Result, "user agent test failed for %s", browser.Name) }) } }