From d8c8a19d355fdd67a984fc696269521919bb58b5 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Thu, 9 Aug 2018 05:26:38 -0400 Subject: 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 | 18 +++----- model/authorize_test.go | 30 ++++-------- model/cluster_message_test.go | 6 +-- model/compliance_post_test.go | 15 ++---- model/emoji_test.go | 46 ++++++------------ model/oauth_test.go | 42 +++++------------ model/preference_test.go | 52 +++++++-------------- model/user_test.go | 66 +++++++++----------------- model/utils_test.go | 12 ++--- model/version_test.go | 105 +++++++++++++----------------------------- 10 files changed, 120 insertions(+), 272 deletions(-) (limited to 'model') diff --git a/model/access_test.go b/model/access_test.go index f0ed2da77..0f124a107 100644 --- a/model/access_test.go +++ b/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 { diff --git a/model/authorize_test.go b/model/authorize_test.go index 81e059305..0775b06c1 100644 --- a/model/authorize_test.go +++ b/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 { diff --git a/model/cluster_message_test.go b/model/cluster_message_test.go index 38603e577..e9225a5c5 100644 --- a/model/cluster_message_test.go +++ b/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 { diff --git a/model/compliance_post_test.go b/model/compliance_post_test.go index ff159ef1b..21044c128 100644 --- a/model/compliance_post_test.go +++ b/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 { diff --git a/model/emoji_test.go b/model/emoji_test.go index 50d741214..4539db873 100644 --- a/model/emoji_test.go +++ b/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()) } diff --git a/model/oauth_test.go b/model/oauth_test.go index 5c0547717..cbed8a633 100644 --- a/model/oauth_test.go +++ b/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()) } diff --git a/model/preference_test.go b/model/preference_test.go index c56d46e2c..d4035125a 100644 --- a/model/preference_test.go +++ b/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") } diff --git a/model/user_test.go b/model/user_test.go index b3aaad522..f86b52919 100644 --- a/model/user_test.go +++ b/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")) } diff --git a/model/utils_test.go b/model/utils_test.go index 9797c7090..d35146b30 100644 --- a/model/utils_test.go +++ b/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("This is a broken test")) - if "body: This is a broken test" != rerr.DetailedError { - t.Fatal() - } + require.Equal(t, "body: This is a broken test", 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{ diff --git a/model/version_test.go b/model/version_test.go index 869ed8ad0..a06db326c 100644 --- a/model/version_test.go +++ b/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])) } -- cgit v1.2.3-1-g7c22