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". --- store/storetest/compliance_store.go | 19 ++++++--------- store/storetest/status_store.go | 5 ++-- store/storetest/system_store.go | 13 ++++------ store/storetest/team_store.go | 47 ++++++++----------------------------- store/storetest/user_store.go | 4 +--- 5 files changed, 24 insertions(+), 64 deletions(-) (limited to 'store') diff --git a/store/storetest/compliance_store.go b/store/storetest/compliance_store.go index a772f6e44..f7f095a00 100644 --- a/store/storetest/compliance_store.go +++ b/store/storetest/compliance_store.go @@ -10,6 +10,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestComplianceStore(t *testing.T, ss store.Store) { @@ -35,9 +36,8 @@ func testComplianceStore(t *testing.T, ss store.Store) { result := <-c compliances := result.Data.(model.Compliances) - if compliances[0].Status != model.COMPLIANCE_STATUS_RUNNING && compliance2.Id != compliances[0].Id { - t.Fatal() - } + require.Equal(t, model.COMPLIANCE_STATUS_RUNNING, compliances[0].Status) + require.Equal(t, compliance2.Id, compliances[0].Id) compliance2.Status = model.COMPLIANCE_STATUS_FAILED store.Must(ss.Compliance().Update(compliance2)) @@ -46,17 +46,14 @@ func testComplianceStore(t *testing.T, ss store.Store) { result = <-c compliances = result.Data.(model.Compliances) - if compliances[0].Status != model.COMPLIANCE_STATUS_FAILED && compliance2.Id != compliances[0].Id { - t.Fatal() - } + require.Equal(t, model.COMPLIANCE_STATUS_FAILED, compliances[0].Status) + require.Equal(t, compliance2.Id, compliances[0].Id) c = ss.Compliance().GetAll(0, 1) result = <-c compliances = result.Data.(model.Compliances) - if len(compliances) != 1 { - t.Fatal("should only have returned 1") - } + require.Len(t, compliances, 1) c = ss.Compliance().GetAll(1, 1) result = <-c @@ -67,9 +64,7 @@ func testComplianceStore(t *testing.T, ss store.Store) { } rc2 := (<-ss.Compliance().Get(compliance2.Id)).Data.(*model.Compliance) - if rc2.Status != compliance2.Status { - t.Fatal() - } + require.Equal(t, compliance2.Status, rc2.Status) } func testComplianceExport(t *testing.T, ss store.Store) { diff --git a/store/storetest/status_store.go b/store/storetest/status_store.go index b26be4c19..5231bc29a 100644 --- a/store/storetest/status_store.go +++ b/store/storetest/status_store.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" @@ -103,9 +104,7 @@ func testActiveUserCount(t *testing.T, ss store.Store) { t.Fatal(result.Err) } else { count := result.Data.(int64) - if count <= 0 { - t.Fatal() - } + require.True(t, count > 0, "expected count > 0, got %d", count) } } diff --git a/store/storetest/system_store.go b/store/storetest/system_store.go index a06b72a83..6dc1efe41 100644 --- a/store/storetest/system_store.go +++ b/store/storetest/system_store.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" @@ -25,9 +26,7 @@ func testSystemStore(t *testing.T, ss store.Store) { result := <-ss.System().Get() systems := result.Data.(model.StringMap) - if systems[system.Name] != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, systems[system.Name]) system.Value = "value2" store.Must(ss.System().Update(system)) @@ -35,15 +34,11 @@ func testSystemStore(t *testing.T, ss store.Store) { result2 := <-ss.System().Get() systems2 := result2.Data.(model.StringMap) - if systems2[system.Name] != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, systems2[system.Name]) result3 := <-ss.System().GetByName(system.Name) rsystem := result3.Data.(*model.System) - if rsystem.Value != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, rsystem.Value) } func testSystemStoreSaveOrUpdate(t *testing.T, ss store.Store) { diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 40d69a2f2..2840f2965 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -590,10 +590,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { t.Fatal(r1.Err) } else { ms := r1.Data.([]*model.TeamMember) - - if len(ms) != 2 { - t.Fatal() - } + require.Len(t, ms, 2) } if r1 := <-ss.Team().GetMembers(teamId2, 0, 100); r1.Err != nil { @@ -601,14 +598,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].UserId != m3.UserId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m3.UserId, ms[0].UserId) } if r1 := <-ss.Team().GetTeamsForUser(m1.UserId); r1.Err != nil { @@ -616,14 +607,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].TeamId != m1.TeamId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m1.TeamId, ms[0].TeamId) } if r1 := <-ss.Team().RemoveMember(teamId1, m1.UserId); r1.Err != nil { @@ -635,14 +620,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].UserId != m2.UserId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m2.UserId, ms[0].UserId) } store.Must(ss.Team().SaveMember(m1, -1)) @@ -656,9 +635,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 0 { - t.Fatal() - } + require.Len(t, ms, 0) } uid := model.NewId() @@ -672,9 +649,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 2 { - t.Fatal() - } + require.Len(t, ms, 2) } if r1 := <-ss.Team().RemoveAllMembersByUser(uid); r1.Err != nil { @@ -686,9 +661,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 0 { - t.Fatal() - } + require.Len(t, ms, 0) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 10fb6a4d9..d1a373f9b 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -246,9 +246,7 @@ func testUserCount(t *testing.T, ss store.Store) { t.Fatal(result.Err) } else { count := result.Data.(int64) - if count <= 0 { - t.Fatal() - } + require.False(t, count <= 0, "expected count > 0, got %d", count) } } -- cgit v1.2.3-1-g7c22