From 84f45634a888f943dd82aa3408df6213ecd24b62 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 13 Feb 2020 17:53:23 +0100 Subject: [PATCH] Remove remaining t.Fatal from the codebase (#13876) * Remove remaining t.Fatal from the codebase * Fix job_store test * Address review comments * Remove comments --- api4/apitestlib.go | 2 +- api4/channel_test.go | 20 ++--- api4/config_test.go | 30 +++---- api4/license_test.go | 28 ++----- api4/preference_test.go | 25 ++---- api4/user_test.go | 6 +- app/channel_test.go | 9 +- app/command_test.go | 20 ++--- app/helper_test.go | 2 +- app/plugin_commands_test.go | 4 +- app/plugin_shutdown_test.go | 4 +- app/webhook_test.go | 2 +- config/watcher_test.go | 4 +- model/channel_member_test.go | 6 -- model/config_test.go | 4 +- model/team_member_test.go | 11 --- store/storetest/emoji_store.go | 23 +++-- store/storetest/job_store.go | 148 ++++++++++++--------------------- 18 files changed, 119 insertions(+), 229 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 2d681c5515..fc035f75b8 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -169,7 +169,7 @@ func (me *TestHelper) ShutdownApp() { select { case <-done: case <-time.After(30 * time.Second): - // panic instead of t.Fatal to terminate all tests in this package, otherwise the + // panic instead of fatal to terminate all tests in this package, otherwise the // still running App could spuriously fail subsequent tests. panic("failed to shutdown App within 30 seconds") } diff --git a/api4/channel_test.go b/api4/channel_test.go index ea272ec52b..adb26fc9b9 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -636,9 +636,7 @@ func TestGetDeletedChannelsForTeam(t *testing.T) { channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") CheckNoError(t, resp) - if len(channels) != numInitialChannelsForTeam+3 { - t.Fatal("should be 3 deleted channels") - } + require.Len(t, channels, numInitialChannelsForTeam+3) // Login as different user and create private channel th.LoginBasic2() @@ -650,9 +648,7 @@ func TestGetDeletedChannelsForTeam(t *testing.T) { channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") CheckNoError(t, resp) - if len(channels) != numInitialChannelsForTeam+3 { - t.Fatal("should still be 3 deleted channels", len(channels), numInitialChannelsForTeam+3) - } + require.Len(t, channels, numInitialChannelsForTeam+3) channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 1, "") CheckNoError(t, resp) @@ -976,18 +972,14 @@ func TestSearchArchivedChannels(t *testing.T) { found := false for _, c := range channels { - if c.Type != model.CHANNEL_OPEN { - t.Fatal("should only return public channels") - } + require.Equal(t, model.CHANNEL_OPEN, c.Type) if c.Id == th.BasicChannel.Id { found = true } } - if !found { - t.Fatal("didn't find channel") - } + require.True(t, found) search.Term = th.BasicPrivateChannel.Name Client.DeleteChannel(th.BasicPrivateChannel.Id) @@ -1002,9 +994,7 @@ func TestSearchArchivedChannels(t *testing.T) { } } - if !found { - t.Fatal("couldn't find private channel") - } + require.True(t, found) search.Term = "" _, resp = Client.SearchArchivedChannels(th.BasicTeam.Id, search) diff --git a/api4/config_test.go b/api4/config_test.go index 5e69e0d5eb..6478ef1e75 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -317,13 +317,8 @@ func TestGetOldClientConfig(t *testing.T) { config, resp := Client.GetOldClientConfig("") CheckNoError(t, resp) - if len(config["Version"]) == 0 { - t.Fatal("config not returned correctly") - } - - if config["GoogleDeveloperKey"] != testKey { - t.Fatal("config missing developer key") - } + require.NotEmpty(t, config["Version"], "config not returned correctly") + require.Equal(t, testKey, config["GoogleDeveloperKey"]) }) t.Run("without session", func(t *testing.T) { @@ -336,29 +331,24 @@ func TestGetOldClientConfig(t *testing.T) { config, resp := Client.GetOldClientConfig("") CheckNoError(t, resp) - if len(config["Version"]) == 0 { - t.Fatal("config not returned correctly") - } - - if _, ok := config["GoogleDeveloperKey"]; ok { - t.Fatal("config should be missing developer key") - } + require.NotEmpty(t, config["Version"], "config not returned correctly") + require.Empty(t, config["GoogleDeveloperKey"], "config should be missing developer key") }) t.Run("missing format", func(t *testing.T) { Client := th.Client - if _, err := Client.DoApiGet("/config/client", ""); err == nil || err.StatusCode != http.StatusNotImplemented { - t.Fatal("should have errored with 501") - } + _, err := Client.DoApiGet("/config/client", "") + require.NotNil(t, err) + require.Equal(t, http.StatusNotImplemented, err.StatusCode) }) t.Run("invalid format", func(t *testing.T) { Client := th.Client - if _, err := Client.DoApiGet("/config/client?format=junk", ""); err == nil || err.StatusCode != http.StatusBadRequest { - t.Fatal("should have errored with 400") - } + _, err := Client.DoApiGet("/config/client?format=junk", "") + require.NotNil(t, err) + require.Equal(t, http.StatusBadRequest, err.StatusCode) }) } diff --git a/api4/license_test.go b/api4/license_test.go index d0b46d7cde..87409fbed0 100644 --- a/api4/license_test.go +++ b/api4/license_test.go @@ -40,9 +40,7 @@ func TestGetOldClientLicense(t *testing.T) { license, resp = th.SystemAdminClient.GetOldClientLicense("") CheckNoError(t, resp) - if len(license["IsLicensed"]) == 0 { - t.Fatal("license not returned correctly") - } + require.NotEmpty(t, license["IsLicensed"], "license not returned correctly") } func TestUploadLicenseFile(t *testing.T) { @@ -53,17 +51,13 @@ func TestUploadLicenseFile(t *testing.T) { t.Run("as system user", func(t *testing.T) { ok, resp := Client.UploadLicenseFile([]byte{}) CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should fail") - } + require.False(t, ok) }) t.Run("as system admin user", func(t *testing.T) { ok, resp := th.SystemAdminClient.UploadLicenseFile([]byte{}) CheckBadRequestStatus(t, resp) - if ok { - t.Fatal("should fail") - } + require.False(t, ok) }) t.Run("as restricted system admin user", func(t *testing.T) { @@ -71,9 +65,7 @@ func TestUploadLicenseFile(t *testing.T) { ok, resp := th.SystemAdminClient.UploadLicenseFile([]byte{}) CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should fail") - } + require.False(t, ok) }) } @@ -85,17 +77,13 @@ func TestRemoveLicenseFile(t *testing.T) { t.Run("as system user", func(t *testing.T) { ok, resp := Client.RemoveLicenseFile() CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should fail") - } + require.False(t, ok) }) t.Run("as system admin user", func(t *testing.T) { ok, resp := th.SystemAdminClient.RemoveLicenseFile() CheckNoError(t, resp) - if !ok { - t.Fatal("should pass") - } + require.True(t, ok) }) t.Run("as restricted system admin user", func(t *testing.T) { @@ -103,8 +91,6 @@ func TestRemoveLicenseFile(t *testing.T) { ok, resp := th.SystemAdminClient.RemoveLicenseFile() CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should fail") - } + require.False(t, ok) }) } diff --git a/api4/preference_test.go b/api4/preference_test.go index 565b2b5231..f9f99ad96c 100644 --- a/api4/preference_test.go +++ b/api4/preference_test.go @@ -328,9 +328,7 @@ func TestDeletePreferences(t *testing.T) { CheckForbiddenStatus(t, resp) prefs, _ = Client.GetPreferences(th.BasicUser.Id) - if len(prefs) != originalCount { - t.Fatal("should've deleted preferences") - } + require.Len(t, prefs, originalCount, "should've deleted preferences") Client.Logout() _, resp = Client.DeletePreferences(th.BasicUser.Id, &preferences) @@ -358,15 +356,12 @@ func TestDeletePreferencesWebsocket(t *testing.T) { CheckNoError(t, resp) WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + wsResp := <-WebSocketClient.ResponseChannel + require.Equal(t, model.STATUS_OK, wsResp.Status, "should have responded OK to authentication challenge") _, resp = th.Client.DeletePreferences(userId, preferences) CheckNoError(t, resp) @@ -383,19 +378,17 @@ func TestDeletePreferencesWebsocket(t *testing.T) { } received, err := model.PreferencesFromJson(strings.NewReader(event.GetData()["preferences"].(string))) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) for i, preference := range *preferences { - if preference.UserId != received[i].UserId || preference.Category != received[i].Category || preference.Name != received[i].Name { - t.Fatal("received incorrect preference") - } + require.Equal(t, preference.UserId, received[i].UserId) + require.Equal(t, preference.Category, received[i].Category) + require.Equal(t, preference.Name, received[i].Name) } waiting = false case <-timeout: - t.Fatal("timed out waiting for preference delete event") + require.Fail(t, "timed out waiting for preference delete event") } } } diff --git a/api4/user_test.go b/api4/user_test.go index a7752427b4..6f6b0d8656 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1681,7 +1681,7 @@ func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, e return } case <-time.After(5 * time.Second): - t.Fatalf("failed to receive expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED) + require.Failf(t, "failed to receive expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED) } } } @@ -2713,7 +2713,7 @@ func TestSetProfileImage(t *testing.T) { } else if resp.StatusCode == http.StatusUnauthorized { CheckUnauthorizedStatus(t, resp) } else { - t.Fatal("Should have failed either forbidden or unauthorized") + require.Fail(t, "Should have failed either forbidden or unauthorized") } buser, err := th.App.GetUser(user.Id) @@ -2753,7 +2753,7 @@ func TestSetDefaultProfileImage(t *testing.T) { } else if resp.StatusCode == http.StatusUnauthorized { CheckUnauthorizedStatus(t, resp) } else { - t.Fatal("Should have failed either forbidden or unauthorized") + require.Fail(t, "Should have failed either forbidden or unauthorized") } _, resp = th.SystemAdminClient.SetDefaultProfileImage(user.Id) diff --git a/app/channel_test.go b/app/channel_test.go index 0944021e0d..d5cd6e623a 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -405,7 +405,7 @@ func TestAddUserToChannelCreatesChannelMemberHistoryRecord(t *testing.T) { // the user leaves that channel if err := th.App.LeaveChannel(publicChannel.Id, th.BasicUser.Id); err != nil { - t.Fatal("Failed to remove user from channel. Error: " + err.Message) + require.Fail(t, "Failed to remove user from channel. Error: " + err.Message) } histories = store.Must(th.App.Srv().Store.ChannelMemberHistory().GetUsersInChannelDuring(model.GetMillis()-100, model.GetMillis()+100, publicChannel.Id)).([]*model.ChannelMemberHistoryResult) assert.Len(t, histories, 1) @@ -477,9 +477,8 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) { // create a user and add it to a channel user := th.CreateUser() - if _, err := th.App.AddTeamMember(th.BasicTeam.Id, user.Id); err != nil { - t.Fatal("Failed to add user to team. Error: " + err.Message) - } + _, err := th.App.AddTeamMember(th.BasicTeam.Id, user.Id) + require.Nil(t, err) groupUserIds := make([]string, 0) groupUserIds = append(groupUserIds, th.BasicUser.Id) @@ -488,7 +487,7 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) { channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN) userRequestorId := "" postRootId := "" - _, err := th.App.AddChannelMember(user.Id, channel, userRequestorId, postRootId) + _, err = th.App.AddChannelMember(user.Id, channel, userRequestorId, postRootId) require.Nil(t, err, "Failed to add user to channel.") // there should be a ChannelMemberHistory record for the user diff --git a/app/command_test.go b/app/command_test.go index 44b0380d4b..b6feed7c3c 100644 --- a/app/command_test.go +++ b/app/command_test.go @@ -70,9 +70,8 @@ func TestCreateCommandPost(t *testing.T) { skipSlackParsing := false _, err := th.App.CreateCommandPost(post, th.BasicTeam.Id, resp, skipSlackParsing) - if err == nil || err.Id != "api.context.invalid_param.app_error" { - t.Fatal("should have failed - bad post type") - } + require.NotNil(t, err) + require.Equal(t, err.Id, "api.context.invalid_param.app_error") } func TestHandleCommandResponsePost(t *testing.T) { @@ -208,9 +207,8 @@ func TestHandleCommandResponsePost(t *testing.T) { args.UserId = th.BasicUser2.Id post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) - if err == nil || err.Id != "api.command.command_post.forbidden.app_error" { - t.Fatal("should have failed - forbidden channel post") - } + require.NotNil(t, err) + require.Equal(t, err.Id, "api.command.command_post.forbidden.app_error") // Test that /code text is not converted with the Slack text conversion. command.Trigger = "code" @@ -252,9 +250,8 @@ func TestHandleCommandResponse(t *testing.T) { builtIn := true _, err := th.App.HandleCommandResponse(command, args, resp, builtIn) - if err == nil || err.Id != "api.command.execute_command.create_post_failed.app_error" { - t.Fatal("should have failed - invalid post type") - } + require.NotNil(t, err) + require.Equal(t, err.Id, "api.command.execute_command.create_post_failed.app_error") resp = &model.CommandResponse{ Text: "message 1", @@ -277,9 +274,8 @@ func TestHandleCommandResponse(t *testing.T) { } _, err = th.App.HandleCommandResponse(command, args, resp, builtIn) - if err == nil || err.Id != "api.command.execute_command.create_post_failed.app_error" { - t.Fatal("should have failed - invalid post type on extra response") - } + require.NotNil(t, err) + require.Equal(t, err.Id, "api.command.execute_command.create_post_failed.app_error") resp = &model.CommandResponse{ ExtraResponses: []*model.CommandResponse{ diff --git a/app/helper_test.go b/app/helper_test.go index 7ae258c9ce..a3374b9cac 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -471,7 +471,7 @@ func (me *TestHelper) ShutdownApp() { select { case <-done: case <-time.After(30 * time.Second): - // panic instead of t.Fatal to terminate all tests in this package, otherwise the + // panic instead of fatal to terminate all tests in this package, otherwise the // still running App could spuriously fail subsequent tests. panic("failed to shutdown App within 30 seconds") } diff --git a/app/plugin_commands_test.go b/app/plugin_commands_test.go index 44700e335f..6fbd678422 100644 --- a/app/plugin_commands_test.go +++ b/app/plugin_commands_test.go @@ -210,9 +210,7 @@ func TestPluginCommand(t *testing.T) { } th.App.RemovePlugin(pluginIds[0]) - if killed { - t.Fatal("execute command appears to have deadlocked") - } + require.False(t, killed, "execute command appears to have deadlocked") }) t.Run("error after plugin command unregistered", func(t *testing.T) { diff --git a/app/plugin_shutdown_test.go b/app/plugin_shutdown_test.go index 9cc618c8c8..dddf786c82 100644 --- a/app/plugin_shutdown_test.go +++ b/app/plugin_shutdown_test.go @@ -6,6 +6,8 @@ package app import ( "testing" "time" + + "github.com/stretchr/testify/require" ) func TestPluginShutdownTest(t *testing.T) { @@ -67,6 +69,6 @@ func TestPluginShutdownTest(t *testing.T) { select { case <-done: case <-time.After(15 * time.Second): - t.Fatal("failed to force plugin shutdown after 10 seconds") + require.Fail(t, "failed to force plugin shutdown after 10 seconds") } } diff --git a/app/webhook_test.go b/app/webhook_test.go index 1af4ca3805..b428c62cff 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -623,7 +623,7 @@ func TestTriggerOutGoingWebhookWithUsernameAndIconURL(t *testing.T) { assert.Nil(t, webhookPost.Props["override_username"]) } case <-time.After(5 * time.Second): - t.Fatal("Timeout, webhook response not created as post") + require.Fail(t, "Timeout, webhook response not created as post") } }) diff --git a/config/watcher_test.go b/config/watcher_test.go index 43f967e54b..2eaf1fc54f 100644 --- a/config/watcher_test.go +++ b/config/watcher_test.go @@ -49,7 +49,7 @@ func TestWatcher(t *testing.T) { ioutil.WriteFile(filepath.Join(tempDir, "unrelated"), []byte("data"), 0644) select { case <-called: - t.Fatal("callback should not have been called for unrelated file") + require.Fail(t, "callback should not have been called for unrelated file") case <-time.After(1 * time.Second): } @@ -58,6 +58,6 @@ func TestWatcher(t *testing.T) { select { case <-called: case <-time.After(5 * time.Second): - t.Fatal("callback should have been called when file written") + require.Fail(t, "callback should have been called when file written") } } diff --git a/model/channel_member_test.go b/model/channel_member_test.go index 033aaf09c0..1e2fea8406 100644 --- a/model/channel_member_test.go +++ b/model/channel_member_test.go @@ -28,12 +28,6 @@ func TestChannelMemberIsValid(t *testing.T) { o.NotifyProps = GetDefaultChannelNotifyProps() o.UserId = NewId() - /*o.Roles = "missing" - o.NotifyProps = GetDefaultChannelNotifyProps() - o.UserId = NewId() - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - }*/ o.NotifyProps["desktop"] = "junk" require.Error(t, o.IsValid(), "should be invalid") diff --git a/model/config_test.go b/model/config_test.go index 2aaff55577..1ca5596f78 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -181,9 +181,7 @@ func TestConfigOverwriteAdminSettings(t *testing.T) { c1.SetDefaults() - if *c1.SamlSettings.AdminAttribute != attribute { - t.Fatal("SamlSettings.AdminAttribute should be overwritten") - } + require.Equal(t, *c1.SamlSettings.AdminAttribute, attribute) } func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) { diff --git a/model/team_member_test.go b/model/team_member_test.go index ba05f25d7f..7652056cff 100644 --- a/model/team_member_test.go +++ b/model/team_member_test.go @@ -26,17 +26,6 @@ func TestTeamMemberIsValid(t *testing.T) { o.TeamId = NewId() require.Error(t, o.IsValid(), "should be invalid") - - /*o.UserId = NewId() - o.Roles = "blahblah" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } - - o.Roles = "" - if err := o.IsValid(); err != nil { - t.Fatal(err) - }*/ } func TestUnreadMemberJson(t *testing.T) { diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 5bdb238e50..14774923ee 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -206,21 +206,20 @@ func testEmojiGetList(t *testing.T, ss store.Store) { } }() - if result, err := ss.Emoji().GetList(0, 100, ""); err != nil { - t.Fatal(err) - } else { - for _, emoji := range emojis { - found := false + result, err := ss.Emoji().GetList(0, 100, "") + require.Nil(t, err) - for _, savedEmoji := range result { - if emoji.Id == savedEmoji.Id { - found = true - break - } + for _, emoji := range emojis { + found := false + + for _, savedEmoji := range result { + if emoji.Id == savedEmoji.Id { + found = true + break } - - require.Truef(t, found, "failed to get emoji with id %v", emoji.Id) } + + require.Truef(t, found, "failed to get emoji with id %v", emoji.Id) } remojis, err := ss.Emoji().GetList(0, 3, model.EMOJI_SORT_BY_NAME) diff --git a/store/storetest/job_store.go b/store/storetest/job_store.go index 8cac520afd..d66933ce3c 100644 --- a/store/storetest/job_store.go +++ b/store/storetest/job_store.go @@ -44,13 +44,10 @@ func testJobSaveGet(t *testing.T, ss store.Store) { defer ss.Job().Delete(job.Id) - if received, err := ss.Job().Get(job.Id); err != nil { - t.Fatal(err) - } else if received.Id != job.Id { - t.Fatal("received incorrect job after save") - } else if received.Data["Total"] != "12345" { - t.Fatal("data field was not retrieved successfully:", received.Data) - } + received, err := ss.Job().Get(job.Id) + require.Nil(t, err) + require.Equal(t, job.Id, received.Id, "received incorrect job after save") + require.Equal(t, "12345", received.Data["Total"]) } func testJobGetAllByType(t *testing.T, ss store.Store) { @@ -77,15 +74,10 @@ func testJobGetAllByType(t *testing.T, ss store.Store) { defer ss.Job().Delete(job.Id) } - if received, err := ss.Job().GetAllByType(jobType); err != nil { - t.Fatal(err) - } else if len(received) != 2 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[0].Id && received[1].Id != jobs[0].Id { - t.Fatal("should've received first jobs") - } else if received[0].Id != jobs[1].Id && received[1].Id != jobs[1].Id { - t.Fatal("should've received second jobs") - } + received, err := ss.Job().GetAllByType(jobType) + require.Nil(t, err) + require.Len(t, received, 2) + require.ElementsMatch(t, []string{jobs[0].Id, jobs[1].Id}, []string{received[0].Id, received[1].Id}) } func testJobGetAllByTypePage(t *testing.T, ss store.Store) { @@ -120,23 +112,16 @@ func testJobGetAllByTypePage(t *testing.T, ss store.Store) { defer ss.Job().Delete(job.Id) } - if received, err := ss.Job().GetAllByTypePage(jobType, 0, 2); err != nil { - t.Fatal(err) - } else if len(received) != 2 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[2].Id { - t.Fatal("should've received newest job first") - } else if received[1].Id != jobs[0].Id { - t.Fatal("should've received second newest job second") - } + received, err := ss.Job().GetAllByTypePage(jobType, 0, 2) + require.Nil(t, err) + require.Len(t, received, 2) + require.Equal(t, received[0].Id, jobs[2].Id, "should've received newest job first") + require.Equal(t, received[1].Id, jobs[0].Id, "should've received second newest job second") - if received, err := ss.Job().GetAllByTypePage(jobType, 2, 2); err != nil { - t.Fatal(err) - } else if len(received) != 1 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[1].Id { - t.Fatal("should've received oldest job last") - } + received, err = ss.Job().GetAllByTypePage(jobType, 2, 2) + require.Nil(t, err) + require.Len(t, received, 1) + require.Equal(t, received[0].Id, jobs[1].Id, "should've received oldest job last") } func testJobGetAllPage(t *testing.T, ss store.Store) { @@ -167,23 +152,16 @@ func testJobGetAllPage(t *testing.T, ss store.Store) { defer ss.Job().Delete(job.Id) } - if received, err := ss.Job().GetAllPage(0, 2); err != nil { - t.Fatal(err) - } else if len(received) != 2 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[2].Id { - t.Fatal("should've received newest job first") - } else if received[1].Id != jobs[0].Id { - t.Fatal("should've received second newest job second") - } + received, err := ss.Job().GetAllPage(0, 2) + require.Nil(t, err) + require.Len(t, received, 2) + require.Equal(t, received[0].Id, jobs[2].Id, "should've received newest job first") + require.Equal(t, received[1].Id, jobs[0].Id, "should've received second newest job second") - if received, err := ss.Job().GetAllPage(2, 2); err != nil { - t.Fatal(err) - } else if len(received) < 1 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[1].Id { - t.Fatal("should've received oldest job last") - } + received, err = ss.Job().GetAllPage(2, 2) + require.Nil(t, err) + require.NotEmpty(t, received) + require.Equal(t, received[0].Id, jobs[1].Id, "should've received oldest job last") } func testJobGetAllByStatus(t *testing.T, ss store.Store) { @@ -226,15 +204,13 @@ func testJobGetAllByStatus(t *testing.T, ss store.Store) { defer ss.Job().Delete(job.Id) } - if received, err := ss.Job().GetAllByStatus(status); err != nil { - t.Fatal(err) - } else if len(received) != 3 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[1].Id || received[1].Id != jobs[0].Id || received[2].Id != jobs[2].Id { - t.Fatal("should've received jobs ordered by CreateAt time") - } else if received[1].Data["test"] != "data" { - t.Fatal("should've received job data field back as saved") - } + received, err := ss.Job().GetAllByStatus(status) + require.Nil(t, err) + require.Len(t, received, 3) + require.Equal(t, received[0].Id, jobs[1].Id) + require.Equal(t, received[1].Id, jobs[0].Id) + require.Equal(t, received[2].Id, jobs[2].Id) + require.Equal(t, "data", received[1].Data["test"], "should've received job data field back as saved") } func testJobStoreGetNewestJobByStatusAndType(t *testing.T, ss store.Store) { @@ -360,24 +336,24 @@ func testJobUpdateOptimistically(t *testing.T, ss store.Store) { "Foo": "Bar", } - if updated, err2 := ss.Job().UpdateOptimistically(job, model.JOB_STATUS_SUCCESS); err2 != nil { - if updated { - t.Fatal("should have failed due to incorrect old status") - } - } + updated, err := ss.Job().UpdateOptimistically(job, model.JOB_STATUS_SUCCESS) + require.False(t, err != nil && updated) time.Sleep(2 * time.Millisecond) - updated, err := ss.Job().UpdateOptimistically(job, model.JOB_STATUS_PENDING) + updated, err = ss.Job().UpdateOptimistically(job, model.JOB_STATUS_PENDING) require.Nil(t, err) require.True(t, updated) updatedJob, err := ss.Job().Get(job.Id) require.Nil(t, err) - if updatedJob.Type != job.Type || updatedJob.CreateAt != job.CreateAt || updatedJob.Status != job.Status || updatedJob.LastActivityAt <= job.LastActivityAt || updatedJob.Progress != job.Progress || updatedJob.Data["Foo"] != job.Data["Foo"] { - t.Fatal("Some update property was not as expected") - } + require.Equal(t, updatedJob.Type, job.Type) + require.Equal(t, updatedJob.CreateAt, job.CreateAt) + require.Equal(t, updatedJob.Status, job.Status) + require.Greater(t, updatedJob.LastActivityAt, job.LastActivityAt) + require.Equal(t, updatedJob.Progress, job.Progress) + require.Equal(t, updatedJob.Data["Foo"], job.Data["Foo"]) } func testJobUpdateStatusUpdateStatusOptimistically(t *testing.T, ss store.Store) { @@ -400,12 +376,8 @@ func testJobUpdateStatusUpdateStatusOptimistically(t *testing.T, ss store.Store) received, err = ss.Job().UpdateStatus(job.Id, model.JOB_STATUS_PENDING) require.Nil(t, err) - if received.Status != model.JOB_STATUS_PENDING { - t.Fatal("status wasn't updated") - } - if received.LastActivityAt <= lastUpdateAt { - t.Fatal("lastActivityAt wasn't updated") - } + require.Equal(t, model.JOB_STATUS_PENDING, received.Status) + require.Greater(t, received.LastActivityAt, lastUpdateAt) lastUpdateAt = received.LastActivityAt time.Sleep(2 * time.Millisecond) @@ -417,12 +389,8 @@ func testJobUpdateStatusUpdateStatusOptimistically(t *testing.T, ss store.Store) received, err = ss.Job().Get(job.Id) require.Nil(t, err) - if received.Status != model.JOB_STATUS_PENDING { - t.Fatal("should still be pending") - } - if received.LastActivityAt != lastUpdateAt { - t.Fatal("last activity at shouldn't have changed") - } + require.Equal(t, model.JOB_STATUS_PENDING, received.Status) + require.Equal(t, received.LastActivityAt, lastUpdateAt) time.Sleep(2 * time.Millisecond) @@ -433,15 +401,9 @@ func testJobUpdateStatusUpdateStatusOptimistically(t *testing.T, ss store.Store) var startAtSet int64 received, err = ss.Job().Get(job.Id) require.Nil(t, err) - if received.Status != model.JOB_STATUS_IN_PROGRESS { - t.Fatal("should be in progress") - } - if received.StartAt == 0 { - t.Fatal("received should have start at set") - } - if received.LastActivityAt <= lastUpdateAt { - t.Fatal("lastActivityAt wasn't updated") - } + require.Equal(t, model.JOB_STATUS_IN_PROGRESS, received.Status) + require.NotEqual(t, 0, received.StartAt) + require.Greater(t, received.LastActivityAt, lastUpdateAt) lastUpdateAt = received.LastActivityAt startAtSet = received.StartAt @@ -453,15 +415,9 @@ func testJobUpdateStatusUpdateStatusOptimistically(t *testing.T, ss store.Store) received, err = ss.Job().Get(job.Id) require.Nil(t, err) - if received.Status != model.JOB_STATUS_SUCCESS { - t.Fatal("should be success status") - } - if received.StartAt != startAtSet { - t.Fatal("startAt should not have changed") - } - if received.LastActivityAt <= lastUpdateAt { - t.Fatal("lastActivityAt wasn't updated") - } + require.Equal(t, model.JOB_STATUS_SUCCESS, received.Status) + require.Equal(t, startAtSet, received.StartAt) + require.Greater(t, received.LastActivityAt, lastUpdateAt) } func testJobDelete(t *testing.T, ss store.Store) {