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

@@ -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")
 	}

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 {
+	require.Len(t, channels, numInitialChannelsForTeam+3)
-		t.Fatal("should be 3 deleted channels")
-	}
 
 	// 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 {
+	require.Len(t, channels, numInitialChannelsForTeam+3)
-		t.Fatal("should still be 3 deleted channels", len(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 {
+		require.Equal(t, model.CHANNEL_OPEN, c.Type)
-			t.Fatal("should only return public channels")
-		}
 
 		if c.Id == th.BasicChannel.Id {
 			found = true
 		}
 	}
 
-	if !found {
+	require.True(t, found)
-		t.Fatal("didn't find channel")
-	}
 
 	search.Term = th.BasicPrivateChannel.Name
 	Client.DeleteChannel(th.BasicPrivateChannel.Id)
@@ -1002,9 +994,7 @@ func TestSearchArchivedChannels(t *testing.T) {
 		}
 	}
 
-	if !found {
+	require.True(t, found)
-		t.Fatal("couldn't find private channel")
-	}
 
 	search.Term = ""
 	_, resp = Client.SearchArchivedChannels(th.BasicTeam.Id, search)

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 {
+		require.NotEmpty(t, config["Version"], "config not returned correctly")
-			t.Fatal("config not returned correctly")
+		require.Equal(t, testKey, config["GoogleDeveloperKey"])
-		}
-
-		if config["GoogleDeveloperKey"] != testKey {
-			t.Fatal("config missing developer key")
-		}
 	})
 
 	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 {
+		require.NotEmpty(t, config["Version"], "config not returned correctly")
-			t.Fatal("config not returned correctly")
+		require.Empty(t, config["GoogleDeveloperKey"], "config should be missing developer key")
-		}
-
-		if _, ok := config["GoogleDeveloperKey"]; ok {
-			t.Fatal("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 {
+		_, err := Client.DoApiGet("/config/client", "")
-			t.Fatal("should have errored with 501")
+		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 {
+		_, err := Client.DoApiGet("/config/client?format=junk", "")
-			t.Fatal("should have errored with 400")
+		require.NotNil(t, err)
-		}
+		require.Equal(t, http.StatusBadRequest, err.StatusCode)
 	})
 }
 

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 {
+	require.NotEmpty(t, license["IsLicensed"], "license not returned correctly")
-		t.Fatal("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 {
+		require.False(t, ok)
-			t.Fatal("should fail")
-		}
 	})
 
 	t.Run("as system admin user", func(t *testing.T) {
 		ok, resp := th.SystemAdminClient.UploadLicenseFile([]byte{})
 		CheckBadRequestStatus(t, resp)
-		if ok {
+		require.False(t, ok)
-			t.Fatal("should fail")
-		}
 	})
 
 	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 {
+		require.False(t, ok)
-			t.Fatal("should fail")
-		}
 	})
 }
 
@@ -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 {
+		require.False(t, ok)
-			t.Fatal("should fail")
-		}
 	})
 
 	t.Run("as system admin user", func(t *testing.T) {
 		ok, resp := th.SystemAdminClient.RemoveLicenseFile()
 		CheckNoError(t, resp)
-		if !ok {
+		require.True(t, ok)
-			t.Fatal("should pass")
-		}
 	})
 
 	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 {
+		require.False(t, ok)
-			t.Fatal("should fail")
-		}
 	})
 }

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 {
+	require.Len(t, prefs, originalCount, "should've deleted preferences")
-		t.Fatal("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 {
+	require.Nil(t, err)
-		t.Fatal(err)
-	}
 
 	WebSocketClient.Listen()
 	time.Sleep(300 * time.Millisecond)
-	if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK {
+	wsResp := <-WebSocketClient.ResponseChannel
-		t.Fatal("should have responded OK to authentication challenge")
+	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 {
+			require.Nil(t, err)
-				t.Fatal(err)
-			}
 
 			for i, preference := range *preferences {
-				if preference.UserId != received[i].UserId || preference.Category != received[i].Category || preference.Name != received[i].Name {
+				require.Equal(t, preference.UserId, received[i].UserId)
-					t.Fatal("received incorrect preference")
+				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")
 		}
 	}
 }

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)

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 {
+	_, err := th.App.AddTeamMember(th.BasicTeam.Id, user.Id)
-		t.Fatal("Failed to add user to team. Error: " + err.Message)
+	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

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" {
+	require.NotNil(t, err)
-		t.Fatal("should have failed - bad post type")
+	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" {
+	require.NotNil(t, err)
-		t.Fatal("should have failed - forbidden channel post")
+	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" {
+	require.NotNil(t, err)
-		t.Fatal("should have failed - invalid post type")
+	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" {
+	require.NotNil(t, err)
-		t.Fatal("should have failed - invalid post type on extra response")
+	require.Equal(t, err.Id, "api.command.execute_command.create_post_failed.app_error")
-	}
 
 	resp = &model.CommandResponse{
 		ExtraResponses: []*model.CommandResponse{

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")
 	}

app/plugin_commands_test.go

@@ -210,9 +210,7 @@ func TestPluginCommand(t *testing.T) {
 		}
 
 		th.App.RemovePlugin(pluginIds[0])
-		if killed {
+		require.False(t, killed, "execute command appears to have deadlocked")
-			t.Fatal("execute command appears to have deadlocked")
-		}
 	})
 
 	t.Run("error after plugin command unregistered", func(t *testing.T) {

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")
 	}
 }

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")
 			}
 
 		})

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")
 	}
 }

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")

model/config_test.go

@@ -181,9 +181,7 @@ func TestConfigOverwriteAdminSettings(t *testing.T) {
 
 	c1.SetDefaults()
 
-	if *c1.SamlSettings.AdminAttribute != attribute {
+	require.Equal(t, *c1.SamlSettings.AdminAttribute, attribute)
-		t.Fatal("SamlSettings.AdminAttribute should be overwritten")
-	}
 }
 
 func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) {

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) {

store/storetest/emoji_store.go

@@ -206,9 +206,9 @@ func testEmojiGetList(t *testing.T, ss store.Store) {
 		}
 	}()
 
-	if result, err := ss.Emoji().GetList(0, 100, ""); err != nil {
+	result, err := ss.Emoji().GetList(0, 100, "")
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+
 	for _, emoji := range emojis {
 		found := false
 
@@ -221,7 +221,6 @@ func testEmojiGetList(t *testing.T, ss store.Store) {
 
 		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)
 	assert.Nil(t, err)

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 {
+	received, err := ss.Job().Get(job.Id)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if received.Id != job.Id {
+	require.Equal(t, job.Id, received.Id, "received incorrect job after save")
-		t.Fatal("received incorrect job after save")
+	require.Equal(t, "12345", received.Data["Total"])
-	} else if received.Data["Total"] != "12345" {
-		t.Fatal("data field was not retrieved successfully:", received.Data)
-	}
 }
 
 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 {
+	received, err := ss.Job().GetAllByType(jobType)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) != 2 {
+	require.Len(t, received, 2)
-		t.Fatal("received wrong number of jobs")
+	require.ElementsMatch(t, []string{jobs[0].Id, jobs[1].Id}, []string{received[0].Id, received[1].Id})
-	} 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")
-	}
 }
 
 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 {
+	received, err := ss.Job().GetAllByTypePage(jobType, 0, 2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) != 2 {
+	require.Len(t, received, 2)
-		t.Fatal("received wrong number of jobs")
+	require.Equal(t, received[0].Id, jobs[2].Id, "should've received newest job first")
-	} else if received[0].Id != jobs[2].Id {
+	require.Equal(t, received[1].Id, jobs[0].Id, "should've received second newest job second")
-		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")
-	}
 
-	if received, err := ss.Job().GetAllByTypePage(jobType, 2, 2); err != nil {
+	received, err = ss.Job().GetAllByTypePage(jobType, 2, 2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) != 1 {
+	require.Len(t, received, 1)
-		t.Fatal("received wrong number of jobs")
+	require.Equal(t, received[0].Id, jobs[1].Id, "should've received oldest job last")
-	} else if received[0].Id != jobs[1].Id {
-		t.Fatal("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 {
+	received, err := ss.Job().GetAllPage(0, 2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) != 2 {
+	require.Len(t, received, 2)
-		t.Fatal("received wrong number of jobs")
+	require.Equal(t, received[0].Id, jobs[2].Id, "should've received newest job first")
-	} else if received[0].Id != jobs[2].Id {
+	require.Equal(t, received[1].Id, jobs[0].Id, "should've received second newest job second")
-		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")
-	}
 
-	if received, err := ss.Job().GetAllPage(2, 2); err != nil {
+	received, err = ss.Job().GetAllPage(2, 2)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) < 1 {
+	require.NotEmpty(t, received)
-		t.Fatal("received wrong number of jobs")
+	require.Equal(t, received[0].Id, jobs[1].Id, "should've received oldest job last")
-	} else if received[0].Id != jobs[1].Id {
-		t.Fatal("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 {
+	received, err := ss.Job().GetAllByStatus(status)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else if len(received) != 3 {
+	require.Len(t, received, 3)
-		t.Fatal("received wrong number of jobs")
+	require.Equal(t, received[0].Id, jobs[1].Id)
-	} else if received[0].Id != jobs[1].Id || received[1].Id != jobs[0].Id || received[2].Id != jobs[2].Id {
+	require.Equal(t, received[1].Id, jobs[0].Id)
-		t.Fatal("should've received jobs ordered by CreateAt time")
+	require.Equal(t, received[2].Id, jobs[2].Id)
-	} else if received[1].Data["test"] != "data" {
+	require.Equal(t, "data", received[1].Data["test"], "should've received job data field back as saved")
-		t.Fatal("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 {
+	updated, err := ss.Job().UpdateOptimistically(job, model.JOB_STATUS_SUCCESS)
-		if updated {
+	require.False(t, err != nil && updated)
-			t.Fatal("should have failed due to incorrect old status")
-		}
-	}
 
 	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"] {
+	require.Equal(t, updatedJob.Type, job.Type)
-		t.Fatal("Some update property was not as expected")
+	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 {
+	require.Equal(t, model.JOB_STATUS_PENDING, received.Status)
-		t.Fatal("status wasn't updated")
+	require.Greater(t, received.LastActivityAt, lastUpdateAt)
-	}
-	if received.LastActivityAt <= lastUpdateAt {
-		t.Fatal("lastActivityAt wasn't updated")
-	}
 	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 {
+	require.Equal(t, model.JOB_STATUS_PENDING, received.Status)
-		t.Fatal("should still be pending")
+	require.Equal(t, received.LastActivityAt, lastUpdateAt)
-	}
-	if received.LastActivityAt != lastUpdateAt {
-		t.Fatal("last activity at shouldn't have changed")
-	}
 
 	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 {
+	require.Equal(t, model.JOB_STATUS_IN_PROGRESS, received.Status)
-		t.Fatal("should be in progress")
+	require.NotEqual(t, 0, received.StartAt)
-	}
+	require.Greater(t, received.LastActivityAt, lastUpdateAt)
-	if received.StartAt == 0 {
-		t.Fatal("received should have start at set")
-	}
-	if received.LastActivityAt <= lastUpdateAt {
-		t.Fatal("lastActivityAt wasn't updated")
-	}
 	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 {
+	require.Equal(t, model.JOB_STATUS_SUCCESS, received.Status)
-		t.Fatal("should be success status")
+	require.Equal(t, startAtSet, received.StartAt)
-	}
+	require.Greater(t, received.LastActivityAt, lastUpdateAt)
-	if received.StartAt != startAtSet {
-		t.Fatal("startAt should not have changed")
-	}
-	if received.LastActivityAt <= lastUpdateAt {
-		t.Fatal("lastActivityAt wasn't updated")
-	}
 }
 
 func testJobDelete(t *testing.T, ss store.Store) {