diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f3c1b82..31df9687 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- Inline test case structs for consistency. +- Use the correct assertion format overload. +- Always use testing `Setenv` with automatic cleanup. +- Makefile target for unit testing all modules. +- Various assertion consistency improvements. +- Use context from `testing.T` introduced in Go 1.24. - Define more sentinel errors for more ergonomic error checking. - -### Tests - Use typed expectations consistently for added type safety. -### Style +### Fixed - Use walrus assignment where possible. - Use the `any` keyword where possible. diff --git a/Makefile b/Makefile index 222c8ef9..67fb668b 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,12 @@ lint/%: @echo "Running golangci-lint in $*/" @cd $* && golangci-lint run --build-tags=vfsintegration +.PHONY: test +test: $(addprefix test/,$(MODULES)) +test/%: + @echo "Running tests in $*/" + @cd $* && go test ./... + .PHONY: install-go-test-coverage install-go-test-coverage: go install github.com/vladopajic/go-test-coverage/v2@latest diff --git a/backend/azure/client_integration_test.go b/backend/azure/client_integration_test.go index e72ca384..52a3b08a 100644 --- a/backend/azure/client_integration_test.go +++ b/backend/azure/client_integration_test.go @@ -1,10 +1,8 @@ //go:build vfsintegration -// +build vfsintegration package azure import ( - "context" "fmt" "io" "os" @@ -38,23 +36,25 @@ func (s *ClientIntegrationTestSuite) SetupSuite() { s.Require().NoError(err) s.containerClient = cli - _, err = s.containerClient.Create(context.Background(), nil) + ctx := s.T().Context() + + _, err = s.containerClient.Create(ctx, nil) s.Require().NoError(err) // The create function claims to be synchronous but for some reason it does not exist for a little bit so // we need to wait for it to be there. - _, err = s.containerClient.GetProperties(context.Background(), nil) + _, err = s.containerClient.GetProperties(ctx, nil) for { time.Sleep(2 * time.Second) if err == nil || !bloberror.HasCode(err, bloberror.BlobNotFound) { break } - _, err = s.containerClient.GetProperties(context.Background(), nil) + _, err = s.containerClient.GetProperties(ctx, nil) } } func (s *ClientIntegrationTestSuite) TearDownSuite() { - _, err := s.containerClient.Delete(context.Background(), nil) + _, err := s.containerClient.Delete(s.T().Context(), nil) s.Require().NoError(err) } diff --git a/backend/azure/file.go b/backend/azure/file.go index 0faf9c78..5f2457e2 100644 --- a/backend/azure/file.go +++ b/backend/azure/file.go @@ -248,7 +248,7 @@ func (f *File) Delete(opts ...options.DeleteOption) error { var allVersions bool for _, o := range opts { switch o.(type) { - case delete.AllVersions, delete.DeleteAllVersions: + case delete.AllVersions, delete.DeleteAllVersions: //nolint:staticcheck // TODO: remove when delete.DeleteAllVersions is removed allVersions = true default: } diff --git a/backend/azure/fileSystem_test.go b/backend/azure/fileSystem_test.go index 2d243d32..ae2a3676 100644 --- a/backend/azure/fileSystem_test.go +++ b/backend/azure/fileSystem_test.go @@ -4,11 +4,11 @@ import ( "errors" "testing" - "github.com/c2fo/vfs/v7/utils" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v7" "github.com/c2fo/vfs/v7/backend/azure/mocks" + "github.com/c2fo/vfs/v7/utils" ) type FileSystemTestSuite struct { diff --git a/backend/azure/location_test.go b/backend/azure/location_test.go index 6f86544c..73b5899e 100644 --- a/backend/azure/location_test.go +++ b/backend/azure/location_test.go @@ -5,12 +5,12 @@ import ( "regexp" "testing" - "github.com/c2fo/vfs/v7/utils" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v7" "github.com/c2fo/vfs/v7/backend/azure/mocks" + "github.com/c2fo/vfs/v7/utils" ) type LocationTestSuite struct { diff --git a/backend/ftp/dataconn_test.go b/backend/ftp/dataconn_test.go index 5ff3da19..26858521 100644 --- a/backend/ftp/dataconn_test.go +++ b/backend/ftp/dataconn_test.go @@ -53,7 +53,7 @@ func (s *dataConnSuite) TestGetDataConn_alreadyExists() { mode: types.OpenRead, } dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -70,7 +70,7 @@ func (s *dataConnSuite) TestGetDataConn_openForRead() { Return(&_ftp.Response{}, nil). Once() dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -85,7 +85,7 @@ func (s *dataConnSuite) TestGetDataConn_errorClientSetup() { defaultClientGetter = clientGetterReturnsError s.ftpFile.Location().FileSystem().(*FileSystem).ftpclient = nil dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -108,7 +108,7 @@ func (s *dataConnSuite) TestGetDataConn_ReadError() { Return(nil, someErr). Once() dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -134,7 +134,7 @@ func (s *dataConnSuite) TestGetDataConn_WriteLocationNotExists() { Return(nil). Once() _, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -158,7 +158,7 @@ func (s *dataConnSuite) TestGetDataConn_WriteLocationNotExistsFails() { Return(someerr). Once() _, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -187,7 +187,7 @@ func (s *dataConnSuite) TestGetDataConn_errorWriting() { Return(someErr). Once() dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -215,7 +215,7 @@ func (s *dataConnSuite) TestGetDataConn_writeSuccess() { Return(nil). Once() dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -235,7 +235,7 @@ func (s *dataConnSuite) TestGetDataConn_readAfterWriteError() { fakedconn.AssertCloseErr(closeErr) s.ftpFile.Location().FileSystem().(*FileSystem).dataconn = fakedconn dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, @@ -265,7 +265,7 @@ func (s *dataConnSuite) TestGetDataConn_writeAfterReadSuccess() { Return(nil). Once() dc, err := getDataConn( - context.Background(), + s.T().Context(), authority.Authority{}, s.ftpFile.Location().FileSystem().(*FileSystem), s.ftpFile, diff --git a/backend/ftp/fileSystem_test.go b/backend/ftp/fileSystem_test.go index 5c84b196..89741e50 100644 --- a/backend/ftp/fileSystem_test.go +++ b/backend/ftp/fileSystem_test.go @@ -1,7 +1,6 @@ package ftp import ( - "context" "testing" "github.com/stretchr/testify/suite" @@ -128,7 +127,7 @@ func (ts *fileSystemTestSuite) TestWithOptions() { func (ts *fileSystemTestSuite) TestClient() { // client already set - client, err := ts.ftpfs.Client(context.Background(), authority.Authority{}) + client, err := ts.ftpfs.Client(ts.T().Context(), authority.Authority{}) ts.Require().NoError(err, "no error") ts.Equal(ts.ftpfs.ftpclient, client, "client was already set") } diff --git a/backend/ftp/options_test.go b/backend/ftp/options_test.go index 6b5d5de9..1aae035f 100644 --- a/backend/ftp/options_test.go +++ b/backend/ftp/options_test.go @@ -2,7 +2,6 @@ package ftp import ( "bytes" - "context" "crypto/tls" "os" "testing" @@ -68,8 +67,7 @@ func (s *optionsSuite) TestFetchUsername() { for _, test := range tests { s.Run(test.description, func() { if test.envVar != nil { - err := os.Setenv(envUsername, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envUsername, *test.envVar) } auth, err := authority.NewAuthority(test.authority) @@ -120,8 +118,7 @@ func (s *optionsSuite) TestFetchPassword() { for _, test := range tests { //nolint:gocritic //rangeValCopy but changing breaks ide integration for table-driven tests s.Run(test.description, func() { if test.envVar != nil { - err := os.Setenv(envPassword, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envPassword, *test.envVar) } password := fetchPassword(test.options) @@ -155,8 +152,7 @@ func (s *optionsSuite) TestFetchHostPortString() { s.Require().NoError(err, test.description) if test.envVar != nil { - err := os.Setenv(envPassword, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envPassword, *test.envVar) } hostPortString := fetchHostPortString(auth) @@ -238,8 +234,7 @@ func (s *optionsSuite) TestIsDisableEPSV() { for _, test := range tests { //nolint:gocritic //rangeValCopy but changing breaks ide integration for table-driven tests s.Run(test.description, func() { if test.envVar != nil { - err := os.Setenv(envDisableEPSV, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envDisableEPSV, *test.envVar) } disabled := isDisableOption(test.options) @@ -383,8 +378,7 @@ func (s *optionsSuite) TestFetchProtocol() { s.Run(test.description, func() { s.Require().NoError(os.Unsetenv(envProtocol)) if test.envVar != nil { - err := os.Setenv(envProtocol, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envProtocol, *test.envVar) } protocol := fetchProtocol(test.options) @@ -471,14 +465,13 @@ func (s *optionsSuite) TestFetchDialOptions() { for _, test := range tests { s.Run(test.description, func() { if test.envVar != nil { - err := os.Setenv(envProtocol, *test.envVar) - s.Require().NoError(err, test.description) + s.T().Setenv(envProtocol, *test.envVar) } auth, err := authority.NewAuthority(test.authority) s.Require().NoError(err, test.description) - dialOpts := fetchDialOptions(context.Background(), auth, test.options) + dialOpts := fetchDialOptions(s.T().Context(), auth, test.options) s.Len(dialOpts, test.expected, test.description) }) } diff --git a/backend/gs/file.go b/backend/gs/file.go index 61d6a99a..1d64196c 100644 --- a/backend/gs/file.go +++ b/backend/gs/file.go @@ -485,7 +485,7 @@ func (f *File) Delete(opts ...options.DeleteOption) error { var allVersions bool for _, o := range opts { switch o.(type) { - case delete.AllVersions, delete.DeleteAllVersions: + case delete.AllVersions, delete.DeleteAllVersions: //nolint:staticcheck // TODO: remove when delete.DeleteAllVersions is removed allVersions = true default: } diff --git a/backend/gs/file_test.go b/backend/gs/file_test.go index 23b6b894..60da6066 100644 --- a/backend/gs/file_test.go +++ b/backend/gs/file_test.go @@ -21,9 +21,8 @@ type fileTestSuite struct { suite.Suite } -func objectExists(bucket *storage.BucketHandle, objectName string) bool { +func objectExists(ctx context.Context, bucket *storage.BucketHandle, objectName string) bool { objectHandle := bucket.Object(objectName) - ctx := context.Background() _, err := objectHandle.Attrs(ctx) if err != nil { if errors.Is(err, storage.ErrObjectNotExist) { @@ -34,9 +33,8 @@ func objectExists(bucket *storage.BucketHandle, objectName string) bool { return true } -func mustReadObject(bucket *storage.BucketHandle, objectName string) []byte { +func mustReadObject(ctx context.Context, bucket *storage.BucketHandle, objectName string) []byte { objectHandle := bucket.Object(objectName) - ctx := context.Background() reader, err := objectHandle.NewReader(ctx) if err != nil { panic(err) @@ -140,7 +138,7 @@ func (ts *fileTestSuite) TestDelete() { ts.Require().NoError(err, "Shouldn't fail deleting the file") bucket := client.Bucket(bucketName) - ts.False(objectExists(bucket, objectName)) + ts.False(objectExists(ts.T().Context(), bucket, objectName)) } func (ts *fileTestSuite) TestDeleteError() { @@ -204,7 +202,7 @@ func (ts *fileTestSuite) TestDeleteRemoveAllVersions() { ts.Require().NoError(err, "Shouldn't fail deleting the file") bucket := client.Bucket(bucketName) - ts.False(objectExists(bucket, objectName)) + ts.False(objectExists(ts.T().Context(), bucket, objectName)) handles, err = f.getObjectGenerationHandles() ts.Require().NoError(err, "Shouldn't fail getting object generation handles") ts.Nil(handles) @@ -235,7 +233,7 @@ func (ts *fileTestSuite) TestWriteWithContentType() { defer server.Stop() client := server.Client() bucket := client.Bucket(bucketName) - ctx := context.Background() + ctx := ts.T().Context() err := bucket.Create(ctx, "", nil) ts.Require().NoError(err) fs := NewFileSystem(WithClient(client)) @@ -261,7 +259,7 @@ func (ts *fileTestSuite) TestTouchWithContentType() { defer server.Stop() client := server.Client() bucket := client.Bucket(bucketName) - ctx := context.Background() + ctx := ts.T().Context() err := bucket.Create(ctx, "", nil) ts.Require().NoError(err) fs := NewFileSystem(WithClient(client)) @@ -330,21 +328,16 @@ func (ts *fileTestSuite) TestNotExists() { } func (ts *fileTestSuite) TestMoveAndCopy() { - type TestCase struct { + testCases := make([]struct { move bool readFirst bool sameBucket bool - } - type TestCases []TestCase - - testCases := TestCases{} + }, 1<<3) - for idx := range 1 << 3 { - testCases = append(testCases, TestCase{ - move: (idx & (1 << 0)) != 0, - readFirst: (idx & (1 << 1)) != 0, - sameBucket: (idx & (1 << 2)) != 0, - }) + for idx := range testCases { + testCases[idx].move = (idx & (1 << 0)) != 0 + testCases[idx].readFirst = (idx & (1 << 1)) != 0 + testCases[idx].sameBucket = (idx & (1 << 2)) != 0 } for _, testCase := range testCases { @@ -386,12 +379,14 @@ func (ts *fileTestSuite) TestMoveAndCopy() { sourceBucket := client.Bucket(sourceBucketName) targetBucket := client.Bucket(targetBucketName) - ts.True(objectExists(sourceBucket, sourceName), "source should exist") + ctx := ts.T().Context() + + ts.True(objectExists(ctx, sourceBucket, sourceName), "source should exist") ts.True(fsFileNameExists(fs, sourceBucketName, sourceName), "source should exist") - ts.Equal(content, mustReadObject(sourceBucket, sourceName)) + ts.Equal(content, mustReadObject(ctx, sourceBucket, sourceName)) ts.Equal(content, fsMustReadFileName(fs, sourceBucketName, sourceName)) - ts.False(objectExists(targetBucket, targetName), "target should not exist") + ts.False(objectExists(ctx, targetBucket, targetName), "target should not exist") ts.False(fsFileNameExists(fs, sourceBucketName, targetName), "target should not exist") sourceFile, err := fs.NewFile(sourceBucketName, "/"+sourceName) @@ -416,18 +411,18 @@ func (ts *fileTestSuite) TestMoveAndCopy() { ts.Require().NoError(err, "Error shouldn't be returned from successful operation") if testCase.move { - ts.False(objectExists(sourceBucket, sourceName), "source should not exist") + ts.False(objectExists(ctx, sourceBucket, sourceName), "source should not exist") ts.False(fsFileNameExists(fs, sourceBucketName, sourceName), "source should not exist") } else { - ts.True(objectExists(sourceBucket, sourceName), "source should exist") + ts.True(objectExists(ctx, sourceBucket, sourceName), "source should exist") ts.True(fsFileNameExists(fs, sourceBucketName, sourceName), "source should exist") - ts.Equal(content, mustReadObject(sourceBucket, sourceName)) + ts.Equal(content, mustReadObject(ctx, sourceBucket, sourceName)) ts.Equal(content, fsMustReadFileName(fs, sourceBucketName, sourceName)) } - ts.True(objectExists(targetBucket, targetName), "target should exist") + ts.True(objectExists(ctx, targetBucket, targetName), "target should exist") ts.True(fsFileNameExists(fs, targetBucketName, targetName), "target should exist") - ts.Equal(content, mustReadObject(targetBucket, targetName)) + ts.Equal(content, mustReadObject(ctx, targetBucket, targetName)) ts.Equal(content, fsMustReadFileName(fs, targetBucketName, targetName)) } }) @@ -435,20 +430,16 @@ func (ts *fileTestSuite) TestMoveAndCopy() { } func (ts *fileTestSuite) TestMoveAndCopyBuffered() { - type TestCase struct { + testCases := make([]struct { move bool readFirst bool sameBucket bool - } - type TestCases []TestCase - testCases := TestCases{} - - for idx := range 1 << 3 { - testCases = append(testCases, TestCase{ - move: (idx & (1 << 0)) != 0, - readFirst: (idx & (1 << 1)) != 0, - sameBucket: (idx & (1 << 2)) != 0, - }) + }, 1<<3) + + for idx := range testCases { + testCases[idx].move = (idx & (1 << 0)) != 0 + testCases[idx].readFirst = (idx & (1 << 1)) != 0 + testCases[idx].sameBucket = (idx & (1 << 2)) != 0 } for _, testCase := range testCases { @@ -491,12 +482,14 @@ func (ts *fileTestSuite) TestMoveAndCopyBuffered() { sourceBucket := client.Bucket(sourceBucketName) targetBucket := client.Bucket(targetBucketName) - ts.True(objectExists(sourceBucket, sourceName), "source should exist") + ctx := ts.T().Context() + + ts.True(objectExists(ctx, sourceBucket, sourceName), "source should exist") ts.True(fsFileNameExists(fs, sourceBucketName, sourceName), "source should exist") - ts.Equal(content, mustReadObject(sourceBucket, sourceName)) + ts.Equal(content, mustReadObject(ctx, sourceBucket, sourceName)) ts.Equal(content, fsMustReadFileName(fs, sourceBucketName, sourceName)) - ts.False(objectExists(targetBucket, targetName), "target should not exist") + ts.False(objectExists(ctx, targetBucket, targetName), "target should not exist") ts.False(fsFileNameExists(fs, sourceBucketName, targetName), "target should not exist") sourceFile, err := fs.NewFile(sourceBucketName, "/"+sourceName) @@ -521,18 +514,18 @@ func (ts *fileTestSuite) TestMoveAndCopyBuffered() { ts.Require().NoError(err, "Error shouldn't be returned from successful operation") if testCase.move { - ts.False(objectExists(sourceBucket, sourceName), "source should not exist") + ts.False(objectExists(ctx, sourceBucket, sourceName), "source should not exist") ts.False(fsFileNameExists(fs, sourceBucketName, sourceName), "source should not exist") } else { - ts.True(objectExists(sourceBucket, sourceName), "source should exist") + ts.True(objectExists(ctx, sourceBucket, sourceName), "source should exist") ts.True(fsFileNameExists(fs, sourceBucketName, sourceName), "source should exist") - ts.Equal(content, mustReadObject(sourceBucket, sourceName)) + ts.Equal(content, mustReadObject(ctx, sourceBucket, sourceName)) ts.Equal(content, fsMustReadFileName(fs, sourceBucketName, sourceName)) } - ts.True(objectExists(targetBucket, targetName), "target should exist") + ts.True(objectExists(ctx, targetBucket, targetName), "target should exist") ts.True(fsFileNameExists(fs, targetBucketName, targetName), "target should exist") - ts.Equal(content, mustReadObject(targetBucket, targetName)) + ts.Equal(content, mustReadObject(ctx, targetBucket, targetName)) ts.Equal(content, fsMustReadFileName(fs, targetBucketName, targetName)) } }) diff --git a/backend/gs/filesystem_test.go b/backend/gs/filesystem_test.go index 3f071ec0..2c056ce9 100644 --- a/backend/gs/filesystem_test.go +++ b/backend/gs/filesystem_test.go @@ -6,9 +6,10 @@ import ( "testing" "cloud.google.com/go/storage" - "github.com/c2fo/vfs/v7/utils" "github.com/stretchr/testify/suite" "google.golang.org/api/option" + + "github.com/c2fo/vfs/v7/utils" ) type fileSystemSuite struct { @@ -222,7 +223,7 @@ func (s *fileSystemSuite) TestClient() { func (s *fileSystemSuite) TestWithContext() { fs := &FileSystem{} - ctx := context.Background() + ctx := s.T().Context() fs = fs.WithContext(ctx) s.Equal(ctx, fs.ctx) } diff --git a/backend/gs/location_test.go b/backend/gs/location_test.go index 810b17ae..5ba278cb 100644 --- a/backend/gs/location_test.go +++ b/backend/gs/location_test.go @@ -5,12 +5,11 @@ import ( "regexp" "testing" + "github.com/fsouza/fake-gcs-server/fakestorage" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v7/utils" "github.com/c2fo/vfs/v7/utils/authority" - - "github.com/fsouza/fake-gcs-server/fakestorage" ) type locationTestSuite struct { diff --git a/backend/mem/file_test.go b/backend/mem/file_test.go index aa2755bf..6a174b91 100644 --- a/backend/mem/file_test.go +++ b/backend/mem/file_test.go @@ -798,7 +798,7 @@ func (s *memFileTest) TestSeekBeyondEOF() { // Verify gap is filled with zeros gap := content[len(initialData):100] for i, b := range gap { - s.Zero(b, "gap should be filled with zeros at position %d", i) + s.Zerof(b, "gap should be filled with zeros at position %d", i) } // Verify additional data is at the correct position diff --git a/backend/mem/test_files/foo.txt b/backend/mem/test_files/foo.txt deleted file mode 100644 index c57eff55..00000000 --- a/backend/mem/test_files/foo.txt +++ /dev/null @@ -1 +0,0 @@ -Hello World! \ No newline at end of file diff --git a/backend/os/location_test.go b/backend/os/location_test.go index 23981982..b8f41d30 100644 --- a/backend/os/location_test.go +++ b/backend/os/location_test.go @@ -36,9 +36,7 @@ func (s *osLocationTest) SetupSuite() { func (s *osLocationTest) SetupTest() { file, err := s.tmploc.NewFile("test_files/test.txt") - if err != nil { - s.Fail("No file was opened") - } + s.Require().NoError(err, "No file was opened") s.testFile = file } diff --git a/backend/s3/file.go b/backend/s3/file.go index 0e6a1722..d8cdd0f3 100644 --- a/backend/s3/file.go +++ b/backend/s3/file.go @@ -210,7 +210,7 @@ func (f *File) Delete(opts ...options.DeleteOption) error { var allVersions bool for _, o := range opts { switch o.(type) { - case delete.AllVersions, delete.DeleteAllVersions: + case delete.AllVersions, delete.DeleteAllVersions: //nolint:staticcheck // TODO: remove when delete.DeleteAllVersions is removed allVersions = true default: } diff --git a/backend/s3/fileSystem_test.go b/backend/s3/fileSystem_test.go index 498f53d5..d0a08dde 100644 --- a/backend/s3/fileSystem_test.go +++ b/backend/s3/fileSystem_test.go @@ -1,7 +1,6 @@ package s3 import ( - "context" "testing" "github.com/aws/aws-sdk-go-v2/config" @@ -24,7 +23,7 @@ type mockClient struct { } func (ts *fileSystemTestSuite) SetupTest() { - cfg, err := config.LoadDefaultConfig(context.Background()) + cfg, err := config.LoadDefaultConfig(ts.T().Context()) if err != nil { panic(err) } diff --git a/backend/s3/file_test.go b/backend/s3/file_test.go index 39ecc0b1..6c392dc5 100644 --- a/backend/s3/file_test.go +++ b/backend/s3/file_test.go @@ -51,9 +51,6 @@ func (ts *fileTestSuite) SetupTest() { ts.Require().NoError(err, "Shouldn't return error creating test s3.File instance.") } -func (ts *fileTestSuite) TearDownTest() { -} - func (ts *fileTestSuite) TestRead() { contents := "hello world!" @@ -304,10 +301,9 @@ func (ts *fileTestSuite) TestMoveToFile() { } func (ts *fileTestSuite) TestGetCopyObject() { - type getCopyObjectTest struct { + tests := []struct { key, expectedCopySource string - } - tests := []getCopyObjectTest{ + }{ { key: "/path/to/nospace.txt", expectedCopySource: "%2Fpath%2Fto%2Fnospace.txt", @@ -710,6 +706,7 @@ func (ts *fileTestSuite) TestNewFile() { fs = &FileSystem{} // bucket is "" _, err = fs.NewFile("", "asdf") + ts.Require().ErrorIs(err, errAuthorityAndNameRequired) // key is "" _, err = fs.NewFile("asdf", "") @@ -759,15 +756,6 @@ func (ts *fileTestSuite) TestCloseWithWrite() { ts.Require().Error(err, "file doesn't exists, retired 5 times") } -type fileTestCase struct { - name string - setup func(*mocks.Client) *File // Function to set up each test case - actions []func(*File) error // Actions to perform on the file (Write, Seek, etc.) - wantErr bool - validate func(*File) error // Additional validations if needed - expectedContents string -} - func (ts *fileTestSuite) TestWriteOperations() { var contents *string setup := func(s3Mock *mocks.Client) { @@ -785,7 +773,14 @@ func (ts *fileTestSuite) TestWriteOperations() { auth, err := authority.NewAuthority("newBucket") ts.Require().NoError(err, "Shouldn't fail creating new authority") - testCases := []fileTestCase{ + testCases := []struct { + name string + setup func(*mocks.Client) *File // Function to set up each test case + actions []func(*File) error // Actions to perform on the file (Write, Seek, etc.) + wantErr bool + validate func(*File) error // Additional validations if needed + expectedContents string + }{ { name: "Write and Close - Close failure", setup: func(s3Mock *mocks.Client) *File { diff --git a/backend/s3/options_test.go b/backend/s3/options_test.go index 44f6c27e..f82d4a87 100644 --- a/backend/s3/options_test.go +++ b/backend/s3/options_test.go @@ -86,7 +86,7 @@ func (o *optionsTestSuite) TestGetClient() { for _, tt := range tests { //nolint:gocritic //rangeValCopy o.Run(tt.name, func() { for k, v := range tt.envVar { - _ = os.Setenv(k, v) + o.T().Setenv(k, v) } client, err := GetClient(tt.opts) tt.expected(o, client, err) diff --git a/backend/sftp/file_test.go b/backend/sftp/file_test.go index e72c70dd..fc3e1d41 100644 --- a/backend/sftp/file_test.go +++ b/backend/sftp/file_test.go @@ -119,16 +119,14 @@ func (ts *fileTestSuite) TestSeek() { ts.Require().NoError(closeErr, "no error expected") } -func (ts *fileTestSuite) Test_openFile() { - type testCase struct { +func (ts *fileTestSuite) TestOpenFile() { + tests := []struct { name string flags int setupMocks func(client *mocks.Client) expectedError bool expectedErrMsg string - } - - tests := []testCase{ + }{ { name: "Open file for read", flags: os.O_RDONLY, @@ -732,16 +730,15 @@ func (ts *fileTestSuite) TestMoveToLocation() { } func (ts *fileTestSuite) TestTouch() { - type testCase struct { + err := errors.New("some error") + testCases := []struct { name string filePath string fileExists bool setPermissions bool expectedError error setupMocks func(client *mocks.Client, sftpFile *mocks.ReadWriteSeekCloser, fileInfo *mocks.FileInfo) - } - err := errors.New("some error") - testCases := []testCase{ + }{ { name: "file exists", filePath: "/some/path.txt", @@ -928,15 +925,13 @@ func (ts *fileTestSuite) TestNewFile() { } func (ts *fileTestSuite) TestSetDefaultPermissions() { - type testCase struct { + tests := []struct { name string client *mocks.Client options Options expectedError bool expectedErrMsg string - } - - tests := []testCase{ + }{ { name: "No options provided", client: func() *mocks.Client { diff --git a/backend/sftp/options_test.go b/backend/sftp/options_test.go index 37611662..e0f34539 100644 --- a/backend/sftp/options_test.go +++ b/backend/sftp/options_test.go @@ -44,14 +44,6 @@ func (o *optionsSuite) SetupSuite() { o.keyFiles = *keyFiles } -type foundFileTest struct { - file string - expected bool - hasError bool - errMessage string - message string -} - func (o *optionsSuite) TestFoundFile() { // test file filename := filepath.Join(o.tmpdir, "some.key") @@ -62,7 +54,13 @@ func (o *optionsSuite) TestFoundFile() { o.Require().NoError(f.Close(), "closing file for foundfile test") defer func() { o.Require().NoError(os.Remove(filename), "clean up file for foundfile test") }() - tests := []foundFileTest{ + tests := []struct { + file string + expected bool + hasError bool + errMessage string + message string + }{ { file: filename, expected: true, @@ -92,17 +90,15 @@ func (o *optionsSuite) TestFoundFile() { } } -type getFileTest struct { - keyfile string - passphrase string - hasError bool - err error - errMessage string - message string -} - func (o *optionsSuite) TestGetKeyFile() { - tests := []getFileTest{ + tests := []struct { + keyfile string + passphrase string + hasError bool + err error + errMessage string + message string + }{ { keyfile: o.keyFiles.SSHPrivateKey, passphrase: o.keyFiles.passphrase, @@ -159,14 +155,6 @@ func (o *optionsSuite) TestGetKeyFile() { } } -type hostkeyTest struct { - options Options - envVars map[string]string - hasError bool - errMessage string - message string -} - func (o *optionsSuite) TestGetHostKeyCallback() { knownHosts := filepath.Join(o.tmpdir, "known_hosts") f, err := os.Create(knownHosts) //nolint:gosec @@ -176,7 +164,13 @@ func (o *optionsSuite) TestGetHostKeyCallback() { o.Require().NoError(f.Close(), "closing file for getHostKeyCallback test") defer func() { o.Require().NoError(os.Remove(knownHosts), "clean up file for getHostKeyCallback test") }() - tests := []hostkeyTest{ + tests := []struct { + options Options + envVars map[string]string + hasError bool + errMessage string + message string + }{ { options: Options{ KnownHostsCallback: ssh.FixedHostKey(o.publicKey), @@ -229,7 +223,7 @@ func (o *optionsSuite) TestGetHostKeyCallback() { errMessage: "", message: "Env fallthrough KnownHostsFile", }, - { // TODO: this may be a bad test if a user/system-wide known_hosts file isn't found + { // TODO: this may be a bad test if a user/system-wide known_hosts file isn't found hasError: false, errMessage: "", message: "default fallthrough KnownHostsFile", @@ -239,10 +233,8 @@ func (o *optionsSuite) TestGetHostKeyCallback() { for _, t := range tests { //nolint:gocritic // rangeValCopy o.Run(t.message, func() { // setup env vars, if any - tmpMap := make(map[string]string) for k, v := range t.envVars { - tmpMap[k] = os.Getenv(k) - o.Require().NoError(os.Setenv(k, v)) + o.T().Setenv(k, v) } // apply test @@ -252,27 +244,20 @@ func (o *optionsSuite) TestGetHostKeyCallback() { } else { o.Require().NoError(err, t.message) } - - // return env vars to original value - for k, v := range tmpMap { - o.Require().NoError(os.Setenv(k, v)) - } }) } } -type authTest struct { - options Options - envVars map[string]string - returnCount int - hasError bool - errMessage string - err error - message string -} - func (o *optionsSuite) TestGetAuthMethods() { - tests := []authTest{ + tests := []struct { + options Options + envVars map[string]string + returnCount int + hasError bool + errMessage string + err error + message string + }{ { options: Options{ Password: "somepassword", @@ -381,10 +366,8 @@ func (o *optionsSuite) TestGetAuthMethods() { for _, t := range tests { //nolint:gocritic // rangeValCopy o.Run(t.message, func() { // setup env vars, if any - tmpMap := make(map[string]string) for k, v := range t.envVars { - tmpMap[k] = os.Getenv(k) - o.Require().NoError(os.Setenv(k, v)) + o.T().Setenv(k, v) } // apply test @@ -399,24 +382,10 @@ func (o *optionsSuite) TestGetAuthMethods() { o.Require().NoError(err, t.message) o.Len(auth, t.returnCount, "auth count") } - - // return env vars to original value - for k, v := range tmpMap { - o.Require().NoError(os.Setenv(k, v)) - } }) } } -type getClientTest struct { - options Options - authority authority.Authority - hasError bool - err error - errRegex string - message string -} - func (o *optionsSuite) TestGetClient() { auth, err := authority.NewAuthority("someuser@badhost") o.Require().NoError(err) @@ -425,21 +394,16 @@ func (o *optionsSuite) TestGetClient() { o.Require().NoError(err) // Set environment variable for testing - origEnvUsername := os.Getenv("VFS_SFTP_USERNAME") - defer func() { - // Reset environment variable after test - if origEnvUsername != "" { - err := os.Setenv("VFS_SFTP_USERNAME", origEnvUsername) - o.Require().NoError(err, "restoring original VFS_SFTP_USERNAME env var") - } else { - err := os.Unsetenv("VFS_SFTP_USERNAME") - o.Require().NoError(err, "unsetting VFS_SFTP_USERNAME env var") - } - }() - err = os.Setenv("VFS_SFTP_USERNAME", "envuser") - o.Require().NoError(err) + o.T().Setenv("VFS_SFTP_USERNAME", "envuser") - tests := []getClientTest{ + tests := []struct { + options Options + authority authority.Authority + hasError bool + err error + errRegex string + message string + }{ { authority: auth, options: Options{ @@ -497,13 +461,12 @@ func (o *optionsSuite) TestGetClient() { o.Run(t.message, func() { _, _, err := GetClient(t.authority, t.options) if t.hasError { - if o.Error(err, "error found") { - if t.err != nil { - o.Require().ErrorIs(err, t.err, t.message) - } else { - re := regexp.MustCompile(t.errRegex) - o.Regexp(re, err.Error(), "error matches") - } + o.Require().Error(err) + if t.err != nil { + o.Require().ErrorIs(err, t.err, t.message) + } else { + re := regexp.MustCompile(t.errRegex) + o.Regexp(re, err.Error(), "error matches") } } else { o.Require().NoError(err, t.message) diff --git a/backend/testsuite/backend_integration_test.go b/backend/testsuite/backend_integration_test.go index b51ecf80..deac9d5c 100644 --- a/backend/testsuite/backend_integration_test.go +++ b/backend/testsuite/backend_integration_test.go @@ -3,7 +3,6 @@ package testsuite import ( - "context" "fmt" "io" "os" @@ -77,7 +76,7 @@ func (s *vfsTestSuite) SetupSuite() { } } -// Test File +// Test Scheme func (s *vfsTestSuite) TestScheme() { for scheme, location := range s.testLocations { fmt.Printf("************** TESTING scheme: %s **************\n", scheme) @@ -624,7 +623,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { // should now exist exists, err = copyFile1.Exists() s.Require().NoError(err) - s.True(exists, "%s should now exist locally", copyFile1) + s.Truef(exists, "%s should now exist locally", copyFile1) err = copyFile1.Close() s.Require().NoError(err) } else { @@ -739,10 +738,9 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { s.Require().NoError(err) // ensure that MoveToFile() works for files with spaces - type moveSpaceTest struct { + tests := []struct { Path, Filename string - } - tests := []moveSpaceTest{ + }{ {Path: "file/", Filename: "has space.txt"}, {Path: "file/", Filename: "has%20encodedSpace.txt"}, {Path: "path has/", Filename: "space.txt"}, @@ -804,17 +802,17 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { defer func() { _ = touchedFile.Delete() }() exists, err = touchedFile.Exists() s.Require().NoError(err) - s.False(exists, "%s shouldn't yet exist", touchedFile) + s.Falsef(exists, "%s shouldn't yet exist", touchedFile) err = touchedFile.Touch() s.Require().NoError(err) exists, err = touchedFile.Exists() s.Require().NoError(err) - s.True(exists, "%s now exists", touchedFile) + s.Truef(exists, "%s now exists", touchedFile) size, err := touchedFile.Size() s.Require().NoError(err) - s.Zero(size, "%s should be empty", touchedFile) + s.Zerof(size, "%s should be empty", touchedFile) // capture last modified modified, err := touchedFile.LastModified() @@ -827,7 +825,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { newModified, err := touchedFile.LastModified() s.Require().NoError(err) - s.Greater(*newModified, modifiedDeRef, "touch updated modified date for %s", touchedFile) + s.Greaterf(*newModified, modifiedDeRef, "touch updated modified date for %s", touchedFile) /* Delete unlinks the File on the file system. @@ -893,7 +891,7 @@ func (s *vfsTestSuite) gsList(baseLoc vfs.Location) { Bucket("enterprise-test"). Object(utils.RemoveLeadingSlash(baseLoc.Path() + "myfolder/")) - ctx := context.Background() + ctx := s.T().Context() // write zero length object writer := objHandle.NewWriter(ctx) diff --git a/backend/testsuite/io_integration_test.go b/backend/testsuite/io_integration_test.go index da9e2d30..17e5cf28 100644 --- a/backend/testsuite/io_integration_test.go +++ b/backend/testsuite/io_integration_test.go @@ -212,18 +212,16 @@ func (s *ioTestSuite) TestFileOperations() { } } -type TestCase struct { - description string - sequence string - fileAlreadyExists bool - expectFailure bool - expectedResults string -} - // unless seek or read is called first, writes should replace a file (not edit) func (s *ioTestSuite) testFileOperations(testPath string) { - testCases := []TestCase{ + testCases := []struct { + description string + sequence string + fileAlreadyExists bool + expectFailure bool + expectedResults string + }{ // Read, Close file { "Read, Close, file exists", diff --git a/contrib/vfsevents/CHANGELOG.md b/contrib/vfsevents/CHANGELOG.md index 53f0c611..5acc0f96 100644 --- a/contrib/vfsevents/CHANGELOG.md +++ b/contrib/vfsevents/CHANGELOG.md @@ -5,10 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] - -### Tests +### Fixed +- Use the correct assertion format overload. +- Use context from `testing.T` introduced in Go 1.24. - Use typed expectations consistently for added type safety. -### Style - Use walrus assignment where possible. ## [contrib/vfsevents/v1.1.1] - 2025-11-13 diff --git a/contrib/vfsevents/vfsevents_test.go b/contrib/vfsevents/vfsevents_test.go index b9367728..72fdde81 100644 --- a/contrib/vfsevents/vfsevents_test.go +++ b/contrib/vfsevents/vfsevents_test.go @@ -424,7 +424,7 @@ func (s *VfseventsTestSuite) TestCalculateBackoff() { for _, tt := range tests { s.Run(tt.name, func() { backoff := CalculateBackoff(tt.attempt, tt.config) - s.True(backoff >= tt.minTime && backoff <= tt.maxTime, + s.Truef(backoff >= tt.minTime && backoff <= tt.maxTime, "Expected backoff between %v and %v, got %v", tt.minTime, tt.maxTime, backoff) }) } diff --git a/contrib/vfsevents/watchers/fsnotify/fsnotify_test.go b/contrib/vfsevents/watchers/fsnotify/fsnotify_test.go index cf63e43e..952b6a2f 100644 --- a/contrib/vfsevents/watchers/fsnotify/fsnotify_test.go +++ b/contrib/vfsevents/watchers/fsnotify/fsnotify_test.go @@ -118,6 +118,8 @@ func (s *FSNotifyWatcherTestSuite) TestStartAndStop() { s.watcher, err = NewFSNotifyWatcher(location) s.Require().NoError(err) + ctx := s.T().Context() + s.Run("Valid start", func() { events := make(chan vfsevents.Event, 10) errors := make(chan error, 10) @@ -132,7 +134,7 @@ func (s *FSNotifyWatcherTestSuite) TestStartAndStop() { errors <- err } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(ctx) defer cancel() err := s.watcher.Start(ctx, eventHandler, errorHandler) @@ -176,8 +178,6 @@ func (s *FSNotifyWatcherTestSuite) TestStartAndStop() { errors <- err } - ctx := context.Background() - // Start first time err := s.watcher.Start(ctx, eventHandler, errorHandler) s.Require().NoError(err) @@ -213,7 +213,7 @@ func (s *FSNotifyWatcherTestSuite) TestFileOperations() { errors <- err } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(s.T().Context()) defer cancel() err = s.watcher.Start(ctx, eventHandler, errorHandler) @@ -367,7 +367,7 @@ func (s *FSNotifyWatcherTestSuite) TestRecursiveWatching() { errors <- err } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(s.T().Context()) defer cancel() err = s.watcher.Start(ctx, eventHandler, errorHandler) @@ -431,7 +431,7 @@ func (s *FSNotifyWatcherTestSuite) TestEventFiltering() { errors <- err } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(s.T().Context()) defer cancel() // Start with event filter - only .txt files @@ -500,7 +500,7 @@ eventLoop: txtEventReceived = true fmt.Printf("TEST DEBUG: Marking txtEventReceived = true\n") // Accept both Created and Modified events for .txt files - s.True(event.Type == vfsevents.EventCreated || event.Type == vfsevents.EventModified, + s.Truef(event.Type == vfsevents.EventCreated || event.Type == vfsevents.EventModified, "Expected Created or Modified event for .txt file, got: %s", event.Type) } else if s.isLogFileEvent(event.URI) { logEventReceived = true @@ -581,7 +581,7 @@ func (s *FSNotifyWatcherTestSuite) TestStatusCallback() { statuses <- status } - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(s.T().Context()) defer cancel() err = s.watcher.Start(ctx, eventHandler, errorHandler, diff --git a/contrib/vfsevents/watchers/gcsevents/gcsevents_test.go b/contrib/vfsevents/watchers/gcsevents/gcsevents_test.go index 56d1e6c6..b2781c2f 100644 --- a/contrib/vfsevents/watchers/gcsevents/gcsevents_test.go +++ b/contrib/vfsevents/watchers/gcsevents/gcsevents_test.go @@ -96,7 +96,6 @@ func (s *GCSWatcherTestSuite) TestStart() { for _, tt := range tests { s.Run(tt.name, func() { tt.setupMocks() - ctx := context.Background() handler := func(event vfsevents.Event) {} errHandler := func(err error) { if tt.wantErr { @@ -105,7 +104,7 @@ func (s *GCSWatcherTestSuite) TestStart() { s.Require().NoError(err) } } - err := s.watcher.Start(ctx, handler, errHandler) + err := s.watcher.Start(s.T().Context(), handler, errHandler) s.Require().NoError(err) s.Require().NoError(s.watcher.Stop()) }) @@ -113,6 +112,7 @@ func (s *GCSWatcherTestSuite) TestStart() { } func (s *GCSWatcherTestSuite) TestPoll() { + ctx := s.T().Context() tests := []struct { name string setupMocks func() @@ -127,9 +127,10 @@ func (s *GCSWatcherTestSuite) TestPoll() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectFinalize, @@ -151,9 +152,10 @@ func (s *GCSWatcherTestSuite) TestPoll() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectDelete, @@ -175,9 +177,10 @@ func (s *GCSWatcherTestSuite) TestPoll() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectMetadataUpdate, @@ -206,7 +209,7 @@ func (s *GCSWatcherTestSuite) TestPoll() { s.Run(tt.name, func() { tt.setupMocks() - _ = s.watcher.Start(context.TODO(), func(event vfsevents.Event) {}, func(err error) { + _ = s.watcher.Start(ctx, func(event vfsevents.Event) {}, func(err error) { if tt.wantErr { s.Require().Error(err) } else { @@ -219,6 +222,7 @@ func (s *GCSWatcherTestSuite) TestPoll() { } func (s *GCSWatcherTestSuite) TestReceiveWithRetry() { + ctx := s.T().Context() tests := []struct { name string retryConfig vfsevents.RetryConfig @@ -244,9 +248,10 @@ func (s *GCSWatcherTestSuite) TestReceiveWithRetry() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectFinalize, @@ -298,9 +303,10 @@ func (s *GCSWatcherTestSuite) TestReceiveWithRetry() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectFinalize, @@ -394,9 +400,10 @@ func (s *GCSWatcherTestSuite) TestReceiveWithRetry() { TimeCreated: JSONTime(time.Now()), } body, _ := json.Marshal(event) + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: map[string]string{ "eventType": EventObjectFinalize, @@ -434,7 +441,7 @@ func (s *GCSWatcherTestSuite) TestReceiveWithRetry() { status := &vfsevents.WatcherStatus{} // Create context with timeout for cancellation test - ctx := context.Background() + ctx := s.T().Context() if tt.name == "Context cancellation during retry" { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, 50*time.Millisecond) @@ -509,10 +516,12 @@ func (s *GCSWatcherTestSuite) TestRetryBackoffTiming() { "overwrittenByGeneration": "1111111111", } + ctx := s.T().Context() + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { // Send OBJECT_FINALIZE event (should generate EventModified) - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: attributes, }) @@ -520,7 +529,7 @@ func (s *GCSWatcherTestSuite) TestRetryBackoffTiming() { Return(nil). Once() - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) defer cancel() config := &vfsevents.StartConfig{} @@ -685,9 +694,11 @@ func (s *GCSWatcherTestSuite) TestEnhancedMetadata() { s.Require().NoError(err) } + ctx := s.T().Context() + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: attributes, }) @@ -695,7 +706,7 @@ func (s *GCSWatcherTestSuite) TestEnhancedMetadata() { Return(nil). Once() - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) defer cancel() config := &vfsevents.StartConfig{} @@ -756,16 +767,18 @@ func (s *GCSWatcherTestSuite) TestOverwriteEventSuppression() { "eventTime": "2023-01-01T12:00:01Z", } + ctx := s.T().Context() + s.pubsubClient.EXPECT().Receive(mock.Anything, mock.Anything). Run(func(_ context.Context, handler func(context.Context, *pubsub.Message)) { // Send OBJECT_FINALIZE event (should generate EventModified) - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: finalizeAttributes, }) // Send OBJECT_DELETE event (should be suppressed) - handler(context.TODO(), &pubsub.Message{ + handler(ctx, &pubsub.Message{ Data: body, Attributes: deleteAttributes, }) @@ -773,7 +786,7 @@ func (s *GCSWatcherTestSuite) TestOverwriteEventSuppression() { Return(nil). Once() - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) defer cancel() config := &vfsevents.StartConfig{} diff --git a/contrib/vfsevents/watchers/s3events/s3events_test.go b/contrib/vfsevents/watchers/s3events/s3events_test.go index ce196b7a..54456cf3 100644 --- a/contrib/vfsevents/watchers/s3events/s3events_test.go +++ b/contrib/vfsevents/watchers/s3events/s3events_test.go @@ -81,7 +81,6 @@ func (s *S3WatcherTestSuite) TestStart() { for _, tt := range tests { s.Run(tt.name, func() { - ctx := context.Background() handler := func(event vfsevents.Event) {} errHandler := func(err error) { if tt.wantErr { @@ -90,7 +89,7 @@ func (s *S3WatcherTestSuite) TestStart() { s.Require().NoError(err) } } - err := s.watcher.Start(ctx, handler, errHandler) + err := s.watcher.Start(s.T().Context(), handler, errHandler) s.Require().NoError(err) s.Require().NoError(s.watcher.Stop()) }) @@ -299,7 +298,7 @@ func (s *S3WatcherTestSuite) TestPoll() { } config := &vfsevents.StartConfig{} - err := s.watcher.pollOnce(context.TODO(), func(event vfsevents.Event) {}, status, config) + err := s.watcher.pollOnce(s.T().Context(), func(event vfsevents.Event) {}, status, config) if tt.wantErr { s.Require().Error(err) } else { @@ -432,7 +431,7 @@ func (s *S3WatcherTestSuite) TestPollWithRetry() { status := &vfsevents.WatcherStatus{} // Create context with timeout to control polling duration - ctx, cancel := context.WithTimeout(context.Background(), tt.contextTimeout) + ctx, cancel := context.WithTimeout(s.T().Context(), tt.contextTimeout) defer cancel() // Track errors via error handler @@ -581,14 +580,14 @@ func (s *S3WatcherTestSuite) TestWithReceivedCount() { config := &vfsevents.StartConfig{} status := &vfsevents.WatcherStatus{} - err = watcher.pollOnce(context.Background(), handler, status, config) + err = watcher.pollOnce(s.T().Context(), handler, status, config) if tt.expectError { s.Require().Error(err) } else { s.Require().NoError(err) } - s.Equal(tt.expectMessageProcessed, handlerCalled, + s.Equalf(tt.expectMessageProcessed, handlerCalled, "Handler call expectation mismatch for receive count %s with threshold %d", tt.messageReceiveCount, tt.receivedCount) }) @@ -644,7 +643,7 @@ func (s *S3WatcherTestSuite) TestWithReceivedCountNoAttributes() { config := &vfsevents.StartConfig{} status := &vfsevents.WatcherStatus{} - err = watcher.pollOnce(context.Background(), handler, status, config) + err = watcher.pollOnce(s.T().Context(), handler, status, config) s.Require().NoError(err) s.True(handlerCalled, "Handler should be called when no attributes are present") } @@ -701,7 +700,7 @@ func (s *S3WatcherTestSuite) TestWithReceivedCountMissingAttribute() { config := &vfsevents.StartConfig{} status := &vfsevents.WatcherStatus{} - err = watcher.pollOnce(context.Background(), handler, status, config) + err = watcher.pollOnce(s.T().Context(), handler, status, config) s.Require().NoError(err) s.True(handlerCalled, "Handler should be called when ApproximateReceiveCount attribute is missing") } @@ -924,7 +923,7 @@ func (s *S3WatcherTestSuite) TestEnhancedMetadata() { Once() // Use longer timeout for Windows compatibility - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(s.T().Context(), 5*time.Second) defer cancel() config := &vfsevents.StartConfig{} @@ -1007,7 +1006,7 @@ func (s *S3WatcherTestSuite) TestNonVersionedBucketMetadata() { Return(&sqs.DeleteMessageOutput{}, nil). Once() - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + ctx, cancel := context.WithTimeout(s.T().Context(), 100*time.Millisecond) defer cancel() config := &vfsevents.StartConfig{} diff --git a/contrib/vfsevents/watchers/vfspoller/vfspoller_test.go b/contrib/vfsevents/watchers/vfspoller/vfspoller_test.go index b114aa6c..c82416b8 100644 --- a/contrib/vfsevents/watchers/vfspoller/vfspoller_test.go +++ b/contrib/vfsevents/watchers/vfspoller/vfspoller_test.go @@ -119,11 +119,10 @@ func (s *PollerTestSuite) TestStart() { location := mocks.NewLocation(s.T()) location.EXPECT().Exists().Return(true, nil) poller, _ := NewPoller(location) - ctx := context.Background() errFunc := func(err error) { s.Require().NoError(err) } - err := poller.Start(ctx, tt.handler, errFunc) + err := poller.Start(s.T().Context(), tt.handler, errFunc) if tt.wantErr { s.Require().Error(err) } else { @@ -135,7 +134,6 @@ func (s *PollerTestSuite) TestStart() { } func (s *PollerTestSuite) TestStop() { - ctx := context.Background() handler := func(event vfsevents.Event) {} location := mocks.NewLocation(s.T()) location.EXPECT().Exists().Return(true, nil) @@ -143,7 +141,7 @@ func (s *PollerTestSuite) TestStop() { errFunc := func(err error) { s.Require().NoError(err) } - err := poller.Start(ctx, handler, errFunc) + err := poller.Start(s.T().Context(), handler, errFunc) s.Require().NoError(err) s.Require().NoError(poller.Stop()) // Ensure that the polling process stops correctly @@ -490,12 +488,12 @@ func (s *PollerTestSuite) TestPerformCleanup() { // Verify remaining entries for _, uri := range tt.expectedRemain { - s.Contains(poller.fileCache, uri, "Expected URI %s to remain in cache", uri) + s.Containsf(poller.fileCache, uri, "Expected URI %s to remain in cache", uri) } // Verify removed entries for _, uri := range tt.expectedRemove { - s.NotContains(poller.fileCache, uri, "Expected URI %s to be removed from cache", uri) + s.NotContainsf(poller.fileCache, uri, "Expected URI %s to be removed from cache", uri) } // Verify lastCleanup was updated @@ -611,13 +609,13 @@ func (s *PollerTestSuite) TestEnforceMaxFiles() { // Verify the newest files remain newestEntries := entries[len(entries)-tt.maxFiles:] for _, entry := range newestEntries { - s.Contains(poller.fileCache, entry.uri, "Expected newest file %s to remain", entry.uri) + s.Containsf(poller.fileCache, entry.uri, "Expected newest file %s to remain", entry.uri) } // Verify oldest files were removed oldestEntries := entries[:len(entries)-tt.maxFiles] for _, entry := range oldestEntries { - s.NotContains(poller.fileCache, entry.uri, "Expected oldest file %s to be removed", entry.uri) + s.NotContainsf(poller.fileCache, entry.uri, "Expected oldest file %s to be removed", entry.uri) } } }) diff --git a/errors.go b/errors.go index a9dd956b..a9d694ac 100644 --- a/errors.go +++ b/errors.go @@ -11,6 +11,7 @@ const ( ErrCopyToNotPossible = Error("current cursor offset is not 0 as required for this operation") // CopyToNotPossible - CopyTo/MoveTo operations are only possible when seek position is 0,0 + // // Deprecated: Use ErrCopyToNotPossible instead CopyToNotPossible = ErrCopyToNotPossible //nolint:errname diff --git a/options/delete/allVersions.go b/options/delete/allVersions.go index 98c798ce..d4195686 100644 --- a/options/delete/allVersions.go +++ b/options/delete/allVersions.go @@ -20,6 +20,7 @@ func (w AllVersions) DeleteOptionName() string { } // WithDeleteAllVersions returns DeleteAllVersions implementation of options.DeleteOption +// // Deprecated: use WithAllVersions instead func WithDeleteAllVersions() options.DeleteOption { return DeleteAllVersions{} @@ -27,6 +28,7 @@ func WithDeleteAllVersions() options.DeleteOption { // DeleteAllVersions represents the DeleteOption that is used to remove all versions of files when deleted. // This will remove all versions of files for the filesystems that support file versioning. +// // Deprecated: use AllVersions instead type DeleteAllVersions struct{} diff --git a/options/delete/allVersions_test.go b/options/delete/allVersions_test.go index 188b655b..9568eb72 100644 --- a/options/delete/allVersions_test.go +++ b/options/delete/allVersions_test.go @@ -2,32 +2,26 @@ package delete import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestWithAllVersions(t *testing.T) { opt := WithAllVersions() - if opt.DeleteOptionName() != "deleteAllVersions" { - t.Errorf("expected `deleteAllVersions`, got %s", opt.DeleteOptionName()) - } + assert.Equal(t, optionNameDeleteAllVersions, opt.DeleteOptionName()) } func TestAllVersionsName(t *testing.T) { var opt AllVersions - if opt.DeleteOptionName() != "deleteAllVersions" { - t.Errorf("expected `deleteAllVersions`, got %s", opt.DeleteOptionName()) - } + assert.Equal(t, optionNameDeleteAllVersions, opt.DeleteOptionName()) } func TestWithDeleteAllVersions(t *testing.T) { opt := WithDeleteAllVersions() - if opt.DeleteOptionName() != "deleteAllVersions" { - t.Errorf("expected `deleteAllVersions`, got %s", opt.DeleteOptionName()) - } + assert.Equal(t, optionNameDeleteAllVersions, opt.DeleteOptionName()) } func TestDeleteAllVersionsName(t *testing.T) { var opt DeleteAllVersions - if opt.DeleteOptionName() != "deleteAllVersions" { - t.Errorf("expected `deleteAllVersions`, got %s", opt.DeleteOptionName()) - } + assert.Equal(t, optionNameDeleteAllVersions, opt.DeleteOptionName()) } diff --git a/options/newfile/contentType_test.go b/options/newfile/contentType_test.go index 27a5812d..c9cdd9c0 100644 --- a/options/newfile/contentType_test.go +++ b/options/newfile/contentType_test.go @@ -3,6 +3,9 @@ package newfile_test import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/c2fo/vfs/v7/options/newfile" ) @@ -10,13 +13,7 @@ func TestWithContentType(t *testing.T) { opt := newfile.WithContentType("application/json") ct, ok := opt.(*newfile.ContentType) - if !ok { - t.Fatalf("expected `*newfile.ContentType`, got %T", opt) - } - if *ct != "application/json" { - t.Errorf("expected `application/json`, got %v", *ct) - } - if ct.NewFileOptionName() != "newFileContentType" { - t.Errorf("expected `newFileContentType`, got %v", ct.NewFileOptionName()) - } + require.Truef(t, ok, "expected `*newfile.ContentType`, got %T", opt) + assert.Equal(t, newfile.ContentType("application/json"), *ct) + assert.Equal(t, "newFileContentType", ct.NewFileOptionName()) } diff --git a/utils/authority/authority_test.go b/utils/authority/authority_test.go index 5b0098ab..520ee781 100644 --- a/utils/authority/authority_test.go +++ b/utils/authority/authority_test.go @@ -14,17 +14,15 @@ type authoritySuite struct { suite.Suite } -type authorityTest struct { - authorityString string - host, user, pass, str, hostPortStr string - port uint16 - hasError bool - errMessage string - message string -} - func (a *authoritySuite) TestAuthority() { - tests := []authorityTest{ + tests := []struct { + authorityString string + host, user, pass, str, hostPortStr string + port uint16 + hasError bool + errMessage string + message string + }{ { authorityString: "some.host.com", host: "some.host.com", @@ -296,14 +294,12 @@ func (a *authoritySuite) TestAuthority() { } } -type encodeAuthorityTest struct { - rawAuthority string - expectedEncoded string - message string -} - func (a *authoritySuite) TestEncodeAuthority() { - tests := []encodeAuthorityTest{ + tests := []struct { + rawAuthority string + expectedEncoded string + message string + }{ { rawAuthority: "user@someserver.com:22", expectedEncoded: "user@someserver.com:22", diff --git a/utils/utils.go b/utils/utils.go index d266782c..f229e20e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -168,6 +168,7 @@ func PathToURI(p string) (string, error) { // TouchCopy is a wrapper around io.Copy which ensures that even empty source files (reader) will get written as an // empty file. It guarantees a Write() call on the target file. +// // Deprecated: Use TouchCopyBuffer Instead func TouchCopy(writer io.Writer, reader io.Reader) error { size, err := io.Copy(writer, reader) diff --git a/utils/utils_test.go b/utils/utils_test.go index 0ea8384b..01039b0e 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -529,15 +529,13 @@ func (s *utilsSuite) TestValidatePrefix() { } } -type URITest struct { - path string - expected string - message string - isRegex bool -} - func (s *utilsSuite) TestPathToURI() { - tests := []URITest{ + tests := []struct { + path string + expected string + message string + isRegex bool + }{ { path: "/absolute/path/", expected: "file:///absolute/path/", diff --git a/vfssimple/vfssimple_test.go b/vfssimple/vfssimple_test.go index f329f3ab..7537cfb5 100644 --- a/vfssimple/vfssimple_test.go +++ b/vfssimple/vfssimple_test.go @@ -291,11 +291,9 @@ func (s *vfsSimpleSuite) TestParseSupportedURI() { case "s3": s3cli, err := fs.(*s3.FileSystem).Client() s.Require().NoError(err, test.message) - if c, ok := s3cli.(*namedS3ClientMock); ok { - s.Equal(c.RegName, test.regFS, test.message) - } else { - s.Fail("should have returned mock", test.message) - } + c, ok := s3cli.(*namedS3ClientMock) + s.Require().True(ok, "should have returned mock", test.message) + s.Equal(test.regFS, c.RegName, test.message) default: s.Fail("we should have a case for returned fs type", test.message) } @@ -316,11 +314,9 @@ func (s *vfsSimpleSuite) TestNewFile() { s.Equal(file.URI(), goodURI) s3cli, err := file.Location().FileSystem().(*s3.FileSystem).Client() s.Require().NoError(err, "no error expected") - if c, ok := s3cli.(*namedS3ClientMock); ok { - s.Equal("filetest-path", c.RegName, "should be 'filetest-path', not 'filetest-bucket' or 's3'") - } else { - s.Fail("should have returned mock", "should not reach this") - } + c, ok := s3cli.(*namedS3ClientMock) + s.Require().True(ok, "should have returned mock") + s.Equal("filetest-path", c.RegName, "should be 'filetest-path', not 'filetest-bucket' or 's3'") // failure badURI := "unknown://filetest/path/file.txt" @@ -348,11 +344,9 @@ func (s *vfsSimpleSuite) TestNewLocation() { s.Equal(loc.URI(), goodURI) s3cli, err := loc.FileSystem().(*s3.FileSystem).Client() s.Require().NoError(err, "no error expected") - if c, ok := s3cli.(*namedS3ClientMock); ok { - s.Equal("loctest-path", c.RegName, "should be 'loctest-path', not 'loctest-bucket' or 's3'") - } else { - s.Fail("should have returned mock", "should not reach this") - } + c, ok := s3cli.(*namedS3ClientMock) + s.Require().True(ok, "should have returned mock") + s.Equal("loctest-path", c.RegName, "should be 'loctest-path', not 'loctest-bucket' or 's3'") // failure badURI := "unknown://filetest/path/to/here/" @@ -376,6 +370,7 @@ type namedS3ClientMock struct { // getS3NamedClientMock returns an s3 client that satisfies the interface but we only really care about the name, // to introspect in the test return func getS3NamedClientMock(t *testing.T, name string) *namedS3ClientMock { + t.Helper() return &namedS3ClientMock{ Client: mocks.NewClient(t), RegName: name,