avoid t.Fatal() in tests (#9189)

I've been burned a few times by tests that simply fatal, requiring me to
run another build to learn more about what the mismatch was. Avoid this.

This is part of a long running goal of mine to make testing "better".

model/access_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestAccessJson(t *testing.T) {
@@ -26,9 +28,7 @@ func TestAccessJson(t *testing.T) {
 func TestAccessIsValid(t *testing.T) {
 	ad := AccessData{}
 
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.ClientId = NewRandomString(28)
 	if err := ad.IsValid(); err == nil {
@@ -41,9 +41,7 @@ func TestAccessIsValid(t *testing.T) {
 	}
 
 	ad.ClientId = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.UserId = NewRandomString(28)
 	if err := ad.IsValid(); err == nil {
@@ -66,9 +64,7 @@ func TestAccessIsValid(t *testing.T) {
 	}
 
 	ad.Token = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.RefreshToken = NewRandomString(28)
 	if err := ad.IsValid(); err == nil {
@@ -76,9 +72,7 @@ func TestAccessIsValid(t *testing.T) {
 	}
 
 	ad.RefreshToken = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.RedirectUri = ""
 	if err := ad.IsValid(); err == nil {

model/authorize_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestAuthJson(t *testing.T) {
@@ -46,9 +48,7 @@ func TestAuthIsValid(t *testing.T) {
 
 	ad := AuthData{}
 
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.ClientId = NewRandomString(28)
 	if err := ad.IsValid(); err == nil {
@@ -56,9 +56,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.ClientId = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.UserId = NewRandomString(28)
 	if err := ad.IsValid(); err == nil {
@@ -66,9 +64,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.UserId = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.Code = NewRandomString(129)
 	if err := ad.IsValid(); err == nil {
@@ -81,9 +77,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.Code = NewId()
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.ExpiresIn = 0
 	if err := ad.IsValid(); err == nil {
@@ -91,9 +85,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.ExpiresIn = 1
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.CreateAt = 0
 	if err := ad.IsValid(); err == nil {
@@ -101,9 +93,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.CreateAt = 1
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.State = NewRandomString(129)
 	if err := ad.IsValid(); err == nil {
@@ -121,9 +111,7 @@ func TestAuthIsValid(t *testing.T) {
 	}
 
 	ad.Scope = NewRandomString(128)
-	if err := ad.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, ad.IsValid())
 
 	ad.RedirectUri = ""
 	if err := ad.IsValid(); err == nil {

model/cluster_message_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestClusterMessage(t *testing.T) {
@@ -17,9 +19,7 @@ func TestClusterMessage(t *testing.T) {
 	json := m.ToJson()
 	result := ClusterMessageFromJson(strings.NewReader(json))
 
-	if result.Data != "hello" {
-		t.Fatal()
-	}
+	require.Equal(t, "hello", result.Data)
 
 	badresult := ClusterMessageFromJson(strings.NewReader("junk"))
 	if badresult != nil {

model/compliance_post_test.go

@@ -5,25 +5,20 @@ package model
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestCompliancePostHeader(t *testing.T) {
-	if CompliancePostHeader()[0] != "TeamName" {
-		t.Fatal()
-	}
+	require.Equal(t, "TeamName", CompliancePostHeader()[0])
 }
 
 func TestCompliancePost(t *testing.T) {
 	o := CompliancePost{TeamName: "test", PostFileIds: "files", PostCreateAt: GetMillis()}
 	r := o.Row()
 
-	if r[0] != "test" {
-		t.Fatal()
-	}
-
-	if r[len(r)-1] != "files" {
-		t.Fatal()
-	}
+	require.Equal(t, "test", r[0])
+	require.Equal(t, "files", r[len(r)-1])
 }
 
 var cleanTests = []struct {

model/emoji_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestEmojiIsValid(t *testing.T) {
@@ -23,61 +25,39 @@ func TestEmojiIsValid(t *testing.T) {
 	}
 
 	emoji.Id = "1234"
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.Id = NewId()
 	emoji.CreateAt = 0
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.CreateAt = 1234
 	emoji.UpdateAt = 0
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.UpdateAt = 1234
 	emoji.CreatorId = strings.Repeat("1", 27)
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.CreatorId = NewId()
 	emoji.Name = strings.Repeat("1", 65)
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.Name = ""
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal(err)
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.Name = strings.Repeat("1", 64)
-	if err := emoji.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, emoji.IsValid())
 
 	emoji.Name = "name-"
-	if err := emoji.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, emoji.IsValid())
 
 	emoji.Name = "name_"
-	if err := emoji.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, emoji.IsValid())
 
 	emoji.Name = "name:"
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal(err)
-	}
+	require.NotNil(t, emoji.IsValid())
 
 	emoji.Name = "croissant"
-	if err := emoji.IsValid(); err == nil {
-		t.Fatal(err)
-	}
+	require.NotNil(t, emoji.IsValid())
 }

model/oauth_test.go

@@ -6,6 +6,8 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestOAuthAppJson(t *testing.T) {
@@ -52,52 +54,32 @@ func TestOAuthAppPreUpdate(t *testing.T) {
 func TestOAuthAppIsValid(t *testing.T) {
 	app := OAuthApp{}
 
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.Id = NewId()
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.CreateAt = 1
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.UpdateAt = 1
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.CreatorId = NewId()
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.ClientSecret = NewId()
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.Name = "TestOAuthApp"
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.CallbackUrls = []string{"https://nowhere.com"}
-	if err := app.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, app.IsValid())
 
 	app.Homepage = "https://nowhere.com"
-	if err := app.IsValid(); err != nil {
-		t.Fatal()
-	}
+	require.Nil(t, app.IsValid())
 
 	app.IconURL = "https://nowhere.com/icon_image.png"
-	if err := app.IsValid(); err != nil {
-		t.Fatal()
-	}
+	require.Nil(t, app.IsValid())
 }

model/preference_test.go

@@ -7,6 +7,8 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestPreferenceIsValid(t *testing.T) {
@@ -16,54 +18,34 @@ func TestPreferenceIsValid(t *testing.T) {
 		Name:     NewId(),
 	}
 
-	if err := preference.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, preference.IsValid())
 
 	preference.UserId = NewId()
-	if err := preference.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, preference.IsValid())
 
 	preference.Category = strings.Repeat("01234567890", 20)
-	if err := preference.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, preference.IsValid())
 
 	preference.Category = PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW
-	if err := preference.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, preference.IsValid())
 
 	preference.Name = strings.Repeat("01234567890", 20)
-	if err := preference.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, preference.IsValid())
 
 	preference.Name = NewId()
-	if err := preference.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, preference.IsValid())
 
 	preference.Value = strings.Repeat("01234567890", 201)
-	if err := preference.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, preference.IsValid())
 
 	preference.Value = "1234garbage"
-	if err := preference.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, preference.IsValid())
 
 	preference.Category = PREFERENCE_CATEGORY_THEME
-	if err := preference.IsValid(); err == nil {
-		t.Fatal()
-	}
+	require.NotNil(t, preference.IsValid())
 
 	preference.Value = `{"color": "#ff0000", "color2": "#faf"}`
-	if err := preference.IsValid(); err != nil {
-		t.Fatal(err)
-	}
+	require.Nil(t, preference.IsValid())
 }
 
 func TestPreferencePreUpdate(t *testing.T) {
@@ -79,11 +61,9 @@ func TestPreferencePreUpdate(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if props["color"] != "#ff0000" || props["color2"] != "#faf" || props["codeTheme"] != "github" {
-		t.Fatal("shouldn't have changed valid props")
-	}
+	require.Equal(t, "#ff0000", props["color"], "shouldn't have changed valid props")
+	require.Equal(t, "#faf", props["color2"], "shouldn't have changed valid props")
+	require.Equal(t, "github", props["codeTheme"], "shouldn't have changed valid props")
 
-	if props["invalid"] == "invalid" {
-		t.Fatal("should have changed invalid prop")
-	}
+	require.NotEqual(t, "invalid", props["invalid"], "should have changed invalid prop")
 }

model/user_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPasswordHash(t *testing.T) {
@@ -117,41 +118,34 @@ func TestUserIsValid(t *testing.T) {
 	}
 
 	user.Id = NewId()
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "create_at", user.Id) {
-		t.Fatal(err)
-	}
+	err := user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "create_at", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.CreateAt = GetMillis()
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "update_at", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "update_at", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.UpdateAt = GetMillis()
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "username", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.Username = NewId() + "^hello#"
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "username", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.Username = NewId()
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "email", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.Email = strings.Repeat("01234567890", 20)
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "email", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.Email = "user@example.com"
 
 	user.Nickname = strings.Repeat("a", 65)
-	if err := user.IsValid(); !HasExpectedUserIsValidError(err, "nickname", user.Id) {
-		t.Fatal(err)
-	}
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "nickname", user.Id), "expected user is valid error: %s", err.Error())
 
 	user.Nickname = strings.Repeat("a", 64)
 	if err := user.IsValid(); err != nil {
@@ -331,28 +325,10 @@ func TestCleanUsername(t *testing.T) {
 }
 
 func TestRoles(t *testing.T) {
-
-	if !IsValidUserRoles("team_user") {
-		t.Fatal()
-	}
-
-	if IsValidUserRoles("system_admin") {
-		t.Fatal()
-	}
-
-	if !IsValidUserRoles("system_user system_admin") {
-		t.Fatal()
-	}
-
-	if IsInRole("system_admin junk", "admin") {
-		t.Fatal()
-	}
-
-	if !IsInRole("system_admin junk", "system_admin") {
-		t.Fatal()
-	}
-
-	if IsInRole("admin", "system_admin") {
-		t.Fatal()
-	}
+	require.True(t, IsValidUserRoles("team_user"))
+	require.False(t, IsValidUserRoles("system_admin"))
+	require.True(t, IsValidUserRoles("system_user system_admin"))
+	require.False(t, IsInRole("system_admin junk", "admin"))
+	require.True(t, IsInRole("system_admin junk", "system_admin"))
+	require.False(t, IsInRole("admin", "system_admin"))
 }

model/utils_test.go

@@ -34,18 +34,14 @@ func TestAppError(t *testing.T) {
 	err := NewAppError("TestAppError", "message", nil, "", http.StatusInternalServerError)
 	json := err.ToJson()
 	rerr := AppErrorFromJson(strings.NewReader(json))
-	if err.Message != rerr.Message {
-		t.Fatal()
-	}
+	require.Equal(t, err.Message, rerr.Message)
 
 	t.Log(err.Error())
 }
 
 func TestAppErrorJunk(t *testing.T) {
 	rerr := AppErrorFromJson(strings.NewReader("<html><body>This is a broken test</body></html>"))
-	if "body: <html><body>This is a broken test</body></html>" != rerr.DetailedError {
-		t.Fatal()
-	}
+	require.Equal(t, "body: <html><body>This is a broken test</body></html>", rerr.DetailedError)
 }
 
 func TestCopyStringMap(t *testing.T) {
@@ -173,9 +169,7 @@ func TestValidLower(t *testing.T) {
 
 func TestEtag(t *testing.T) {
 	etag := Etag("hello", 24)
-	if len(etag) <= 0 {
-		t.Fatal()
-	}
+	require.NotEqual(t, "", etag)
 }
 
 var hashtags = map[string]string{

model/version_test.go

@@ -6,100 +6,59 @@ package model
 import (
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestSplitVersion(t *testing.T) {
 	major1, minor1, patch1 := SplitVersion("junk")
-	if major1 != 0 || minor1 != 0 || patch1 != 0 {
-		t.Fatal()
-	}
+	require.EqualValues(t, 0, major1)
+	require.EqualValues(t, 0, minor1)
+	require.EqualValues(t, 0, patch1)
 
 	major2, minor2, patch2 := SplitVersion("1.2.3")
-	if major2 != 1 || minor2 != 2 || patch2 != 3 {
-		t.Fatal()
-	}
+	require.EqualValues(t, 1, major2)
+	require.EqualValues(t, 2, minor2)
+	require.EqualValues(t, 3, patch2)
 
 	major3, minor3, patch3 := SplitVersion("1.2")
-	if major3 != 1 || minor3 != 2 || patch3 != 0 {
-		t.Fatal()
-	}
+	require.EqualValues(t, 1, major3)
+	require.EqualValues(t, 2, minor3)
+	require.EqualValues(t, 0, patch3)
 
 	major4, minor4, patch4 := SplitVersion("1")
-	if major4 != 1 || minor4 != 0 || patch4 != 0 {
-		t.Fatal()
-	}
+	require.EqualValues(t, 1, major4)
+	require.EqualValues(t, 0, minor4)
+	require.EqualValues(t, 0, patch4)
 
 	major5, minor5, patch5 := SplitVersion("1.2.3.junkgoeswhere")
-	if major5 != 1 || minor5 != 2 || patch5 != 3 {
-		t.Fatal()
-	}
+	require.EqualValues(t, 1, major5)
+	require.EqualValues(t, 2, minor5)
+	require.EqualValues(t, 3, patch5)
 }
 
 func TestGetPreviousVersion(t *testing.T) {
-	if GetPreviousVersion("1.3.0") != "1.2.0" {
-		t.Fatal()
-	}
-
-	if GetPreviousVersion("1.2.1") != "1.1.0" {
-		t.Fatal()
-	}
-
-	if GetPreviousVersion("1.1.0") != "1.0.0" {
-		t.Fatal()
-	}
-
-	if GetPreviousVersion("1.0.0") != "0.7.0" {
-		t.Fatal()
-	}
-
-	if GetPreviousVersion("0.7.1") != "0.6.0" {
-		t.Fatal()
-	}
-
-	if GetPreviousVersion("0.5.0") != "" {
-		t.Fatal()
-	}
+	require.Equal(t, "1.2.0", GetPreviousVersion("1.3.0"))
+	require.Equal(t, "1.1.0", GetPreviousVersion("1.2.1"))
+	require.Equal(t, "1.0.0", GetPreviousVersion("1.1.0"))
+	require.Equal(t, "0.7.0", GetPreviousVersion("1.0.0"))
+	require.Equal(t, "0.6.0", GetPreviousVersion("0.7.1"))
+	require.Equal(t, "", GetPreviousVersion("0.5.0"))
 }
 
 func TestIsCurrentVersion(t *testing.T) {
 	major, minor, patch := SplitVersion(CurrentVersion)
 
-	if !IsCurrentVersion(CurrentVersion) {
-		t.Fatal()
-	}
-
-	if !IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor, patch+100)) {
-		t.Fatal()
-	}
-
-	if IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor+1, patch)) {
-		t.Fatal()
-	}
-
-	if IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major+1, minor, patch)) {
-		t.Fatal()
-	}
+	require.True(t, IsCurrentVersion(CurrentVersion))
+	require.True(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor, patch+100)))
+	require.False(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor+1, patch)))
+	require.False(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major+1, minor, patch)))
 }
 
 func TestIsPreviousVersionsSupported(t *testing.T) {
-
-	if !IsPreviousVersionsSupported(versionsWithoutHotFixes[0]) {
-		t.Fatal()
-	}
-
-	if !IsPreviousVersionsSupported(versionsWithoutHotFixes[1]) {
-		t.Fatal()
-	}
-
-	if !IsPreviousVersionsSupported(versionsWithoutHotFixes[2]) {
-		t.Fatal()
-	}
-
-	if IsPreviousVersionsSupported(versionsWithoutHotFixes[4]) {
-		t.Fatal()
-	}
-
-	if IsPreviousVersionsSupported(versionsWithoutHotFixes[5]) {
-		t.Fatal()
-	}
+	require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[0]))
+	require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[1]))
+	require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[2]))
+	require.False(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[4]))
+	require.False(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[5]))
 }