From b219f9b1c2bcd7a62cfae473fce93c51ffc006d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Sat, 16 Mar 2024 11:09:18 +0100 Subject: [PATCH] Manager tests: replace `assert.NoError()` with `require.NoError()` Back in the days when I wrote the code, I didn't know about the `require` package yet. Using `require.NoError()` makes the test code more straight-forward. No functional changes, except that when tests fail, they now fail without panicking. --- internal/manager/persistence/jobs_test.go | 26 ++++----------- .../persistence/task_scheduler_test.go | 32 ++++--------------- .../sleep_scheduler/sleep_scheduler_test.go | 5 ++- internal/manager/task_logs/task_logs_test.go | 9 ++---- .../timeout_checker/timeout_checker_test.go | 5 ++- 5 files changed, 21 insertions(+), 56 deletions(-) diff --git a/internal/manager/persistence/jobs_test.go b/internal/manager/persistence/jobs_test.go index d77c5d9e..06aed199 100644 --- a/internal/manager/persistence/jobs_test.go +++ b/internal/manager/persistence/jobs_test.go @@ -762,17 +762,12 @@ func createTestAuthoredJob(jobID string, tasks ...job_compilers.AuthoredTask) jo func persistAuthoredJob(t *testing.T, ctx context.Context, db *DB, authoredJob job_compilers.AuthoredJob) *Job { err := db.StoreAuthoredJob(ctx, authoredJob) - if err != nil { - t.Fatalf("error storing authored job in DB: %v", err) - } + require.NoError(t, err, "error storing authored job in DB") dbJob, err := db.FetchJob(ctx, authoredJob.JobID) - if err != nil { - t.Fatalf("error fetching job from DB: %v", err) - } - if dbJob == nil { - t.Fatalf("nil job obtained from DB but with no error!") - } + require.NoError(t, err, "error fetching job from DB") + require.NotNil(t, dbJob, "nil job obtained from DB but with no error!") + return dbJob } @@ -849,18 +844,11 @@ func createWorker(ctx context.Context, t *testing.T, db *DB, updaters ...func(*W } err := db.CreateWorker(ctx, &w) - if err != nil { - t.Fatalf("error creating worker: %v", err) - } - require.NoError(t, err) + require.NoError(t, err, "error creating worker") fetchedWorker, err := db.FetchWorker(ctx, w.UUID) - if err != nil { - t.Fatalf("error fetching worker: %v", err) - } - if fetchedWorker == nil { - t.Fatal("fetched worker is nil, but no error returned") - } + require.NoError(t, err, "error fetching worker") + require.NotNil(t, fetchedWorker, "fetched worker is nil, but no error returned") return fetchedWorker } diff --git a/internal/manager/persistence/task_scheduler_test.go b/internal/manager/persistence/task_scheduler_test.go index 1d0ae355..640b867f 100644 --- a/internal/manager/persistence/task_scheduler_test.go +++ b/internal/manager/persistence/task_scheduler_test.go @@ -410,21 +410,15 @@ func constructTestJob( ctx context.Context, t *testing.T, db *DB, authoredJob job_compilers.AuthoredJob, ) *Job { err := db.StoreAuthoredJob(ctx, authoredJob) - if err != nil { - t.Fatalf("storing authored job: %v", err) - } + require.NoError(t, err, "storing authored job") dbJob, err := db.FetchJob(ctx, authoredJob.JobID) - if err != nil { - t.Fatalf("fetching authored job: %v", err) - } + require.NoError(t, err, "fetching authored job") // Queue the job. dbJob.Status = api.JobStatusQueued err = db.SaveJobStatus(ctx, dbJob) - if err != nil { - t.Fatalf("queueing job: %v", err) - } + require.NoError(t, err, "queueing job") return dbJob } @@ -457,16 +451,11 @@ func authorTestTask(name, taskType string, dependencies ...*job_compilers.Author func setTaskStatus(t *testing.T, db *DB, taskUUID string, status api.TaskStatus) { ctx := context.Background() task, err := db.FetchTask(ctx, taskUUID) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) task.Status = status - err = db.SaveTask(ctx, task) - if err != nil { - t.Fatal(err) - } + require.NoError(t, db.SaveTask(ctx, task)) } func linuxWorker(t *testing.T, db *DB, updaters ...func(worker *Worker)) Worker { @@ -483,10 +472,7 @@ func linuxWorker(t *testing.T, db *DB, updaters ...func(worker *Worker)) Worker } err := db.gormDB.Save(&w).Error - if err != nil { - t.Logf("cannot save Linux worker: %v", err) - t.FailNow() - } + require.NoError(t, err, "cannot save Linux worker") return w } @@ -501,10 +487,6 @@ func windowsWorker(t *testing.T, db *DB) Worker { } err := db.gormDB.Save(&w).Error - if err != nil { - t.Logf("cannot save Windows worker: %v", err) - t.FailNow() - } - + require.NoError(t, err, "cannot save Windows worker") return w } diff --git a/internal/manager/sleep_scheduler/sleep_scheduler_test.go b/internal/manager/sleep_scheduler/sleep_scheduler_test.go index 378d489b..e02d1d94 100644 --- a/internal/manager/sleep_scheduler/sleep_scheduler_test.go +++ b/internal/manager/sleep_scheduler/sleep_scheduler_test.go @@ -267,9 +267,8 @@ func testFixtures(t *testing.T) (*SleepScheduler, TestMocks, context.Context) { mockedClock := clock.NewMock() mockedNow, err := time.Parse(time.RFC3339, "2022-06-07T11:14:47+02:00") - if err != nil { - panic(err) - } + require.NoError(t, err) + mockedClock.Set(mockedNow) if !assert.Equal(t, time.Tuesday.String(), mockedNow.Weekday().String()) { t.Fatal("tests assume 'now' is a Tuesday") diff --git a/internal/manager/task_logs/task_logs_test.go b/internal/manager/task_logs/task_logs_test.go index 17e35794..8fc4f512 100644 --- a/internal/manager/task_logs/task_logs_test.go +++ b/internal/manager/task_logs/task_logs_test.go @@ -217,9 +217,7 @@ func taskLogsTestFixtures(t *testing.T) (*Storage, func(), *TaskLogsMocks) { mockCtrl := gomock.NewController(t) temppath, err := ioutil.TempDir("", "testlogs") - if err != nil { - panic(err) - } + require.NoError(t, err) mocks := &TaskLogsMocks{ temppath: temppath, @@ -229,9 +227,8 @@ func taskLogsTestFixtures(t *testing.T) (*Storage, func(), *TaskLogsMocks) { } mockedNow, err := time.Parse(time.RFC3339, "2022-06-09T16:52:04+02:00") - if err != nil { - panic(err) - } + require.NoError(t, err) + mocks.clock.Set(mockedNow) // This should be called at the end of each unit test. diff --git a/internal/manager/timeout_checker/timeout_checker_test.go b/internal/manager/timeout_checker/timeout_checker_test.go index f0a3ed9a..d24cbef2 100644 --- a/internal/manager/timeout_checker/timeout_checker_test.go +++ b/internal/manager/timeout_checker/timeout_checker_test.go @@ -11,6 +11,7 @@ import ( "github.com/benbjohnson/clock" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "projects.blender.org/studio/flamenco/internal/manager/timeout_checker/mocks" ) @@ -50,9 +51,7 @@ func timeoutCheckerTestFixtures(t *testing.T) (*TimeoutChecker, func(), *Timeout } mockedNow, err := time.Parse(time.RFC3339, "2022-06-09T12:00:00+00:00") - if err != nil { - panic(err) - } + require.NoError(t, err) mocks.clock.Set(mockedNow) ctx, cancel := context.WithCancel(context.Background())