From 5da5edd33368e4a0ee2ddc13caff606953f65e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 24 Jul 2024 15:35:21 +0200 Subject: [PATCH] Manager: modernise Shaman unit test code Replace calls to `assert.NoError()` with `require.NoError()` --- pkg/shaman/checkout/checkout_test.go | 3 ++- pkg/shaman/checkout/manager_test.go | 15 ++++++----- .../checkout/report_requirements_test.go | 3 ++- pkg/shaman/cleanup_test.go | 25 ++++++++++--------- pkg/shaman/fileserver/receivefile_test.go | 5 ++-- pkg/shaman/filestore/filestore_test.go | 17 +++++++------ pkg/shaman/filestore/substore_test.go | 7 +++--- pkg/shaman/touch/touch_test.go | 3 ++- 8 files changed, 42 insertions(+), 36 deletions(-) diff --git a/pkg/shaman/checkout/checkout_test.go b/pkg/shaman/checkout/checkout_test.go index 53f0cd07..b5bb81cd 100644 --- a/pkg/shaman/checkout/checkout_test.go +++ b/pkg/shaman/checkout/checkout_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "projects.blender.org/studio/flamenco/pkg/api" "projects.blender.org/studio/flamenco/pkg/shaman/filestore" "projects.blender.org/studio/flamenco/pkg/shaman/testsupport" @@ -56,7 +57,7 @@ func TestCheckout(t *testing.T) { func assertLinksTo(t *testing.T, linkPath, expectedTarget string) { actualTarget, err := os.Readlink(linkPath) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, expectedTarget, actualTarget) } diff --git a/pkg/shaman/checkout/manager_test.go b/pkg/shaman/checkout/manager_test.go index ebd7d319..59453a4f 100644 --- a/pkg/shaman/checkout/manager_test.go +++ b/pkg/shaman/checkout/manager_test.go @@ -24,7 +24,6 @@ package checkout import ( "context" - "io/ioutil" "os" "path/filepath" "strings" @@ -54,33 +53,33 @@ func TestSymlinkToCheckout(t *testing.T) { // Fake an older file. blobPath := filepath.Join(manager.checkoutBasePath, "jemoeder.blob") - err := ioutil.WriteFile(blobPath, []byte("op je hoofd"), 0600) - assert.NoError(t, err) + err := os.WriteFile(blobPath, []byte("op je hoofd"), 0600) + require.NoError(t, err) wayBackWhen := time.Now().Add(-time.Hour * 24 * 100) err = os.Chtimes(blobPath, wayBackWhen, wayBackWhen) - assert.NoError(t, err) + require.NoError(t, err) symlinkRelativePath := "path/to/jemoeder.txt" err = manager.SymlinkToCheckout(blobPath, manager.checkoutBasePath, symlinkRelativePath) - assert.NoError(t, err) + require.NoError(t, err) err = manager.SymlinkToCheckout(blobPath, manager.checkoutBasePath, symlinkRelativePath) - assert.NoError(t, err, "symlinking a file twice should not be an issue") + require.NoError(t, err, "symlinking a file twice should not be an issue") // Wait for touch() calls to be done. manager.wg.Wait() // The blob should have been touched to indicate it was referenced just now. stat, err := os.Stat(blobPath) - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, stat.ModTime().After(wayBackWhen), "File must be touched (%v must be later than %v)", stat.ModTime(), wayBackWhen) symlinkPath := filepath.Join(manager.checkoutBasePath, symlinkRelativePath) stat, err = os.Lstat(symlinkPath) - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, stat.Mode()&os.ModeType == os.ModeSymlink, "%v should be a symlink", symlinkPath) } diff --git a/pkg/shaman/checkout/report_requirements_test.go b/pkg/shaman/checkout/report_requirements_test.go index 656fa32f..89d9bbd4 100644 --- a/pkg/shaman/checkout/report_requirements_test.go +++ b/pkg/shaman/checkout/report_requirements_test.go @@ -27,6 +27,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "projects.blender.org/studio/flamenco/pkg/api" ) @@ -44,7 +45,7 @@ func TestReportRequirements(t *testing.T) { } response, err := manager.ReportRequirements(context.Background(), required) - assert.NoError(t, err) + require.NoError(t, err) // We should not be required to upload the same file twice, so the duplicate // should not be in the response. diff --git a/pkg/shaman/cleanup_test.go b/pkg/shaman/cleanup_test.go index 584dda43..a9fd3692 100644 --- a/pkg/shaman/cleanup_test.go +++ b/pkg/shaman/cleanup_test.go @@ -33,6 +33,7 @@ import ( "github.com/rs/zerolog/log" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "projects.blender.org/studio/flamenco/pkg/shaman/config" "projects.blender.org/studio/flamenco/pkg/shaman/filestore" "projects.blender.org/studio/flamenco/pkg/shaman/jwtauth" @@ -101,7 +102,7 @@ func TestGCFindOldFiles(t *testing.T) { // Since all the links have just been created, nothing should be considered old. ageThreshold := server.gcAgeThreshold() old, err := server.gcFindOldFiles(ageThreshold, log.With().Str("test", "test").Logger()) - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, mtimeMap{}, old) // Make some files old, they should show up in a scan. @@ -111,7 +112,7 @@ func TestGCFindOldFiles(t *testing.T) { makeOld(server, expectOld, "stored/dc/89f15de821ad1df3e78f8ef455e653a2d1862f2eb3f5ee78aa4ca68eb6fb35/781.blob") old, err = server.gcFindOldFiles(ageThreshold, log.With().Str("package", "shaman/test").Logger()) - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, expectOld, old) } @@ -151,18 +152,18 @@ func TestGCComponents(t *testing.T) { // No symlinks created yet, so this should report all the files in oldFiles. oldFiles := copymap(expectOld) err := server.gcFilterLinkedFiles(server.config.CheckoutPath(), oldFiles, log.With().Str("package", "shaman/test").Logger(), nil) - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, expectOld, oldFiles) // Create some symlinks checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID") - assert.NoError(t, err) + require.NoError(t, err) err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) - assert.NoError(t, err) + require.NoError(t, err) err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob")) - assert.NoError(t, err) + require.NoError(t, err) // There should only be two old file reported now. expectRemovable := mtimeMap{ @@ -173,17 +174,17 @@ func TestGCComponents(t *testing.T) { stats := GCStats{} err = server.gcFilterLinkedFiles(server.config.CheckoutPath(), oldFiles, log.With().Str("package", "shaman/test").Logger(), &stats) assert.Equal(t, 1, stats.numSymlinksChecked) // 1 is in checkoutPath, the other in extraCheckoutDir - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, len(expectRemovable)+1, len(oldFiles)) // one file is linked from the extra checkout dir err = server.gcFilterLinkedFiles(extraCheckoutDir, oldFiles, log.With().Str("package", "shaman/test").Logger(), &stats) assert.Equal(t, 2, stats.numSymlinksChecked) // 1 is in checkoutPath, the other in extraCheckoutDir - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, expectRemovable, oldFiles) // Touching a file before requesting deletion should not delete it. now := time.Now() err = os.Chtimes(absPaths["6001.blob"], now, now) - assert.NoError(t, err) + require.NoError(t, err) // Running the garbage collector should only remove that one unused and untouched file. assert.FileExists(t, absPaths["6001.blob"], "file should exist before GC") @@ -228,13 +229,13 @@ func TestGarbageCollect(t *testing.T) { // Create some symlinks checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID") - assert.NoError(t, err) + require.NoError(t, err) err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) - assert.NoError(t, err) + require.NoError(t, err) err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob")) - assert.NoError(t, err) + require.NoError(t, err) // Running the garbage collector should only remove those two unused files. assert.FileExists(t, absPaths["6001.blob"], "file should exist before GC") diff --git a/pkg/shaman/fileserver/receivefile_test.go b/pkg/shaman/fileserver/receivefile_test.go index 384720c4..be51f11e 100644 --- a/pkg/shaman/fileserver/receivefile_test.go +++ b/pkg/shaman/fileserver/receivefile_test.go @@ -33,6 +33,7 @@ import ( "projects.blender.org/studio/flamenco/pkg/shaman/hasher" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "projects.blender.org/studio/flamenco/pkg/shaman/filestore" ) @@ -79,14 +80,14 @@ func TestStoreFile(t *testing.T) { // The correct checksum should be accepted. err = testWithChecksum(correctChecksum, filesize) - assert.NoError(t, err) + require.NoError(t, err) path, status = server.fileStore.ResolveFile(correctChecksum, filesize, filestore.ResolveEverything) assert.Equal(t, filestore.StatusStored, status) assert.FileExists(t, path) savedContent, err := ioutil.ReadFile(path) - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, payload, savedContent, "The file should be saved uncompressed") } diff --git a/pkg/shaman/filestore/filestore_test.go b/pkg/shaman/filestore/filestore_test.go index 57d407e7..d9ef274d 100644 --- a/pkg/shaman/filestore/filestore_test.go +++ b/pkg/shaman/filestore/filestore_test.go @@ -29,6 +29,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // mustCreateFile creates an empty file. @@ -105,17 +106,17 @@ func TestOpenForUpload(t *testing.T) { fileSize := int64(len(contents)) file, err := store.OpenForUpload("abcdefxxx", fileSize) - assert.NoError(t, err) + require.NoError(t, err) _, err = file.Write(contents) - assert.NoError(t, err) - assert.NoError(t, file.Close()) + require.NoError(t, err) + require.NoError(t, file.Close()) foundPath, status := store.ResolveFile("abcdefxxx", fileSize, ResolveEverything) assert.Equal(t, file.Name(), foundPath) assert.Equal(t, StatusUploading, status) readContents, err := ioutil.ReadFile(foundPath) - assert.NoError(t, err) + require.NoError(t, err) assert.EqualValues(t, contents, readContents) } @@ -130,14 +131,14 @@ func TestMoveToStored(t *testing.T) { assert.Error(t, err) file, err := store.OpenForUpload("abcdefxxx", fileSize) - assert.NoError(t, err) + require.NoError(t, err) _, err = file.Write(contents) - assert.NoError(t, err) - assert.NoError(t, file.Close()) + require.NoError(t, err) + require.NoError(t, file.Close()) tempLocation := file.Name() err = store.MoveToStored("abcdefxxx", fileSize, file.Name()) - assert.NoError(t, err, "moving file %s", file.Name()) + require.NoError(t, err, "moving file %s", file.Name()) foundPath, status := store.ResolveFile("abcdefxxx", fileSize, ResolveEverything) assert.NotEqual(t, file.Name(), foundPath) diff --git a/pkg/shaman/filestore/substore_test.go b/pkg/shaman/filestore/substore_test.go index ae188021..2d3822ce 100644 --- a/pkg/shaman/filestore/substore_test.go +++ b/pkg/shaman/filestore/substore_test.go @@ -29,6 +29,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestStoragePrefix(t *testing.T) { @@ -61,7 +62,7 @@ func TestFilePermissions(t *testing.T) { t.SkipNow() } dirname, err := os.MkdirTemp("", "file-permission-test") - assert.NoError(t, err) + require.NoError(t, err) defer os.RemoveAll(dirname) bin := storageBin{ @@ -71,11 +72,11 @@ func TestFilePermissions(t *testing.T) { } file, err := bin.openForWriting("testfilename.blend") - assert.NoError(t, err) + require.NoError(t, err) defer file.Close() filestat, err := file.Stat() - assert.NoError(t, err) + require.NoError(t, err) // The exact permissions depend on the current (unittest) process umask. This // umask is not easy to get, which is why we have a copy of `tempfile.go` in diff --git a/pkg/shaman/touch/touch_test.go b/pkg/shaman/touch/touch_test.go index 46eff968..75c41c59 100644 --- a/pkg/shaman/touch/touch_test.go +++ b/pkg/shaman/touch/touch_test.go @@ -29,6 +29,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTouch(t *testing.T) { @@ -46,7 +47,7 @@ func TestTouch(t *testing.T) { assert.Nil(t, Touch(testPath)) stat, err := os.Stat(testPath) - assert.NoError(t, err) + require.NoError(t, err) threshold := time.Now().Add(-5 * time.Second) assert.True(t, stat.ModTime().After(threshold),