diff --git a/CHANGELOG.md b/CHANGELOG.md index fd2319b3..456fef5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- Unit Test improvements: report underlying unit tests errors, always run test cases in a sub-test, always use test suite functions, use more specific assert functions where possible. - General spelling and grammar corrections everywhere. ## [6.20.0] - 2024-10-15 diff --git a/backend/azure/client_integration_test.go b/backend/azure/client_integration_test.go index e3f40916..8e4a628b 100644 --- a/backend/azure/client_integration_test.go +++ b/backend/azure/client_integration_test.go @@ -162,7 +162,7 @@ func (s *ClientIntegrationTestSuite) TestDeleteAllVersions() { // make sure the file doesn't exist exists, err := f.Exists() s.NoError(err, "no error should be returned on exists check") - s.Equal(false, exists) + s.False(exists) } func (s *ClientIntegrationTestSuite) TestProperties() { diff --git a/backend/azure/fileSystem_test.go b/backend/azure/fileSystem_test.go index 5f392f38..5a337a86 100644 --- a/backend/azure/fileSystem_test.go +++ b/backend/azure/fileSystem_test.go @@ -22,26 +22,23 @@ func (s *FileSystemTestSuite) TestVFSFileSystemImplementor() { func (s *FileSystemTestSuite) TestNewFile() { fs := NewFileSystem().WithOptions(Options{AccountName: "test-container"}) file, err := fs.NewFile("", "") - s.Error(err, "volume and path are required") + s.EqualError(err, "non-empty strings for container and path are required", "volume and path are required") s.Nil(file) - s.Equal("non-empty strings for container and path are required", err.Error()) fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) file, err = fs.NewFile("temp", "") - s.Error(err, "volume and path are required") + s.EqualError(err, "non-empty strings for container and path are required", "volume and path are required") s.Nil(file) - s.Equal("non-empty strings for container and path are required", err.Error()) fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) file, err = fs.NewFile("", "/blah/blah.txt") - s.Error(err, "volume and path are required") + s.EqualError(err, "non-empty strings for container and path are required", "volume and path are required") s.Nil(file) - s.Equal("non-empty strings for container and path are required", err.Error()) fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) file, err = fs.NewFile("temp", "blah/blah.txt") - s.Error(err, "the path is invalid so we expect an error") - s.Equal("absolute file path is invalid - must include leading slash and may not include trailing slash", err.Error()) + s.EqualError(err, "absolute file path is invalid - must include leading slash and may not include trailing slash", + "the path is invalid so we expect an error") s.Nil(file, "Since an error was returned we expect a nil file to be returned") fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) @@ -54,8 +51,7 @@ func (s *FileSystemTestSuite) TestNewFile() { func (s *FileSystemTestSuite) TestNewFile_NilReceiver() { var fs *FileSystem file, err := fs.NewFile("temp", "/foo/bar/test.txt") - s.Error(err, "the receiver pointer is nil so we would receive an error") - s.Equal("azure.FileSystem receiver pointer must be non-nil", err.Error()) + s.EqualError(err, "azure.FileSystem receiver pointer must be non-nil", "the receiver pointer is nil so we would receive an error") s.Nil(file, "Since there was an error we expect a nil file to be returned") } @@ -77,14 +73,14 @@ func (s *FileSystemTestSuite) TestNewLocation() { fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) loc, err = fs.NewLocation("temp", "foo/bar/") - s.Error(err, "The path does not start with a slash and therefore not an absolute path so we expect an error") - s.Equal("absolute location path is invalid - must include leading and trailing slashes", err.Error()) + s.EqualError(err, "absolute location path is invalid - must include leading and trailing slashes", + "The path does not start with a slash and therefore not an absolute path so we expect an error") s.Nil(loc, "Since an error was returned the location is nil") fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) loc, err = fs.NewLocation("temp", "/foo/bar") - s.Error(err, "The path does not end with a slash and therefore not an absolute path so we expect an error") - s.Equal("absolute location path is invalid - must include leading and trailing slashes", err.Error()) + s.EqualError(err, "absolute location path is invalid - must include leading and trailing slashes", + "The path does not end with a slash and therefore not an absolute path so we expect an error") s.Nil(loc, "Since an error was returned the location is nil") fs = NewFileSystem().WithOptions(Options{AccountName: "test-container"}) @@ -109,8 +105,8 @@ func (s *FileSystemTestSuite) TestNewLocation() { func (s *FileSystemTestSuite) TestNewLocation_NilReceiver() { var fs *FileSystem loc, err := fs.NewLocation("temp", "/foo/bar/") - s.Error(err, "The receiver pointer on the function call is nill so we should get an error") - s.Equal("azure.FileSystem receiver pointer must be non-nil", err.Error()) + s.EqualError(err, "azure.FileSystem receiver pointer must be non-nil", + "The receiver pointer on the function call is nill so we should get an error") s.Nil(loc, "The call returned an error so the location should be nil") } @@ -142,8 +138,7 @@ func (s *FileSystemTestSuite) TestRetry() { fs = NewFileSystem().WithOptions(Options{RetryFunc: errorRetry}) retryFn = fs.Retry() err = retryFn(doNothing) - s.Error(err, "This implementation should use the retry function from the options which always errors") - s.Equal("i always error", err.Error()) + s.EqualError(err, "i always error", "This implementation should use the retry function from the options which always errors") } func (s *FileSystemTestSuite) TestNewFileSystem() { @@ -157,7 +152,7 @@ func (s *FileSystemTestSuite) TestWithOptions() { s.Equal("foo-account", fs.options.AccountName) fs = NewFileSystem().WithOptions("Not Azure Options...") - s.Equal("", fs.options.AccountName) + s.Empty(fs.options.AccountName) } func (s *FileSystemTestSuite) TestClient() { @@ -187,8 +182,8 @@ func (s *FileSystemTestSuite) TestParsePath() { u, _ = url.Parse(uri) volume, path, err = ParsePath(u.Path) s.Error(err, "a container is required so we should get an error") - s.Equal("", volume, "we got an error so volume should be empty") - s.Equal("", path, "we got an error so path should be empty") + s.Empty(volume, "we got an error so volume should be empty") + s.Empty(path, "we got an error so path should be empty") uri = "https://my-account.blob.core.windows.net/my_container/foo/bar/baz.txt" u, _ = url.Parse(uri) diff --git a/backend/azure/file_test.go b/backend/azure/file_test.go index 9d7d836a..76db4f70 100644 --- a/backend/azure/file_test.go +++ b/backend/azure/file_test.go @@ -249,7 +249,7 @@ func (s *FileTestSuite) TestSize_NonExistentFile() { s.NoError(err, "The path is valid so no error should be returned") size, err := f.Size() s.Error(err, "If the file does not exist we get an error") - s.Equal(uint64(0), size, "the file does not exist so the size is 0") + s.Zero(size, "the file does not exist so the size is 0") } func (s *FileTestSuite) TestPath() { @@ -336,7 +336,7 @@ func (s *FileTestSuite) TestCheckTempFile_FileDoesNotExist() { contents, err := io.ReadAll(azureFile.tempFile) s.NoError(err, "No error should occur while reading the tempFile") - s.Equal("", string(contents)) + s.Empty(contents) } func (s *FileTestSuite) TestCheckTempFile_DownloadError() { diff --git a/backend/azure/location_test.go b/backend/azure/location_test.go index 97e8709c..314e694c 100644 --- a/backend/azure/location_test.go +++ b/backend/azure/location_test.go @@ -142,8 +142,7 @@ func (s *LocationTestSuite) TestNewLocation() { func (s *LocationTestSuite) TestNewLocation_NilReceiver() { var l *Location nl, err := l.NewLocation("test-container/") - s.Error(err, "The receiver for NewLocation must be non-nil so we expect an error") - s.Equal("azure.Location receiver pointer must be non-nil", err.Error()) + s.EqualError(err, "azure.Location receiver pointer must be non-nil", "The receiver for NewLocation must be non-nil so we expect an error") s.Nil(nl, "An error was returned so we expect a nil location to be returned") } @@ -159,26 +158,25 @@ func (s *LocationTestSuite) TestChangeDir() { l = Location{} err = l.ChangeDir("/test-container/") - s.Error(err, "The path begins with a slash and therefore is not a relative path so this should return an error") - s.Equal("relative location path is invalid - may not include leading slash but must include trailing slash", err.Error()) + s.EqualError(err, "relative location path is invalid - may not include leading slash but must include trailing slash", + "The path begins with a slash and therefore is not a relative path so this should return an error") l = Location{} err = l.ChangeDir("test-container") - s.Error(err, "The path does not end with a slash and therefore is not a relative path so this should return an error") - s.Equal("relative location path is invalid - may not include leading slash but must include trailing slash", err.Error()) + s.EqualError(err, "relative location path is invalid - may not include leading slash but must include trailing slash", + "The path does not end with a slash and therefore is not a relative path so this should return an error") l = Location{} err = l.ChangeDir("") - s.Error(err, "An empty relative path does not end with a slash and therefore is not a valid relative path so this should return an error") - s.Equal("relative location path is invalid - may not include leading slash but must include trailing slash", err.Error()) + s.EqualError(err, "relative location path is invalid - may not include leading slash but must include trailing slash", + "An empty relative path does not end with a slash and therefore is not a valid relative path so this should return an error") } func (s *LocationTestSuite) TestChangeDir_NilReceiver() { var l *Location s.Nil(l) err := l.ChangeDir("") - s.Error(err) - s.Equal("azure.Location receiver pointer must be non-nil", err.Error()) + s.EqualError(err, "azure.Location receiver pointer must be non-nil") } func (s *LocationTestSuite) TestFileSystem() { @@ -192,18 +190,18 @@ func (s *LocationTestSuite) TestNewFile() { l, _ := fs.NewLocation("test-container", "/folder/") f, err := l.NewFile("") - s.Error(err, "Empty string is not a valid relative file path so we expect an error") - s.Equal("relative file path is invalid - may not include leading or trailing slashes", err.Error()) + s.EqualError(err, "relative file path is invalid - may not include leading or trailing slashes", + "Empty string is not a valid relative file path so we expect an error") s.Nil(f, "Since the call to NewFile resulted in an error we expect a nil pointer") f, err = l.NewFile("/foo/bar.txt") - s.Error(err, "The file path begins with a slash therefore it is not a valid relative file path so we expect an error") - s.Equal("relative file path is invalid - may not include leading or trailing slashes", err.Error()) + s.EqualError(err, "relative file path is invalid - may not include leading or trailing slashes", + "The file path begins with a slash therefore it is not a valid relative file path so we expect an error") s.Nil(f, "Since the call to NewFile resulted in an error we expect a nil pointer") f, err = l.NewFile("foo/bar/") - s.Error(err, "The file path ends with a slash therefore it is not a valid relative file path so we expect an error") - s.Equal("relative file path is invalid - may not include leading or trailing slashes", err.Error()) + s.EqualError(err, "relative file path is invalid - may not include leading or trailing slashes", + "The file path ends with a slash therefore it is not a valid relative file path so we expect an error") s.Nil(f, "Since the call to NewFile resulted in an error we expect a nil pointer") f, err = l.NewFile("foo/bar.txt") @@ -216,8 +214,7 @@ func (s *LocationTestSuite) TestNewFile() { func (s *LocationTestSuite) TestNewFile_NilReceiver() { var l *Location f, err := l.NewFile("foo/bar.txt") - s.Error(err, "Can't create a new file from a nil location so we expect an error") - s.Equal("azure.Location receiver pointer must be non-nil", err.Error()) + s.EqualError(err, "azure.Location receiver pointer must be non-nil", "Can't create a new file from a nil location so we expect an error") s.Nil(f, "the call to NewFile returned an error so we expect a nil pointer") } diff --git a/backend/ftp/dataconn_test.go b/backend/ftp/dataconn_test.go index 47d55e86..44a3be33 100644 --- a/backend/ftp/dataconn_test.go +++ b/backend/ftp/dataconn_test.go @@ -232,7 +232,7 @@ func (s *dataConnSuite) TestRead() { w := &strings.Builder{} written, err := io.Copy(w, dc) s.NoError(err, "error not expected") - s.EqualValues(len(contents), written, "byte count should equal contents of reader") + s.Len(contents, int(written), "byte count should equal contents of reader") s.Equal(contents, w.String(), "read contents equals original contents") } diff --git a/backend/ftp/fileSystem_test.go b/backend/ftp/fileSystem_test.go index 4217f501..0af0b3aa 100644 --- a/backend/ftp/fileSystem_test.go +++ b/backend/ftp/fileSystem_test.go @@ -31,7 +31,7 @@ func (ts *fileSystemTestSuite) TestNewFileSystem() { func (ts *fileSystemTestSuite) TestNewFile() { filePath := "/path/to/file.txt" file, err := ts.ftpfs.NewFile("host.com", filePath) - ts.Nil(err, "No errors returned by NewFile(%s)", filePath) + ts.NoError(err, "No errors returned by NewFile(%s)", filePath) ts.NotNil(file, "ftpfs.NewFile(%s) should assign all but first name component to key", filePath) } @@ -119,8 +119,7 @@ func (ts *fileSystemTestSuite) TestClient() { ts.ftpfs.ftpclient = nil ts.ftpfs.options = badOpt _, err = ts.ftpfs.Client(context.Background(), utils.Authority{}) - ts.Error(err, "error found") - ts.Equal("unable to create client, vfs.Options must be an ftp.Options", err.Error(), "client was already set") + ts.EqualError(err, "unable to create client, vfs.Options must be an ftp.Options", "client was already set") } func TestFileSystem(t *testing.T) { diff --git a/backend/ftp/file_test.go b/backend/ftp/file_test.go index 398004b2..59e10fe2 100644 --- a/backend/ftp/file_test.go +++ b/backend/ftp/file_test.go @@ -38,9 +38,7 @@ func (ts *fileTestSuite) SetupTest() { ts.ftpClientMock = &mocks.Client{} ts.fs = FileSystem{ftpclient: ts.ftpClientMock, options: Options{}} ts.testFile, err = ts.fs.NewFile("user@host.com:22", "/some/path/to/file.txt") - if err != nil { - ts.Fail("Shouldn't return error creating test ftp.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test ftp.File instance.") } var errClientGetter = errors.New("some dataconn getter error") @@ -76,7 +74,7 @@ func (ts *fileTestSuite) TestRead() { var localFile = bytes.NewBuffer([]byte{}) b, copyErr := io.Copy(localFile, ftpfile) ts.NoError(copyErr, "no error expected") - ts.EqualValues(len(contents), b, "byte count after copy") + ts.Len(contents, int(b), "byte count after copy") ts.Equal(contents, localFile.String(), "Copying an ftp file to a buffer should fill buffer with localfile's contents") // test read error @@ -85,7 +83,7 @@ func (ts *fileTestSuite) TestRead() { cnt, rErr := ftpfile.Read(make([]byte, 1)) ts.Error(rErr, "no error expected") ts.ErrorIs(rErr, myReadErr, "error is a read error") - ts.Equal(0, cnt, "byte count is 0") + ts.Zero(cnt, "byte count is 0") // get dataconn error dconnErr := errors.New("some getDataConn error") @@ -163,9 +161,9 @@ func (ts *fileTestSuite) TestWrite() { // test write success count, err := file.Write([]byte(contents)) - ts.Equal(len(contents), count, "Returned count of bytes written should match number of bytes passed to Write.") + ts.Len(contents, count, "Returned count of bytes written should match number of bytes passed to Write.") ts.Equal(fakeDataConn.GetWriteContents(), contents, "expected contents written") - ts.Nil(err, "Error should be nil when calling Write") + ts.NoError(err, "Error should be nil when calling Write") // test write failure myWriteErr := errors.New("some write error") @@ -173,7 +171,7 @@ func (ts *fileTestSuite) TestWrite() { count, wErr := file.Write([]byte(contents)) ts.Error(wErr, "no error expected") ts.ErrorIs(wErr, myWriteErr, "error is a write error") - ts.Equal(0, count, "byte count is 0") + ts.Zero(count, "byte count is 0") // get client error dconnErr := errors.New("some getDataConn error") @@ -1113,7 +1111,7 @@ func (ts *fileTestSuite) TestSize() { size, err = ts.testFile.Size() ts.Error(err, "expect error") ts.ErrorIs(err, myErr, "got correct error") - ts.Equal(uint64(0), size, "Size should be 0 on error") + ts.Zero(size, "Size should be 0 on error") ts.ftpClientMock.AssertExpectations(ts.T()) } diff --git a/backend/ftp/location_test.go b/backend/ftp/location_test.go index 71889758..7ee30b81 100644 --- a/backend/ftp/location_test.go +++ b/backend/ftp/location_test.go @@ -55,7 +55,7 @@ func (lt *locationTestSuite) TestList() { loc, err := lt.ftpfs.NewLocation(authority, locPath) lt.NoError(err) fileList, err := loc.List() - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return the expected number of files.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in expected file list.") @@ -190,20 +190,22 @@ func (lt *locationTestSuite) TestListByPrefix() { } for _, test := range tests { - // setup location - loc, err := lt.ftpfs.NewLocation("host.com", test.path) - lt.NoError(err, test.description) - - // setup mock List - lt.client.EXPECT(). - List(test.resolvedPath). - Return(test.allEntries, nil). - Once() - - // perform ListByPrefix - fileList, err := loc.ListByPrefix(test.prefix) - lt.NoError(err, test.description) - lt.Equal(test.expectedFiles, fileList, test.description) + lt.Run(test.description, func() { + // setup location + loc, err := lt.ftpfs.NewLocation("host.com", test.path) + lt.NoError(err, test.description) + + // setup mock List + lt.client.EXPECT(). + List(test.resolvedPath). + Return(test.allEntries, nil). + Once() + + // perform ListByPrefix + fileList, err := loc.ListByPrefix(test.prefix) + lt.NoError(err, test.description) + lt.Equal(test.expectedFiles, fileList, test.description) + }) } // client.List returns no results, return empty string slice and nil (error) @@ -295,7 +297,7 @@ func (lt *locationTestSuite) TestListByRegex() { fileTypeRegex := regexp.MustCompile("txt$") fileList, err := loc.ListByRegex(fileTypeRegex) - lt.Nil(err, "Shouldn't return an error on successful call to ListByRegex") + lt.NoError(err, "Shouldn't return an error on successful call to ListByRegex") lt.Len(fileList, len(expectedFileList), "Should return expected number of file keys.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in the expected list.") @@ -416,7 +418,7 @@ func (lt *locationTestSuite) TestExists() { loc, err := lt.ftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(exists, "Call to Exists expected to return true.") // locations does not exist @@ -433,7 +435,7 @@ func (lt *locationTestSuite) TestExists() { loc, err = lt.ftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err = loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(!exists, "Call to Exists expected to return false.") // some error calling list @@ -463,7 +465,7 @@ func (lt *locationTestSuite) TestExists() { loc, err = lt.ftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err = loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(!exists, "Call to Exists expected to return false.") // error getting client diff --git a/backend/ftp/options_test.go b/backend/ftp/options_test.go index 77be3d0a..92137b31 100644 --- a/backend/ftp/options_test.go +++ b/backend/ftp/options_test.go @@ -22,7 +22,7 @@ func TestOptions(t *testing.T) { } func (s *optionsSuite) TestFetchUsername() { - tests := []struct { + tests := []*struct { description string authority string options Options @@ -40,12 +40,14 @@ func (s *optionsSuite) TestFetchUsername() { }, } - for i := range tests { - auth, err := utils.NewAuthority(tests[i].authority) - s.NoError(err, tests[i].description) + for _, test := range tests { + s.Run(test.description, func() { + auth, err := utils.NewAuthority(test.authority) + s.NoError(err, test.description) - username := fetchUsername(auth) - s.Equal(tests[i].expected, username, tests[i].description) + username := fetchUsername(auth) + s.Equal(test.expected, username, test.description) + }) } } @@ -80,14 +82,16 @@ func (s *optionsSuite) TestFetchPassword() { }, } - for i := range tests { - if tests[i].envVar != nil { - err := os.Setenv(envPassword, *tests[i].envVar) - s.NoError(err, tests[i].description) - } + for _, test := range tests { + s.Run(test.description, func() { + if test.envVar != nil { + err := os.Setenv(envPassword, *test.envVar) + s.NoError(err, test.description) + } - password := fetchPassword(tests[i].options) - s.Equal(tests[i].expected, password, tests[i].description) + password := fetchPassword(test.options) + s.Equal(test.expected, password, test.description) + }) } } @@ -110,17 +114,19 @@ func (s *optionsSuite) TestFetchHostPortString() { }, } - for i := range tests { - auth, err := utils.NewAuthority(tests[i].authority) - s.NoError(err, tests[i].description) + for _, test := range tests { + s.Run(test.description, func() { + auth, err := utils.NewAuthority(test.authority) + s.NoError(err, test.description) - if tests[i].envVar != nil { - err := os.Setenv(envPassword, *tests[i].envVar) - s.NoError(err, tests[i].description) - } + if test.envVar != nil { + err := os.Setenv(envPassword, *test.envVar) + s.NoError(err, test.description) + } - hostPortString := fetchHostPortString(auth) - s.Equal(tests[i].expected, hostPortString, tests[i].description) + hostPortString := fetchHostPortString(auth) + s.Equal(test.expected, hostPortString, test.description) + }) } } @@ -194,14 +200,16 @@ func (s *optionsSuite) TestIsDisableEPSV() { }, } - for i := range tests { - if tests[i].envVar != nil { - err := os.Setenv(envDisableEPSV, *tests[i].envVar) - s.NoError(err, tests[i].description) - } + for _, test := range tests { + s.Run(test.description, func() { + if test.envVar != nil { + err := os.Setenv(envDisableEPSV, *test.envVar) + s.NoError(err, test.description) + } - disabled := isDisableOption(tests[i].options) - s.Equal(tests[i].expected, disabled, tests[i].description) + disabled := isDisableOption(test.options) + s.Equal(test.expected, disabled, test.description) + }) } } @@ -214,7 +222,7 @@ func (s *optionsSuite) TestFetchTLSConfig() { SessionTicketsDisabled: true, } - tests := []struct { + tests := []*struct { description string authority string options Options @@ -256,21 +264,23 @@ func (s *optionsSuite) TestFetchTLSConfig() { }, } - for i := range tests { - auth, err := utils.NewAuthority(tests[i].authority) - s.NoError(err, tests[i].description) - - tlsCfg := fetchTLSConfig(auth, tests[i].options) - s.Equal(tests[i].expected.MinVersion, tlsCfg.MinVersion, tests[i].description) - s.Equal(tests[i].expected.InsecureSkipVerify, tlsCfg.InsecureSkipVerify, tests[i].description) - s.Equal(tests[i].expected.ClientSessionCache, tlsCfg.ClientSessionCache, tests[i].description) - s.Equal(tests[i].expected.ServerName, tlsCfg.ServerName, tests[i].description) - s.Equal(tests[i].expected.SessionTicketsDisabled, tlsCfg.SessionTicketsDisabled, tests[i].description) - - if tests[i].expectInsecureCipherSuites { - s.NotEmpty(tlsCfg.CipherSuites, tests[i].description) - s.True(containsInsecureCipherSuites(tlsCfg.CipherSuites), tests[i].description) - } + for _, test := range tests { + s.Run(test.description, func() { + auth, err := utils.NewAuthority(test.authority) + s.NoError(err, test.description) + + tlsCfg := fetchTLSConfig(auth, test.options) + s.Equal(test.expected.MinVersion, tlsCfg.MinVersion, test.description) + s.Equal(test.expected.InsecureSkipVerify, tlsCfg.InsecureSkipVerify, test.description) + s.Equal(test.expected.ClientSessionCache, tlsCfg.ClientSessionCache, test.description) + s.Equal(test.expected.ServerName, tlsCfg.ServerName, test.description) + s.Equal(test.expected.SessionTicketsDisabled, tlsCfg.SessionTicketsDisabled, test.description) + + if test.expectInsecureCipherSuites { + s.NotEmpty(tlsCfg.CipherSuites, test.description) + s.True(containsInsecureCipherSuites(tlsCfg.CipherSuites), test.description) + } + }) } } @@ -334,20 +344,22 @@ func (s *optionsSuite) TestFetchProtocol() { }, } - for i := range tests { - s.NoError(os.Unsetenv(envProtocol)) - if tests[i].envVar != nil { - err := os.Setenv(envProtocol, *tests[i].envVar) - s.NoError(err, tests[i].description) - } + for _, test := range tests { + s.Run(test.description, func() { + s.NoError(os.Unsetenv(envProtocol)) + if test.envVar != nil { + err := os.Setenv(envProtocol, *test.envVar) + s.NoError(err, test.description) + } - protocol := fetchProtocol(tests[i].options) - s.Equal(tests[i].expected, protocol, tests[i].description) + protocol := fetchProtocol(test.options) + s.Equal(test.expected, protocol, test.description) + }) } } func (s *optionsSuite) TestFetchDialOptions() { - tests := []struct { + tests := []*struct { description string authority string options Options @@ -421,17 +433,19 @@ func (s *optionsSuite) TestFetchDialOptions() { }, } - for i := range tests { - if tests[i].envVar != nil { - err := os.Setenv(envProtocol, *tests[i].envVar) - s.NoError(err, tests[i].description) - } + for _, test := range tests { + s.Run(test.description, func() { + if test.envVar != nil { + err := os.Setenv(envProtocol, *test.envVar) + s.NoError(err, test.description) + } - auth, err := utils.NewAuthority(tests[i].authority) - s.NoError(err, tests[i].description) + auth, err := utils.NewAuthority(test.authority) + s.NoError(err, test.description) - dialOpts := fetchDialOptions(context.Background(), auth, tests[i].options) - s.Len(dialOpts, tests[i].expected, tests[i].description) + dialOpts := fetchDialOptions(context.Background(), auth, test.options) + s.Len(dialOpts, test.expected, test.description) + }) } } diff --git a/backend/gs/file_test.go b/backend/gs/file_test.go index c3cbf2a7..c43ec292 100644 --- a/backend/gs/file_test.go +++ b/backend/gs/file_test.go @@ -9,7 +9,6 @@ import ( "cloud.google.com/go/storage" "github.com/fsouza/fake-gcs-server/fakestorage" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v6/options/delete" @@ -98,17 +97,15 @@ func (ts *fileTestSuite) TestRead() { fs := NewFileSystem().WithClient(server.Client()) file, err := fs.NewFile(bucketName, "/"+objectName) - if err != nil { - ts.Fail("Shouldn't fail creating new file") - } + ts.Require().NoError(err, "Shouldn't fail creating new file") var localFile = bytes.NewBuffer([]byte{}) buffer := make([]byte, utils.TouchCopyMinBufferSize) _, copyErr := io.CopyBuffer(localFile, file, buffer) - assert.NoError(ts.T(), copyErr, "no error expected") + ts.NoError(copyErr, "no error expected") closeErr := file.Close() - assert.NoError(ts.T(), closeErr, "no error expected") + ts.NoError(closeErr, "no error expected") ts.Equal(localFile.String(), contents, "Copying an gs file to a buffer should fill buffer with file's contents") } @@ -135,17 +132,13 @@ func (ts *fileTestSuite) TestDelete() { fs := NewFileSystem().WithClient(client) file, err := fs.NewFile(bucketName, "/"+objectName) - if err != nil { - ts.Fail("Shouldn't fail creating new file") - } + ts.Require().NoError(err, "Shouldn't fail creating new file") err = file.Delete() - if err != nil { - ts.Fail("Shouldn't fail deleting the file") - } + ts.Require().NoError(err, "Shouldn't fail deleting the file") bucket := client.Bucket(bucketName) - assert.Equal(ts.T(), false, objectExists(bucket, objectName)) + ts.False(objectExists(bucket, objectName)) } func (ts *fileTestSuite) TestDeleteError() { @@ -170,12 +163,10 @@ func (ts *fileTestSuite) TestDeleteError() { fs := NewFileSystem().WithClient(client) file, err := fs.NewFile(bucketName, "/invalidObject") - if err != nil { - ts.Fail("Shouldn't fail creating new file") - } + ts.Require().NoError(err, "Shouldn't fail creating new file") err = file.Delete() - ts.NotNil(err, "Should return an error if gs client had error") + ts.Error(err, "Should return an error if gs client had error") } func (ts *fileTestSuite) TestDeleteRemoveAllVersions() { @@ -200,29 +191,21 @@ func (ts *fileTestSuite) TestDeleteRemoveAllVersions() { fs := NewFileSystem().WithClient(client) file, err := fs.NewFile(bucketName, "/"+objectName) - if err != nil { - ts.Fail("Shouldn't fail creating new file") - } + ts.Require().NoError(err, "Shouldn't fail creating new file") f := file.(*File) handles, err := f.getObjectGenerationHandles() - if err != nil { - ts.Fail("Shouldn't fail getting object generation handles") - } - assert.Equal(ts.T(), 1, len(handles)) + ts.Require().NoError(err, "Shouldn't fail getting object generation handles") + ts.Len(handles, 1) err = file.Delete(delete.WithDeleteAllVersions()) - if err != nil { - ts.Fail("Shouldn't fail deleting the file") - } + ts.Require().NoError(err, "Shouldn't fail deleting the file") bucket := client.Bucket(bucketName) - assert.Equal(ts.T(), false, objectExists(bucket, objectName)) + ts.False(objectExists(bucket, objectName)) handles, err = f.getObjectGenerationHandles() - if err != nil { - ts.Fail("Shouldn't fail getting object generation handles") - } - assert.Nil(ts.T(), handles) + ts.Require().NoError(err, "Shouldn't fail getting object generation handles") + ts.Nil(handles) } func (ts *fileTestSuite) TestWrite() { @@ -238,8 +221,8 @@ func (ts *fileTestSuite) TestWrite() { count, err := file.Write([]byte(contents)) - ts.Equal(len(contents), count, "Returned count of bytes written should match number of bytes passed to Write.") - ts.Nil(err, "Error should be nil when calling Write") + ts.Len(contents, count, "Returned count of bytes written should match number of bytes passed to Write.") + ts.NoError(err, "Error should be nil when calling Write") } func (ts *fileTestSuite) TestGetLocation() { @@ -274,13 +257,11 @@ func (ts *fileTestSuite) TestExists() { fs := NewFileSystem().WithClient(server.Client()) file, err := fs.NewFile(bucketName, "/"+objectName) - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") exists, err := file.Exists() ts.True(exists, "Should return true for exists based on this setup") - ts.Nil(err, "Shouldn't return an error when exists is true") + ts.NoError(err, "Shouldn't return an error when exists is true") } func (ts *fileTestSuite) TestNotExists() { @@ -289,13 +270,11 @@ func (ts *fileTestSuite) TestNotExists() { fs := NewFileSystem().WithClient(server.Client()) file, err := fs.NewFile("bucket", "/path/hello.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") exists, err := file.Exists() ts.False(exists, "Should return false for exists based on setup") - ts.Nil(err, "Error from key not existing should be hidden since it just confirms it doesn't") + ts.NoError(err, "Error from key not existing should be hidden since it just confirms it doesn't") } func (ts *fileTestSuite) TestMoveAndCopy() { @@ -382,7 +361,7 @@ func (ts *fileTestSuite) TestMoveAndCopy() { if testCase.readFirst { ts.Error(err, "Error should be returned for operation on file that has been read (i.e. has non 0 cursor position)") } else { - ts.Nil(err, "Error shouldn't be returned from successful operation") + ts.NoError(err, "Error shouldn't be returned from successful operation") if testCase.move { ts.False(objectExists(sourceBucket, sourceName), "source should not exist") @@ -487,7 +466,7 @@ func (ts *fileTestSuite) TestMoveAndCopyBuffered() { if testCase.readFirst { ts.Error(err, "Error should be returned for operation on file that has been read (i.e. has non 0 cursor position)") } else { - ts.Nil(err, "Error shouldn't be returned from successful operation") + ts.NoError(err, "Error shouldn't be returned from successful operation") if testCase.move { ts.False(objectExists(sourceBucket, sourceName), "source should not exist") diff --git a/backend/gs/location_test.go b/backend/gs/location_test.go index 6b45ead0..6a2ad55a 100644 --- a/backend/gs/location_test.go +++ b/backend/gs/location_test.go @@ -5,7 +5,6 @@ import ( "regexp" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v6/utils" @@ -68,61 +67,45 @@ func (lt *locationTestSuite) TestList() { fs := NewFileSystem().WithClient(server.Client()) for _, objectPrefix := range objectPrefixes { - lt.T().Run("list direct "+objectPrefix, func(t *testing.T) { + lt.Run("list direct "+objectPrefix, func() { loc, err := fs.NewLocation(bucket, "/"+objectPrefix) - if err != nil { - lt.T().Fatal(err) - } - t.Logf("location URI: %q", loc.URI()) + lt.Require().NoError(err) + lt.T().Logf("location URI: %q", loc.URI()) files, err := loc.List() - if err != nil { - lt.T().Fatal(err) - } - assert.ElementsMatch(t, objectBaseNames, files, "should find all files in the location") + lt.Require().NoError(err) + lt.ElementsMatch(objectBaseNames, files, "should find all files in the location") }) - lt.T().Run("list prefix "+objectPrefix, func(t *testing.T) { + lt.Run("list prefix "+objectPrefix, func() { loc, err := fs.NewLocation(bucket, "/") - if err != nil { - lt.T().Fatal(err) - } - t.Logf("location URI: %q", loc.URI()) + lt.Require().NoError(err) + lt.T().Logf("location URI: %q", loc.URI()) - t.Run("without slash", func(t *testing.T) { + lt.Run("without slash", func() { files, err := loc.ListByPrefix(objectPrefix) - if err != nil { - lt.T().Fatal(err) - } - assert.ElementsMatch(t, objectBaseNames, files, "should find all files in the location") + lt.Require().NoError(err) + lt.ElementsMatch(objectBaseNames, files, "should find all files in the location") }) - t.Run("with slash", func(t *testing.T) { + lt.Run("with slash", func() { files, err := loc.ListByPrefix(objectPrefix + "/") - if err != nil { - lt.T().Fatal(err) - } - assert.ElementsMatch(t, objectBaseNames, files, "should find all files in the location") + lt.Require().NoError(err) + lt.ElementsMatch(objectBaseNames, files, "should find all files in the location") }) - t.Run("include object-level filename prefix f2", func(t *testing.T) { + lt.Run("include object-level filename prefix f2", func() { files, err := loc.ListByPrefix(objectPrefix + "/f2") - if err != nil { - lt.T().Fatal(err) - } + lt.Require().NoError(err) fileObjectBaseNames := []string{"f2.txt"} - assert.ElementsMatch(t, fileObjectBaseNames, files, "should find all files in the location matching f2") + lt.ElementsMatch(fileObjectBaseNames, files, "should find all files in the location matching f2") }) }) - lt.T().Run("list regex "+objectPrefix, func(t *testing.T) { + lt.Run("list regex "+objectPrefix, func() { loc, err := fs.NewLocation(bucket, "/"+objectPrefix) - if err != nil { - lt.T().Fatal(err) - } - t.Logf("location URI: %q", loc.URI()) + lt.Require().NoError(err) + lt.T().Logf("location URI: %q", loc.URI()) files, err := loc.ListByRegex(regexp.MustCompile("^f[02].txt$")) - if err != nil { - lt.T().Fatal(err) - } - assert.ElementsMatch(t, []string{"f0.txt", "f2.txt"}, files, + lt.Require().NoError(err) + lt.ElementsMatch([]string{"f0.txt", "f2.txt"}, files, "should find exactly two files f0.txt and f2.txt") }) } @@ -203,7 +186,7 @@ func (lt *locationTestSuite) TestExists_true() { loc, err := fs.NewLocation(bucket, "/") lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(exists, "Call to Exists expected to return true.") } @@ -215,7 +198,7 @@ func (lt *locationTestSuite) TestExists_false() { loc, err := fs.NewLocation(bucket, "/") lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.False(exists, "Call to Exists expected to return true.") } @@ -309,12 +292,12 @@ func (lt *locationTestSuite) TestDeleteFile() { loc, err := fs.NewLocation(bucket, "/old/") lt.NoError(err) - lt.T().Run("delete existing", func(t *testing.T) { + lt.Run("delete existing", func() { err = loc.DeleteFile("filename.txt") - lt.Nil(err, "Successful delete should not return an error.") + lt.NoError(err, "Successful delete should not return an error.") }) - lt.T().Run("delete non-existing", func(t *testing.T) { + lt.Run("delete non-existing", func() { err = loc.DeleteFile("filename.txt") lt.Error(err, "Delete of non existing file should fail") }) diff --git a/backend/mem/file_test.go b/backend/mem/file_test.go index a04be56f..4192e1ad 100644 --- a/backend/mem/file_test.go +++ b/backend/mem/file_test.go @@ -3,7 +3,6 @@ package mem import ( "io" "io/fs" - "log" "os" "path" "path/filepath" @@ -175,7 +174,7 @@ func (s *memFileTest) TestNewFile() { filePath := file.Path() returnedFile, err := s.fileSystem.NewFile(file.Location().Volume(), filePath) // checking our system map for a match to the given fileName s.NoError(err, "unexpected error retrieving file") - s.True(returnedFile.Name() == "foo.txt") // casting the object to a file so we can call "Name()" + s.Equal("foo.txt", returnedFile.Name()) // casting the object to a file so we can call "Name()" } @@ -203,7 +202,7 @@ func (s *memFileTest) TestSeek2() { num, err := newFile.Read(testByte) s.NoError(err, "unexpected read error") - s.True(string(testByte) == "H") + s.Equal("H", string(testByte)) s.Equal(1, num) _, err = newFile.Seek(-2, 1) @@ -260,7 +259,7 @@ func (s *memFileTest) TestOpenFile() { _, err = s.testFile.Read(data) s.Error(err, "read error expected") - s.False(expectedText == string(data)) + s.NotEqual(expectedText, string(data)) } // TestSeek writes to a file and seeks to the beginning of it to read what it wrote @@ -313,16 +312,16 @@ func (s *memFileTest) TestCopyToLocation() { s.NoError(cerr, "CopyToLocation unexpectedly failed") s.NoError(copiedFile.Touch(), "unexpected error touching file") - s.True(copiedFile != nil) + s.NotNil(copiedFile) // making sure the path was correctly updated - s.EqualValues("/home/test.txt", copiedFile.Path()) + s.Equal("/home/test.txt", copiedFile.Path()) _, err = copiedFile.Read(readSlice1) s.NoError(err, "unexpected read error") _, err = s.testFile.Read(readSlice2) s.NoError(err, "unexpected read error") - s.EqualValues(string(readSlice2), string(readSlice1)) + s.Equal(readSlice2, readSlice1) } @@ -347,13 +346,13 @@ func (s *memFileTest) TestCopyToLocationOW() { readSlice := make([]byte, len(expectedText)) copiedFile, err := s.testFile.CopyToLocation(newFile.Location()) s.NoError(err, "CopyToLocation unexpectedly failed") - s.True(copiedFile != nil) + s.NotNil(copiedFile) s.NoError(copiedFile.Close(), "unexpected close error") - s.EqualValues("/home/test.txt", copiedFile.Path()) + s.Equal("/home/test.txt", copiedFile.Path()) _, err = copiedFile.Read(readSlice) s.NoError(err, "unexpected read error") - s.EqualValues("hello world!", string(readSlice)) + s.Equal("hello world!", string(readSlice)) } @@ -377,9 +376,7 @@ func (s *memFileTest) TestCopyToLocationOS() { var osFile vfs.File dir, err := os.MkdirTemp("", "osDir") - if err != nil { - log.Fatal(err) - } + s.Require().NoError(err) osFileName := filepath.Join(dir, "osFile.txt") osFile, err = backend.Backend(_os.Scheme).NewFile("", osFileName) @@ -399,17 +396,17 @@ func (s *memFileTest) TestCopyToLocationOS() { s.NoError(err, "CopyToLocation unexpectedly failed") s.NoError(copiedFile.Close(), "unexpected Close error") - s.True(copiedFile != nil) - s.EqualValues("/test_files/test.txt", s.testFile.Path()) // testFile's path should be unchanged - s.EqualValues(filepath.Join(dir, "test.txt"), copiedFile.Path()) // new path should be that + s.NotNil(copiedFile) + s.Equal("/test_files/test.txt", s.testFile.Path()) // testFile's path should be unchanged + s.Equal(filepath.Join(dir, "test.txt"), copiedFile.Path()) // new path should be that _, err = copiedFile.Read(readSlice) s.NoError(err, "unexpected read error") _, err = s.testFile.Read(readSlice2) s.NoError(err, "unexpected read error") - s.EqualValues(string(readSlice2), string(readSlice)) // both reads should be the same - cleanErr := os.RemoveAll(dir) // clean up + s.Equal(readSlice2, readSlice) // both reads should be the same + cleanErr := os.RemoveAll(dir) // clean up s.NoError(cleanErr, "unexpected error cleaning up osFiles") } @@ -422,7 +419,7 @@ func (s *memFileTest) TestCopyToFile() { readSlice1 := make([]byte, len(expectedText)) readSlice2 := make([]byte, len(expectedText)) num, err := s.testFile.Write([]byte(expectedText)) - s.False(num == 0) + s.NotZero(num) s.NoError(err, "no error expected from Write but got one") s.NoError(s.testFile.Close(), "unexpected error closing a file") @@ -436,7 +433,7 @@ func (s *memFileTest) TestCopyToFile() { s.NoError(err, "unexpected creation error") _, err = otherFile.Read(readSlice2) s.NoError(err, "unexpected read error") - s.EqualValues(string(readSlice1), string(readSlice2)) + s.Equal(readSlice1, readSlice2) } @@ -447,16 +444,14 @@ func (s *memFileTest) TestCopyToFileOS() { var osFile vfs.File var err error dir, err := os.MkdirTemp("", "osDir") - if err != nil { - log.Fatal(err) - } + s.Require().NoError(err) osFileName := filepath.Join(dir, "osFile.txt") osFile, err = backend.Backend(_os.Scheme).NewFile("", osFileName) s.NoError(err, "unexpected error creating osFile") _, err = osFile.Write(make([]byte, 0)) s.NoError(err, "unexpected error writing zero bytes to osFile") num, err := s.testFile.Write([]byte(expectedText)) - s.False(num == 0) + s.NotZero(num) s.NoError(err, "no error expected from Write but got one") s.NoError(s.testFile.Close(), "unexpected error closing a file") @@ -579,7 +574,7 @@ func (s *memFileTest) TestMoveToFile() { _, err = newFile.Read(newFileSlice) s.NoError(err, "read unexpectedly failed") - s.Equal(string(expectedSlice), string(newFileSlice)) + s.Equal(expectedSlice, newFileSlice) s.Equal("/samples/test.txt", newFile.Path()) } @@ -604,7 +599,7 @@ func (s *memFileTest) TestMoveToFile2() { _, err = newFile.Read(newFileSlice) s.NoError(err, "read unexpectedly failed") - s.Equal(string(expectedSlice), string(newFileSlice)) + s.Equal(expectedSlice, newFileSlice) s.Equal("/samples/diffName.txt", newFile.Path()) } @@ -628,7 +623,7 @@ func (s *memFileTest) TestWrite() { s.NoError(s.testFile.Close(), "unexpected close error") actualText, err := io.ReadAll(s.testFile) s.NoError(err, "unexpected read error") - s.EqualValues("Hello World!ith this world", string(actualText)) + s.Equal("Hello World!ith this world", string(actualText)) } // TestRead ensures read can be called successively to get an entire file's contents in chunks @@ -644,7 +639,7 @@ func (s *memFileTest) TestRead() { b, err := io.ReadAll(fileToRead) s.NoError(err, "read error not expected") - s.EqualValues(12, len(b)) + s.Len(b, 12) s.Equal(expectedSlice, b) // test for reading from a non-existent file diff --git a/backend/mem/location_test.go b/backend/mem/location_test.go index f7af02e7..4cd35a07 100644 --- a/backend/mem/location_test.go +++ b/backend/mem/location_test.go @@ -51,24 +51,24 @@ func (s *memLocationTest) TestList() { // by showing that no files live on those directories func (s *memLocationTest) TestList_NonExistentDirectory() { location, err := s.testFile.Location().NewLocation("not/a/directory/") - s.Nil(err, "error isn't expected") + s.NoError(err, "error isn't expected") exists, err := location.Exists() - s.Nil(err, "error isn't expected") + s.NoError(err, "error isn't expected") s.True(exists, "location should return true for Exists") contents, err := location.List() - s.Nil(err, "error isn't expected") - s.Equal(0, len(contents), "list should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(contents, "list should return empty slice for non-existent directory") prefixContents, err := location.ListByPrefix("anything") - s.Nil(err, "error isn't expected") - s.Equal(0, len(prefixContents), "ListByPrefix should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(prefixContents, "ListByPrefix should return empty slice for non-existent directory") regex := regexp.MustCompile("-+") regexContents, err := location.ListByRegex(regex) - s.Nil(err, "error isn't expected") - s.Equal(0, len(regexContents), "ListByRegex should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(regexContents, "ListByRegex should return empty slice for non-existent directory") } // TestListByPrefix creates some files and provides a prefix. Succeeds on correct string slice returned @@ -103,7 +103,7 @@ func (s *memLocationTest) TestListByPrefix() { s.Equal(expectedSlice, nameSlice) emptySlice, err := s.testFile.Location().ListByPrefix("m") s.NoError(err, "unexpected error retrieving files by prefix") - s.Equal(make([]string, 0), emptySlice) // no files should be found with this prefix at this location + s.Empty(emptySlice) // no files should be found with this prefix at this location } // TestListByRegex provides a simple regular expression and ensures that the correct fileNames matched that regEx @@ -235,7 +235,7 @@ func (s *memLocationTest) TestChangeDir() { exists, eerr := loc.Exists() s.NoError(eerr, "unexpected error checking for Existence") s.True(exists) - s.False(newFile.Location().Path() == loc.Path()) + s.NotEqual(newFile.Location().Path(), loc.Path()) } // TestVolume makes sure that the mem-fs returns the empty string for its volume diff --git a/backend/os/fileSystem_test.go b/backend/os/fileSystem_test.go index 08050378..a097fd34 100644 --- a/backend/os/fileSystem_test.go +++ b/backend/os/fileSystem_test.go @@ -37,8 +37,7 @@ func (o *osFileSystemTest) TestNewFile() { // failure on validation file, err := fs.NewFile("", "invalid/file") - o.Error(err, "error expected for invalid file") - o.EqualError(err, utils.ErrBadAbsFilePath) + o.EqualError(err, utils.ErrBadAbsFilePath, "error expected for invalid file") o.Nil(file, "file should be nil on err") // success @@ -52,13 +51,11 @@ func (o *osFileSystemTest) TestNewLocation() { // failure on validation loc, err := fs.NewLocation("", "/invalid/location") - o.Error(err, "error expected for invalid file") - o.EqualError(err, utils.ErrBadAbsLocationPath) + o.EqualError(err, utils.ErrBadAbsLocationPath, "error expected for invalid file") o.Nil(loc, "file should be nil on err") loc, err = fs.NewLocation("", "invalid/location/") - o.Error(err, "error expected for invalid file") - o.EqualError(err, utils.ErrBadAbsLocationPath) + o.EqualError(err, utils.ErrBadAbsLocationPath, "error expected for invalid file") o.Nil(loc, "file should be nil on err") // success diff --git a/backend/os/file_test.go b/backend/os/file_test.go index 04a6ceee..295f08bd 100644 --- a/backend/os/file_test.go +++ b/backend/os/file_test.go @@ -88,7 +88,7 @@ func (s *osFileTest) TestTouch() { // size should be zero size, err := testfile.Size() s.NoError(err) - s.Equal(size, uint64(0), "size should be zero") + s.Zero(size, "size should be zero") // capture last_modified firstModTime, err := testfile.LastModified() @@ -101,7 +101,7 @@ func (s *osFileTest) TestTouch() { // size should still be zero size, err = testfile.Size() s.NoError(err) - s.Equal(size, uint64(0), "size should be zero") + s.Zero(size, "size should be zero") // LastModified should be later than previous LastModified nextModTime, err := testfile.LastModified() @@ -151,7 +151,7 @@ func (s *osFileTest) TestRead() { data = make([]byte, 4) b, err = f.Read(data) s.Error(err) - s.Equal(0, b) + s.Zero(b) f.(*File).fileOpener = nil b, err = f.Write([]byte("blah")) @@ -162,7 +162,7 @@ func (s *osFileTest) TestRead() { data = make([]byte, 4) b, err = f.Read(data) s.Error(err) - s.Equal(0, b) + s.Zero(b) } func (s *osFileTest) TestSeek() { @@ -267,10 +267,7 @@ func (s *osFileTest) TestCopyToLocationIgnoreExtraSeparator() { location := Location{"/some/path/", otherFs} _, err := s.testFile.CopyToLocation(&location) - - if err != nil { - s.Fail(err.Error()) - } + s.Require().NoError(err) otherFs.AssertCalled(s.T(), "NewFile", "", "/some/path/test.txt") } @@ -377,7 +374,7 @@ func (s *osFileTest) TestSafeOsRename() { err = safeOsRename(badfile, newFile) s.Error(err) - s.NotEqual(err.Error(), osCrossDeviceLinkError) + s.NotEqual(osCrossDeviceLinkError, err.Error()) } func (s *osFileTest) TestOsCopy() { @@ -676,7 +673,7 @@ func (s *osFileTest) TestSize() { s.NoError(err) size, err = noFile.Size() s.Error(err) - s.Equal(uint64(0), size) + s.Zero(size) } func (s *osFileTest) TestPath() { diff --git a/backend/os/location_test.go b/backend/os/location_test.go index 21bba629..7993f2d5 100644 --- a/backend/os/location_test.go +++ b/backend/os/location_test.go @@ -6,7 +6,6 @@ import ( "regexp" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/c2fo/vfs/v6" @@ -54,24 +53,24 @@ func (s *osLocationTest) TestList() { func (s *osLocationTest) TestList_NonExistentDirectory() { location, err := s.testFile.Location().NewLocation("not/a/directory/") - s.Nil(err, "error isn't expected") + s.NoError(err, "error isn't expected") exists, err := location.Exists() - s.Nil(err, "error isn't expected") + s.NoError(err, "error isn't expected") s.False(exists, "location should return false for Exists") contents, err := location.List() - s.Nil(err, "error isn't expected") - s.Equal(0, len(contents), "List should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(contents, "List should return empty slice for non-existent directory") prefixContents, err := location.ListByPrefix("anything") - s.Nil(err, "error isn't expected") - s.Equal(0, len(prefixContents), "ListByPrefix should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(prefixContents, "ListByPrefix should return empty slice for non-existent directory") regex := regexp.MustCompile("-+") regexContents, err := location.ListByRegex(regex) - s.Nil(err, "error isn't expected") - s.Equal(0, len(regexContents), "ListByRegex should return empty slice for non-existent directory") + s.NoError(err, "error isn't expected") + s.Empty(regexContents, "ListByRegex should return empty slice for non-existent directory") } func (s *osLocationTest) TestListByPrefix() { @@ -116,7 +115,7 @@ func (s *osLocationTest) TestChangeDir() { fileLocation := otherFile.Location() cwd := fileLocation.Path() err := fileLocation.ChangeDir("other/") - assert.NoError(s.T(), err, "change dir error not expected") + s.NoError(err, "change dir error not expected") s.Equal(fileLocation.Path(), utils.EnsureTrailingSlash(filepath.Join(cwd, "other"))) } diff --git a/backend/s3/fileSystem_test.go b/backend/s3/fileSystem_test.go index 38e1b098..84ef0509 100644 --- a/backend/s3/fileSystem_test.go +++ b/backend/s3/fileSystem_test.go @@ -36,7 +36,7 @@ func (ts *fileSystemTestSuite) TestNewFileSystem() { func (ts *fileSystemTestSuite) TestNewFile() { filePath := "/path/to/file.txt" file, err := s3fs.NewFile("bucketName", filePath) - ts.Nil(err, "No errors returned by NewFile(%s)", filePath) + ts.NoError(err, "No errors returned by NewFile(%s)", filePath) ts.NotNil(file, "fs.NewFile(%s) should assign all but first name component to key", filePath) } @@ -108,8 +108,7 @@ func (ts *fileSystemTestSuite) TestClient() { s3fs.client = nil s3fs.options = badOpt _, err = s3fs.Client() - ts.Error(err, "error found") - ts.Equal("unable to create client, vfs.Options must be an s3.Options", err.Error(), "client was already set") + ts.EqualError(err, "unable to create client, vfs.Options must be an s3.Options", "client was already set") s3fs = &FileSystem{} client, err = s3fs.Client() diff --git a/backend/s3/file_test.go b/backend/s3/file_test.go index 317885d8..be3c4e79 100644 --- a/backend/s3/file_test.go +++ b/backend/s3/file_test.go @@ -17,7 +17,6 @@ import ( "github.com/aws/aws-sdk-go/service/s3/s3iface" "github.com/aws/aws-sdk-go/service/s3/s3manager" "github.com/aws/aws-sdk-go/service/s3/s3manager/s3manageriface" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -49,10 +48,7 @@ func (ts *fileTestSuite) SetupTest() { testFileName = "/some/path/to/file.txt" bucket = "bucket" testFile, err = fs.NewFile(bucket, testFileName) - - if err != nil { - ts.Fail("Shouldn't return error creating test s3.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test s3.File instance.") } func (ts *fileTestSuite) TearDownTest() { @@ -62,9 +58,7 @@ func (ts *fileTestSuite) TestRead() { contents := "hello world!" file, err := fs.NewFile("bucket", "/some/path/file.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file") - } + ts.Require().NoError(err, "Shouldn't fail creating new file") var localFile = bytes.NewBuffer([]byte{}) s3apiMock. @@ -105,8 +99,8 @@ func (ts *fileTestSuite) TestWrite() { contents := []byte("Hello world!") count, err := file.Write(contents) - ts.Equal(len(contents), count, "Returned count of bytes written should match number of bytes passed to Write.") - ts.Nil(err, "Error should be nil when calling Write") + ts.Len(contents, count, "Returned count of bytes written should match number of bytes passed to Write.") + ts.NoError(err, "Error should be nil when calling Write") } func (ts *fileTestSuite) TestSeek() { @@ -191,29 +185,25 @@ func (ts *fileTestSuite) TestGetLocation() { func (ts *fileTestSuite) TestExists() { file, err := fs.NewFile("bucket", "/path/hello.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") s3apiMock.On("HeadObject", mock.AnythingOfType("*s3.HeadObjectInput")).Return(&s3.HeadObjectOutput{}, nil) exists, err := file.Exists() ts.True(exists, "Should return true for exists based on this setup") - ts.Nil(err, "Shouldn't return an error when exists is true") + ts.NoError(err, "Shouldn't return an error when exists is true") } func (ts *fileTestSuite) TestNotExists() { file, err := fs.NewFile("bucket", "/path/hello.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") s3apiMock.On("HeadObject", mock.AnythingOfType("*s3.HeadObjectInput")). Return(&s3.HeadObjectOutput{}, awserr.New(s3.ErrCodeNoSuchKey, "key doesn't exist", nil)) exists, err := file.Exists() ts.False(exists, "Should return false for exists based on setup") - ts.Nil(err, "Error from key not existing should be hidden since it just confirms it doesn't") + ts.NoError(err, "Error from key not existing should be hidden since it just confirms it doesn't") } func (ts *fileTestSuite) TestCopyToFile() { @@ -229,7 +219,7 @@ func (ts *fileTestSuite) TestCopyToFile() { s3apiMock.On("CopyObject", mock.AnythingOfType("*s3.CopyObjectInput")).Return(&s3.CopyObjectOutput{}, nil) err := testFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") s3apiMock.AssertExpectations(ts.T()) // Test With Non Minimum Buffer Size in TouchCopyBuffered @@ -248,7 +238,7 @@ func (ts *fileTestSuite) TestCopyToFile() { s3apiMock.On("CopyObject", mock.AnythingOfType("*s3.CopyObjectInput")).Return(&s3.CopyObjectOutput{}, nil) err = testFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") s3apiMock.AssertExpectations(ts.T()) } @@ -265,7 +255,7 @@ func (ts *fileTestSuite) TestEmptyCopyToFile() { Return(&s3.GetObjectOutput{Body: io.NopCloser(strings.NewReader(""))}, nil). Once() err := testFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") // Assert that file was still written to and closed when the reader size is 0 bytes. targetFile.AssertExpectations(ts.T()) @@ -285,7 +275,7 @@ func (ts *fileTestSuite) TestMoveToFile() { s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(&s3.DeleteObjectOutput{}, nil) err := testFile.MoveToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to MoveToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to MoveToFile") s3apiMock.AssertExpectations(ts.T()) } @@ -317,35 +307,37 @@ func (ts *fileTestSuite) TestGetCopyObject() { } // ensure spaces are properly encoded (or not) - for _, t := range tests { - sourceFile := &File{ - fileSystem: &FileSystem{ - client: s3apiMock, - options: Options{ - AccessKeyID: "abc", - DisableServerSideEncryption: true, + for i, t := range tests { + ts.Run(fmt.Sprintf("%d", i), func() { + sourceFile := &File{ + fileSystem: &FileSystem{ + client: s3apiMock, + options: Options{ + AccessKeyID: "abc", + DisableServerSideEncryption: true, + }, }, - }, - bucket: "TestBucket", - key: t.key, - } - - targetFile := &File{ - fileSystem: &FileSystem{ - client: s3apiMock, - options: Options{ - AccessKeyID: "abc", + bucket: "TestBucket", + key: t.key, + } + + targetFile := &File{ + fileSystem: &FileSystem{ + client: s3apiMock, + options: Options{ + AccessKeyID: "abc", + }, }, - }, - bucket: "TestBucket", - key: "source.txt", - } - - // copy from t.key to /source.txt - actual, err := sourceFile.getCopyObjectInput(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") - ts.Equal("TestBucket"+t.expectedCopySource, *actual.CopySource) - ts.Nil(actual.ServerSideEncryption, "sse is disabled") + bucket: "TestBucket", + key: "source.txt", + } + + // copy from t.key to /source.txt + actual, err := sourceFile.getCopyObjectInput(targetFile) + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.Equal("TestBucket"+t.expectedCopySource, *actual.CopySource) + ts.Nil(actual.ServerSideEncryption, "sse is disabled") + }) } // test that different options returns nil @@ -370,7 +362,7 @@ func (ts *fileTestSuite) TestGetCopyObject() { key: "/path/to/otherFile.txt", } actual, err := sourceFile.getCopyObjectInput(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") ts.Nil(actual, "copyObjectInput should be nil (can't do s3-to-s3 copyObject)") s3apiMock.AssertExpectations(ts.T()) @@ -389,7 +381,7 @@ func (ts *fileTestSuite) TestMoveToFile_CopyError() { s3apiMock.On("CopyObject", mock.AnythingOfType("*s3.CopyObjectInput")).Return(nil, errors.New("some copy error")) err := testFile.MoveToFile(targetFile) - ts.NotNil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.Error(err, "Error shouldn't be returned from successful call to CopyToFile") s3apiMock.AssertNotCalled(ts.T(), "DeleteObject", mock.Anything) s3apiMock.AssertExpectations(ts.T()) } @@ -411,7 +403,7 @@ func (ts *fileTestSuite) TestCopyToLocation() { defer func() { closeErr := f.Close() - assert.NoError(ts.T(), closeErr, "no error expected") + ts.NoError(closeErr, "no error expected") }() l := &Location{ @@ -512,9 +504,7 @@ func (ts *fileTestSuite) TestMoveToLocation() { s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(&s3.DeleteObjectOutput{}, nil) file, err := fs.NewFile("bucket", "/hello.txt") - if err != nil { - ts.Fail("Shouldn't return error creating test s3.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test s3.File instance.") defer func() { closeErr := file.Close() @@ -534,9 +524,7 @@ func (ts *fileTestSuite) TestMoveToLocation() { fs = FileSystem{client: s3apiMock2} file2, err := fs.NewFile("bucket", "/hello.txt") - if err != nil { - ts.Fail("Shouldn't return error creating test s3.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test s3.File instance.") _, err = file2.CopyToLocation(mockLocation) ts.NoError(err, "MoveToLocation error not expected") @@ -556,9 +544,7 @@ func (ts *fileTestSuite) TestMoveToLocationFail() { s3apiMock.On("CopyObject", mock.AnythingOfType("*s3.CopyObjectInput")).Return(nil, errors.New("didn't copy, oh noes")) file, err := fs.NewFile("bucket", "/hello.txt") - if err != nil { - ts.Fail("Shouldn't return error creating test s3.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test s3.File instance.") _, merr := file.MoveToLocation(location) ts.Error(merr, "MoveToLocation error not expected") @@ -575,15 +561,14 @@ func (ts *fileTestSuite) TestMoveToLocationFail() { func (ts *fileTestSuite) TestDelete() { s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(&s3.DeleteObjectOutput{}, nil) err := testFile.Delete() - ts.Nil(err, "Successful delete should not return an error.") + ts.NoError(err, "Successful delete should not return an error.") s3apiMock.AssertExpectations(ts.T()) } func (ts *fileTestSuite) TestDeleteError() { s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(nil, errors.New("something went wrong")) err := testFile.Delete() - ts.NotNil(err, "Delete should return an error if s3 api had error.") - ts.Equal(err.Error(), "something went wrong") + ts.EqualError(err, "something went wrong", "Delete should return an error if s3 api had error.") s3apiMock.AssertExpectations(ts.T()) } @@ -600,7 +585,7 @@ func (ts *fileTestSuite) TestDeleteWithDeleteAllVersionsOption() { s3apiMock.On("DeleteObject", mock.AnythingOfType("*s3.DeleteObjectInput")).Return(&s3.DeleteObjectOutput{}, nil) err := testFile.Delete(delete.WithDeleteAllVersions()) - ts.Nil(err, "Successful delete should not return an error.") + ts.NoError(err, "Successful delete should not return an error.") s3apiMock.AssertExpectations(ts.T()) s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 3) } @@ -620,7 +605,7 @@ func (ts *fileTestSuite) TestDeleteWithDeleteAllVersionsOptionError() { Return(nil, errors.New("something went wrong")) err := testFile.Delete(delete.WithDeleteAllVersions()) - ts.NotNil(err, "Delete should return an error if s3 api had error.") + ts.Error(err, "Delete should return an error if s3 api had error.") s3apiMock.AssertExpectations(ts.T()) s3apiMock.AssertNumberOfCalls(ts.T(), "DeleteObject", 2) } @@ -631,7 +616,7 @@ func (ts *fileTestSuite) TestLastModified() { LastModified: &now, }, nil) modTime, err := testFile.LastModified() - ts.Nil(err, "Error should be nil when correctly returning time of object.") + ts.NoError(err, "Error should be nil when correctly returning time of object.") ts.Equal(&now, modTime, "Returned time matches expected LastModified time.") } @@ -655,7 +640,7 @@ func (ts *fileTestSuite) TestSize() { }, nil) size, err := testFile.Size() - ts.Nil(err, "Error should be nil when requesting size for file that exists.") + ts.NoError(err, "Error should be nil when requesting size for file that exists.") ts.Equal(uint64(100), size, "Size should return the ContentLength value from s3 HEAD request.") s3apiMock.AssertExpectations(ts.T()) } diff --git a/backend/s3/location_test.go b/backend/s3/location_test.go index 7c5503f9..c2b41eb2 100644 --- a/backend/s3/location_test.go +++ b/backend/s3/location_test.go @@ -47,7 +47,7 @@ func (lt *locationTestSuite) TestList() { loc, err := lt.fs.NewLocation(bucket, locPath) lt.NoError(err) fileList, err := loc.List() - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return the expected number of files.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in expected file list.") @@ -91,7 +91,7 @@ func (lt *locationTestSuite) TestList_pagedCall() { loc, err := lt.fs.NewLocation(bucket, locPath) lt.NoError(err) fileList, err := loc.List() - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return the expected number of files.") for _, expectedKey := range expectedFileList { lt.Contains(fileList, expectedKey, "All returned keys should be in expected file list.") @@ -120,7 +120,7 @@ func (lt *locationTestSuite) TestListByPrefix() { loc, err := lt.fs.NewLocation(bucket, locPath) lt.NoError(err) fileList, err := loc.ListByPrefix(prefix) - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return expected number of file keys.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in the expected list.") @@ -150,7 +150,7 @@ func (lt *locationTestSuite) TestListByRegex() { fileTypeRegex := regexp.MustCompile("txt$") fileList, err := loc.ListByRegex(fileTypeRegex) - lt.Nil(err, "Shouldn't return an error on successful call to ListByRegex") + lt.NoError(err, "Shouldn't return an error on successful call to ListByRegex") lt.Len(fileList, len(expectedFileList), "Should return expected number of file keys.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in the expected list.") @@ -212,7 +212,7 @@ func (lt *locationTestSuite) TestExists_true() { loc, err := lt.fs.NewLocation(bucket, "/") lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(exists, "Call to Exists expected to return true.") lt.s3apiMock.AssertExpectations(lt.T()) } @@ -225,7 +225,7 @@ func (lt *locationTestSuite) TestExists_false() { loc, err := lt.fs.NewLocation(bucket, "/") lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.False(exists, "Call to Exists expected to return true.") lt.s3apiMock.AssertExpectations(lt.T()) } @@ -296,7 +296,7 @@ func (lt *locationTestSuite) TestDeleteFile() { lt.NoError(err) err = loc.DeleteFile("filename.txt") - lt.Nil(err, "Successful delete should not return an error.") + lt.NoError(err, "Successful delete should not return an error.") lt.s3apiMock.AssertExpectations(lt.T()) } @@ -315,7 +315,7 @@ func (lt *locationTestSuite) TestDeleteFileWithDeleteAllVersionsOption() { lt.NoError(err) err = loc.DeleteFile("filename.txt", delete.WithDeleteAllVersions()) - lt.Nil(err, "Successful delete should not return an error.") + lt.NoError(err, "Successful delete should not return an error.") lt.s3apiMock.AssertExpectations(lt.T()) lt.s3apiMock.AssertNumberOfCalls(lt.T(), "DeleteObject", 3) } diff --git a/backend/s3/options_test.go b/backend/s3/options_test.go index dfca179b..0ae6a9fe 100644 --- a/backend/s3/options_test.go +++ b/backend/s3/options_test.go @@ -22,7 +22,7 @@ func (o *optionsTestSuite) TestGetClient() { client, err := getClient(opts) o.NoError(err) o.NotNil(client, "client is set") - o.Equal("", *client.(*s3.S3).Config.Region, "config is empty") + o.Empty(*client.(*s3.S3).Config.Region, "config is empty") // options set opts = Options{ diff --git a/backend/sftp/fileSystem_test.go b/backend/sftp/fileSystem_test.go index ae880522..20ec5e1f 100644 --- a/backend/sftp/fileSystem_test.go +++ b/backend/sftp/fileSystem_test.go @@ -33,7 +33,7 @@ func (ts *fileSystemTestSuite) TestNewFileSystem() { func (ts *fileSystemTestSuite) TestNewFile() { filePath := "/path/to/file.txt" file, err := ts.sftpfs.NewFile("host.com", filePath) - ts.Nil(err, "No errors returned by NewFile(%s)", filePath) + ts.NoError(err, "No errors returned by NewFile(%s)", filePath) ts.NotNil(file, "sftpfs.NewFile(%s) should assign all but first name component to key", filePath) } @@ -121,8 +121,7 @@ func (ts *fileSystemTestSuite) TestClient() { ts.sftpfs.sftpclient = nil ts.sftpfs.options = badOpt _, err = ts.sftpfs.Client(utils.Authority{}) - ts.Error(err, "error found") - ts.Equal("unable to create client, vfs.Options must be an sftp.Options", err.Error(), "client was already set") + ts.EqualError(err, "unable to create client, vfs.Options must be an sftp.Options", "client was already set") } func (ts *fileSystemTestSuite) TestClientWithAutoDisconnect() { diff --git a/backend/sftp/file_test.go b/backend/sftp/file_test.go index f071557c..b8553510 100644 --- a/backend/sftp/file_test.go +++ b/backend/sftp/file_test.go @@ -32,9 +32,7 @@ func (ts *fileTestSuite) SetupTest() { ts.sftpMock = &mocks.Client{} ts.fs = FileSystem{sftpclient: ts.sftpMock, options: Options{}} ts.testFile, err = ts.fs.NewFile("user@host.com:22", "/some/path/to/file.txt") - if err != nil { - ts.Fail("Shouldn't return error creating test sftp.File instance.") - } + ts.Require().NoError(err, "Shouldn't return error creating test sftp.File instance.") } // this wraps strings.Reader to satisfy ReadWriteSeekCloser interface @@ -217,29 +215,25 @@ func (ts *fileTestSuite) Test_openFile() { func (ts *fileTestSuite) TestExists() { sftpfile, err := ts.fs.NewFile("user@host.com", "/path/hello.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") ts.sftpMock.On("MkdirAll", sftpfile.Location().Path()).Return(nil).Once() ts.sftpMock.On("Stat", sftpfile.Path()).Return(nil, nil).Once() exists, err := sftpfile.Exists() ts.True(exists, "Should return true for exists based on this setup") - ts.Nil(err, "Shouldn't return an error when exists is true") + ts.NoError(err, "Shouldn't return an error when exists is true") } func (ts *fileTestSuite) TestNotExists() { sftpfile, err := ts.fs.NewFile("user@host.com", "/path/hello.txt") - if err != nil { - ts.Fail("Shouldn't fail creating new file.") - } + ts.Require().NoError(err, "Shouldn't fail creating new file.") ts.sftpMock.On("MkdirAll", sftpfile.Location().Path()).Return(nil).Once() ts.sftpMock.On("Stat", sftpfile.Path()).Return(nil, os.ErrNotExist).Once() exists, err := sftpfile.Exists() ts.False(exists, "Should return false for exists based on setup") - ts.Nil(err, "Error from key not existing should be hidden since it just confirms it doesn't") + ts.NoError(err, "Error from key not existing should be hidden since it just confirms it doesn't") } func (ts *fileTestSuite) TestCopyToFile() { @@ -290,7 +284,7 @@ func (ts *fileTestSuite) TestCopyToFile() { // run tests err = sourceFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) sourceSftpFile.AssertExpectations(ts.T()) @@ -349,7 +343,7 @@ func (ts *fileTestSuite) TestCopyToFileBuffered() { // run tests err = sourceFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) sourceSftpFile.AssertExpectations(ts.T()) @@ -406,7 +400,7 @@ func (ts *fileTestSuite) TestCopyToFileEmpty() { // run tests err = sourceFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) sourceSftpFile.AssertExpectations(ts.T()) @@ -463,7 +457,7 @@ func (ts *fileTestSuite) TestCopyToFileEmptyBuffered() { // run tests err = sourceFile.CopyToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) sourceSftpFile.AssertExpectations(ts.T()) @@ -521,7 +515,7 @@ func (ts *fileTestSuite) TestCopyToLocation() { // run tests newFile, err := sourceFile.CopyToLocation(targetMockLocation) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") ts.Equal(newFile.URI(), "sftp://user@host2.com:22/some/path.txt", "new file uri check") @@ -579,7 +573,7 @@ func (ts *fileTestSuite) TestMoveToFile_differentAuthority() { // run tests err = sourceFile.MoveToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) sourceSftpFile.AssertExpectations(ts.T()) @@ -629,7 +623,7 @@ func (ts *fileTestSuite) TestMoveToFile_sameAuthority() { // run tests err = sourceFile.MoveToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) targetClient.AssertExpectations(ts.T()) @@ -680,7 +674,7 @@ func (ts *fileTestSuite) TestMoveToFile_fileExists() { // run tests err = sourceFile.MoveToFile(targetFile) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") sourceClient.AssertExpectations(ts.T()) targetClient.AssertExpectations(ts.T()) @@ -737,7 +731,7 @@ func (ts *fileTestSuite) TestMoveToLocation() { // run tests newFile, err := sourceFile.MoveToLocation(targetMockLocation) - ts.Nil(err, "Error shouldn't be returned from successful call to CopyToFile") + ts.NoError(err, "Error shouldn't be returned from successful call to CopyToFile") ts.Equal(newFile.URI(), "sftp://user@host2.com:22/some/other/path.txt", "new file uri check") @@ -859,7 +853,7 @@ func (ts *fileTestSuite) TestTouch() { func (ts *fileTestSuite) TestDelete() { ts.sftpMock.On("Remove", ts.testFile.Path()).Return(nil).Once() err := ts.testFile.Delete() - ts.Nil(err, "Successful delete should not return an error.") + ts.NoError(err, "Successful delete should not return an error.") ts.sftpMock.AssertExpectations(ts.T()) } @@ -869,7 +863,7 @@ func (ts *fileTestSuite) TestLastModified() { file1.On("ModTime").Return(now, nil) ts.sftpMock.On("Stat", ts.testFile.Path()).Return(file1, nil) modTime, err := ts.testFile.LastModified() - ts.Nil(err, "Error should be nil when correctly returning time of object.") + ts.NoError(err, "Error should be nil when correctly returning time of object.") ts.Equal(&now, modTime, "Returned time matches expected LastModified time.") } @@ -893,13 +887,13 @@ func (ts *fileTestSuite) TestSize() { file1.On("Size").Return(contentLength) ts.sftpMock.On("Stat", ts.testFile.Path()).Return(file1, nil).Once() size, err := ts.testFile.Size() - ts.Nil(err, "Error should be nil when requesting size for file that exists.") + ts.NoError(err, "Error should be nil when requesting size for file that exists.") ts.Equal(uint64(contentLength), size, "Size should return the ContentLength value from s3 HEAD request.") ts.sftpMock.On("Stat", ts.testFile.Path()).Return(&mocks.FileInfo{}, errors.New("some error")).Once() size, err = ts.testFile.Size() ts.Error(err, "expect error") - ts.Equal(uint64(0), size, "Size should be 0 on error") + ts.Zero(size, "Size should be 0 on error") ts.sftpMock.AssertExpectations(ts.T()) } diff --git a/backend/sftp/location_test.go b/backend/sftp/location_test.go index 23c5a401..bf021d75 100644 --- a/backend/sftp/location_test.go +++ b/backend/sftp/location_test.go @@ -48,7 +48,7 @@ func (lt *locationTestSuite) TestList() { loc, err := lt.sftpfs.NewLocation(authority, locPath) lt.NoError(err) fileList, err := loc.List() - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return the expected number of files.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in expected file list.") @@ -97,7 +97,7 @@ func (lt *locationTestSuite) TestListByPrefix() { lt.NoError(err) prefix := "fil" fileList, err := loc.ListByPrefix(prefix) - lt.Nil(err, "Shouldn't return an error when successfully returning list.") + lt.NoError(err, "Shouldn't return an error when successfully returning list.") lt.Len(fileList, len(expectedFileList), "Should return expected number of file keys.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in the expected list.") @@ -137,7 +137,7 @@ func (lt *locationTestSuite) TestListByRegex() { fileTypeRegex := regexp.MustCompile("txt$") fileList, err := loc.ListByRegex(fileTypeRegex) - lt.Nil(err, "Shouldn't return an error on successful call to ListByRegex") + lt.NoError(err, "Shouldn't return an error on successful call to ListByRegex") lt.Len(fileList, len(expectedFileList), "Should return expected number of file keys.") for _, fileKey := range fileList { lt.Contains(expectedFileList, fileKey, "All returned keys should be in the expected list.") @@ -244,7 +244,7 @@ func (lt *locationTestSuite) TestExists() { loc, err := lt.sftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err := loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(exists, "Call to Exists expected to return true.") // locations does not exist @@ -254,7 +254,7 @@ func (lt *locationTestSuite) TestExists() { loc, err = lt.sftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err = loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(!exists, "Call to Exists expected to return false.") // some error calling stat @@ -274,7 +274,7 @@ func (lt *locationTestSuite) TestExists() { loc, err = lt.sftpfs.NewLocation(authority, locPath) lt.NoError(err) exists, err = loc.Exists() - lt.Nil(err, "No error expected from Exists") + lt.NoError(err, "No error expected from Exists") lt.True(!exists, "Call to Exists expected to return false.") lt.client.AssertExpectations(lt.T()) diff --git a/backend/sftp/options_test.go b/backend/sftp/options_test.go index 34cec379..b4ef91e3 100644 --- a/backend/sftp/options_test.go +++ b/backend/sftp/options_test.go @@ -87,13 +87,15 @@ func (o *optionsSuite) TestFoundFile() { } for _, t := range tests { - actual, err := foundFile(t.file) - if t.hasError { - o.EqualError(err, t.errMessage, t.message) - } else { - o.NoError(err, t.message) - o.Equal(t.expected, actual, t.message) - } + o.Run(t.message, func() { + actual, err := foundFile(t.file) + if t.hasError { + o.EqualError(err, t.errMessage, t.message) + } else { + o.NoError(err, t.message) + o.Equal(t.expected, actual, t.message) + } + }) } } @@ -149,12 +151,14 @@ func (o *optionsSuite) TestGetKeyFile() { } for _, t := range tests { - _, err := getKeyFile(t.keyfile, t.passphrase) - if t.hasError { - o.EqualError(err, t.errMessage, t.message) - } else { - o.NoError(err, t.message) - } + o.Run(t.message, func() { + _, err := getKeyFile(t.keyfile, t.passphrase) + if t.hasError { + o.EqualError(err, t.errMessage, t.message) + } else { + o.NoError(err, t.message) + } + }) } } @@ -237,25 +241,27 @@ func (o *optionsSuite) TestGetHostKeyCallback() { } // #nosec - InsecureIgnoreHostKey only used for testing for _, t := range tests { //nolint:gocritic // rangeValCopy - // setup env vars, if any - tmpMap := make(map[string]string) - for k, v := range t.envVars { - tmpMap[k] = os.Getenv(k) - o.NoError(os.Setenv(k, v)) - } + 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.NoError(os.Setenv(k, v)) + } - // apply test - _, err := getHostKeyCallback(t.options) - if t.hasError { - o.EqualError(err, t.errMessage, t.message) - } else { - o.NoError(err, t.message) - } + // apply test + _, err := getHostKeyCallback(t.options) + if t.hasError { + o.EqualError(err, t.errMessage, t.message) + } else { + o.NoError(err, t.message) + } - // return env vars to original value - for k, v := range tmpMap { - o.NoError(os.Setenv(k, v)) - } + // return env vars to original value + for k, v := range tmpMap { + o.NoError(os.Setenv(k, v)) + } + }) } } @@ -287,7 +293,7 @@ func (o *optionsSuite) TestGetAuthMethods() { returnCount: 1, hasError: false, errMessage: "", - message: "explicit Options password", + message: "env var password", }, { envVars: map[string]string{ @@ -377,26 +383,28 @@ func (o *optionsSuite) TestGetAuthMethods() { } for _, t := range tests { //nolint:gocritic // rangeValCopy - // setup env vars, if any - tmpMap := make(map[string]string) - for k, v := range t.envVars { - tmpMap[k] = os.Getenv(k) - o.NoError(os.Setenv(k, v)) - } + 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.NoError(os.Setenv(k, v)) + } - // apply test - auth, err := getAuthMethods(t.options) - if t.hasError { - o.EqualError(err, t.errMessage, t.message) - } else { - o.NoError(err, t.message) - o.Equal(t.returnCount, len(auth), "auth count") - } + // apply test + auth, err := getAuthMethods(t.options) + if t.hasError { + o.EqualError(err, t.errMessage, t.message) + } else { + o.NoError(err, t.message) + o.Len(auth, t.returnCount, "auth count") + } - // return env vars to original value - for k, v := range tmpMap { - o.NoError(os.Setenv(k, v)) - } + // return env vars to original value + for k, v := range tmpMap { + o.NoError(os.Setenv(k, v)) + } + }) } } @@ -447,16 +455,17 @@ func (o *optionsSuite) TestGetClient() { } // #nosec - InsecureIgnoreHostKey only used for testing for _, t := range tests { //nolint:gocritic // rangeValCopy - // apply test - _, _, err := getClient(t.authority, t.options) - if t.hasError { - if o.Error(err, "error found") { - re := regexp.MustCompile(t.errRegex) - o.Regexp(re, err.Error(), "error matches") + o.Run(t.message, func() { + _, _, err := getClient(t.authority, t.options) + if t.hasError { + if o.Error(err, "error found") { + re := regexp.MustCompile(t.errRegex) + o.Regexp(re, err.Error(), "error matches") + } + } else { + o.NoError(err, t.message) } - } else { - o.NoError(err, t.message) - } + }) } } @@ -471,10 +480,10 @@ func (o *optionsSuite) TestMarshalOptions() { } raw, err := json.Marshal(opts) - o.Nil(err) + o.NoError(err) optStruct := &Options{} err = json.Unmarshal(raw, optStruct) - o.Nil(err) + o.NoError(err) o.Equal(kh, optStruct.KeyFilePath, "KeyFilePath check") o.Equal(pw, optStruct.Password, "Password check") diff --git a/backend/testsuite/backend_integration_test.go b/backend/testsuite/backend_integration_test.go index 7e868812..19c248ba 100644 --- a/backend/testsuite/backend_integration_test.go +++ b/backend/testsuite/backend_integration_test.go @@ -153,9 +153,7 @@ func (s *vfsTestSuite) Location(baseLoc vfs.Location) { // clean up srcLoc after test for OS if srcLoc.FileSystem().Scheme() == "file" { exists, err := srcLoc.Exists() - if err != nil { - panic(err) - } + s.Require().NoError(err) if exists { s.NoError(os.RemoveAll(srcLoc.Path()), "failed to clean up location test srcLoc") } @@ -323,16 +321,16 @@ func (s *vfsTestSuite) Location(baseLoc vfs.Location) { files, err := srcLoc.List() s.NoError(err) - s.Equal(3, len(files), "list srcLoc location") + s.Len(files, 3, "list srcLoc location") files, err = subLoc.List() s.NoError(err) - s.Equal(1, len(files), "list subLoc location") + s.Len(files, 1, "list subLoc location") s.Equal("that.txt", files[0], "returned basename") files, err = cdTestLoc.List() s.NoError(err) - s.Equal(0, len(files), "non-existent location") + s.Empty(files, "non-existent location") switch baseLoc.FileSystem().Scheme() { case "gs": @@ -353,21 +351,21 @@ func (s *vfsTestSuite) Location(baseLoc vfs.Location) { files, err = srcLoc.ListByPrefix("file") s.NoError(err) - s.Equal(2, len(files), "list srcLoc location matching prefix") + s.Len(files, 2, "list srcLoc location matching prefix") files, err = srcLoc.ListByPrefix("s") s.NoError(err) - s.Equal(1, len(files), "list srcLoc location") + s.Len(files, 1, "list srcLoc location") s.Equal("self.txt", files[0], "returned only file basename, not subdir matching prefix") files, err = srcLoc.ListByPrefix("somepath/t") s.NoError(err) - s.Equal(1, len(files), "list 'somepath' location relative to srcLoc") + s.Len(files, 1, "list 'somepath' location relative to srcLoc") s.Equal("that.txt", files[0], "returned only file basename, using relative prefix") files, err = cdTestLoc.List() s.NoError(err) - s.Equal(0, len(files), "non-existent location") + s.Empty(files, "non-existent location") // ListByRegex returns a slice of strings representing the base names of the files found in the Location that matched the // given regular expression. @@ -379,15 +377,15 @@ func (s *vfsTestSuite) Location(baseLoc vfs.Location) { files, err = srcLoc.ListByRegex(regexp.MustCompile("^f")) s.NoError(err) - s.Equal(2, len(files), "list srcLoc location matching prefix") + s.Len(files, 2, "list srcLoc location matching prefix") files, err = srcLoc.ListByRegex(regexp.MustCompile(`.txt$`)) s.NoError(err) - s.Equal(3, len(files), "list srcLoc location matching prefix") + s.Len(files, 3, "list srcLoc location matching prefix") files, err = srcLoc.ListByRegex(regexp.MustCompile(`Z`)) s.NoError(err) - s.Equal(0, len(files), "list srcLoc location matching prefix") + s.Empty(files, "list srcLoc location matching prefix") // DeleteFile deletes the file of the given name at the location. // @@ -416,9 +414,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { // clean up srcLoc after test for OS if srcLoc.FileSystem().Scheme() == "file" { exists, err := srcLoc.Exists() - if err != nil { - panic(err) - } + s.Require().NoError(err) if exists { s.NoError(os.RemoveAll(srcLoc.Path()), "failed to clean up file test srcLoc") } @@ -542,9 +538,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { // clean up dstLoc after test for OS if dstLoc.FileSystem().Scheme() == "file" { exists, err := dstLoc.Exists() - if err != nil { - panic(err) - } + s.Require().NoError(err) if exists { s.NoError(os.RemoveAll(dstLoc.Path()), "failed to clean up file test dstLoc") } @@ -742,50 +736,51 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { {Path: "path%20has/", Filename: "encodedSpace.txt"}, } - for _, test := range tests { - // setup src - srcSpaces, err := srcLoc.NewFile(path.Join(test.Path, test.Filename)) - s.NoError(err) - b, err := srcSpaces.Write([]byte("something")) - s.NoError(err) - s.Equal(9, b, "byte count is correct") - err = srcSpaces.Close() - s.NoError(err) - - testDestLoc, err := dstLoc.NewLocation(test.Path) - s.NoError(err) - - dstSpaces, err := srcSpaces.MoveToLocation(testDestLoc) - s.NoError(err) - exists, err := dstSpaces.Exists() - s.NoError(err) - s.True(exists, "dstSpaces should now exist") - exists, err = srcSpaces.Exists() - s.NoError(err) - s.False(exists, "srcSpaces should no longer exist") - s.True( - strings.HasSuffix(dstSpaces.URI(), path.Join(test.Path, test.Filename)), - "destination file %s ends with source string for %s", dstSpaces.URI(), path.Join(test.Path, test.Filename), - ) - - newSrcSpaces, err := dstSpaces.MoveToLocation(srcSpaces.Location()) - s.NoError(err) - exists, err = newSrcSpaces.Exists() - s.NoError(err) - s.True(exists, "newSrcSpaces should now exist") - exists, err = dstSpaces.Exists() - s.NoError(err) - s.False(exists, "dstSpaces should no longer exist") - hasSuffix := strings.HasSuffix(newSrcSpaces.URI(), path.Join(test.Path, test.Filename)) - s.True(hasSuffix, "destination file %s ends with source string for %s", dstSpaces.URI(), path.Join(test.Path, test.Filename)) - - err = newSrcSpaces.Delete() - s.NoError(err) - exists, err = newSrcSpaces.Exists() - s.NoError(err) - s.False(exists, "newSrcSpaces should now exist") + for i, test := range tests { + s.Run(fmt.Sprintf("%d", i), func() { + // setup src + srcSpaces, err := srcLoc.NewFile(path.Join(test.Path, test.Filename)) + s.NoError(err) + b, err := srcSpaces.Write([]byte("something")) + s.NoError(err) + s.Equal(9, b, "byte count is correct") + err = srcSpaces.Close() + s.NoError(err) + + testDestLoc, err := dstLoc.NewLocation(test.Path) + s.NoError(err) + + dstSpaces, err := srcSpaces.MoveToLocation(testDestLoc) + s.NoError(err) + exists, err := dstSpaces.Exists() + s.NoError(err) + s.True(exists, "dstSpaces should now exist") + exists, err = srcSpaces.Exists() + s.NoError(err) + s.False(exists, "srcSpaces should no longer exist") + s.True( + strings.HasSuffix(dstSpaces.URI(), path.Join(test.Path, test.Filename)), + "destination file %s ends with source string for %s", dstSpaces.URI(), path.Join(test.Path, test.Filename), + ) + + newSrcSpaces, err := dstSpaces.MoveToLocation(srcSpaces.Location()) + s.NoError(err) + exists, err = newSrcSpaces.Exists() + s.NoError(err) + s.True(exists, "newSrcSpaces should now exist") + exists, err = dstSpaces.Exists() + s.NoError(err) + s.False(exists, "dstSpaces should no longer exist") + hasSuffix := strings.HasSuffix(newSrcSpaces.URI(), path.Join(test.Path, test.Filename)) + s.True(hasSuffix, "destination file %s ends with source string for %s", dstSpaces.URI(), path.Join(test.Path, test.Filename)) + + err = newSrcSpaces.Delete() + s.NoError(err) + exists, err = newSrcSpaces.Exists() + s.NoError(err) + s.False(exists, "newSrcSpaces should now exist") + }) } - } // Touch creates a zero-length file on the vfs.File if no File exists. Update File's last modified timestamp. @@ -806,7 +801,7 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { size, err := touchedFile.Size() s.NoError(err) - s.Equal(uint64(0), size, "%s should be empty", touchedFile) + s.Zero(size, "%s should be empty", touchedFile) // capture last modified modified, err := touchedFile.LastModified() @@ -842,14 +837,14 @@ func (s *vfsTestSuite) File(baseLoc vfs.Location) { size, err = srcFile.Size() s.Error(err, "expected error because file does not exist") - s.Equal(uint64(0x0), size) + s.Zero(size) _, err = srcFile.LastModified() s.Error(err, "expected error because file does not exist") seeked, err := srcFile.Seek(-1, 2) s.Error(err, "expected error because file does not exist") - s.Equal(int64(0x0), seeked) + s.Zero(seeked) _, err = srcFile.Read(make([]byte, 1)) s.Error(err, "expected error because file does not exist") @@ -911,7 +906,7 @@ func (s *vfsTestSuite) gsList(baseLoc vfs.Location) { files, err := f.Location().List() s.NoError(err) - s.Equal(1, len(files), "check file count found") + s.Len(len(files), 1, "check file count found") s.Equal("file.txt", files[0], "file.txt was found") // CLEAN UP diff --git a/backend/testsuite/io_integration_test.go b/backend/testsuite/io_integration_test.go index d305de8d..4afb806e 100644 --- a/backend/testsuite/io_integration_test.go +++ b/backend/testsuite/io_integration_test.go @@ -184,12 +184,12 @@ func (s *ioTestSuite) SetupSuite() { func (s *ioTestSuite) TestFileOperations() { if s.localDir != "" { - s.T().Run("local", func(t *testing.T) { + s.Run("local", func() { s.testFileOperations(s.localDir) }) } for scheme, location := range s.testLocations { - s.T().Run(scheme, func(t *testing.T) { + s.Run(scheme, func() { s.testFileOperations(location.URI()) }) } @@ -367,24 +367,22 @@ func (s *ioTestSuite) testFileOperations(testPath string) { _ = file.Delete() } }() - if err != nil { - s.T().Fatalf("expected failure did not match err: %s", err.Error()) - } + s.Require().NoError(err) // Use vfs to execute the sequence of operations described by the description actualContents, err := executeSequence(s.T(), file, tc.sequence) // Implement this function // Assert expected outcomes if tc.expectFailure && err == nil { - s.T().Errorf("%s: expected failure but got success", tc.description) + s.Failf("%s: expected failure but got success", tc.description) } if err != nil && !tc.expectFailure { - s.T().Errorf("%s: expected success but got failure: %v", tc.description, err) + s.Failf("%s: expected success but got failure: %v", tc.description, err) } if tc.expectedResults != actualContents { - s.T().Errorf("%s: expected results %s but got %s", tc.description, tc.expectedResults, actualContents) + s.Failf("%s: expected results %s but got %s", tc.description, tc.expectedResults, actualContents) } }() }) diff --git a/utils/utils_test.go b/utils/utils_test.go index ff3fd5ac..0e1510b0 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -610,8 +610,7 @@ func (s *utilsSuite) TestPathToURI() { // parse path with null character _, err := utils.PathToURI(fmt.Sprintf("/some%s/path/", string(nullChar))) - s.Error(err, "expected error on ctrl char in path") - s.EqualError(err, "parse \"/some\\x00/path/\": net/url: invalid control character in URL") + s.EqualError(err, "parse \"/some\\x00/path/\": net/url: invalid control character in URL", "expected error on ctrl char in path") } func (s *utilsSuite) TestGetURI() { @@ -654,9 +653,7 @@ func (s *utilsSuite) TestTouchCopy() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") - if err != nil { - s.NoError(err, "unexpected temp file setup error") - } + s.NoError(err, "unexpected temp file setup error") defer func() { err := os.Remove(tmpfile.Name()) if err != nil { @@ -664,12 +661,10 @@ func (s *utilsSuite) TestTouchCopy() { } }() - if _, err := tmpfile.Write([]byte{}); err != nil { - s.NoError(err, "unexpected temp file writing error") - } - if err := tmpfile.Close(); err != nil { - s.NoError(err, "unexpected temp file close error") - } + _, err = tmpfile.Write([]byte{}) + s.NoError(err, "unexpected temp file writing error") + err = tmpfile.Close() + s.NoError(err, "unexpected temp file close error") // setup reader vfs.File osfs := _os.FileSystem{} @@ -683,12 +678,12 @@ func (s *utilsSuite) TestTouchCopy() { buffer := make([]byte, utils.TouchCopyMinBufferSize) byteCount, err := io.CopyBuffer(writer, reader, buffer) s.NoError(err, "unexpected doing io.Copy") - s.Equal(int64(0), byteCount, "should be no content") + s.Zero(byteCount, "should be no content") // writer file should not exist _, err = os.Stat(writer.Path()) s.Error(err, "should have failed stat") - s.True(os.IsNotExist(err), "should be true: not exists") + s.ErrorIs(err, os.ErrNotExist, "should be not exists") // now with TouchCopy _, err = reader.Seek(0, 0) // reset reader @@ -707,7 +702,7 @@ func (s *utilsSuite) TestTouchCopy() { fi, err := os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") if fi != nil { - s.Equal(int64(0), fi.Size(), "file should be zero length") + s.Zero(fi.Size(), "file should be zero length") } // TouchCopy on file that actually has data @@ -720,7 +715,7 @@ func (s *utilsSuite) TestTouchCopy() { s.NoError(err, "unexpected error running TouchCopy()") fi, err = os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") - s.NotEqual(fi, 0, "file should have a non-zero byte size") + s.NotZero(fi, "file should have a non-zero byte size") // TouchCopy should fail on a reader.Size() error nonexistentFile := path.Join(writer.Path(), "nonexistent.file") @@ -735,9 +730,7 @@ func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") - if err != nil { - s.NoError(err, "unexpected temp file setup error") - } + s.NoError(err, "unexpected temp file setup error") defer func() { err := os.Remove(tmpfile.Name()) if err != nil { @@ -745,12 +738,10 @@ func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { } }() - if _, err := tmpfile.Write([]byte{}); err != nil { - s.NoError(err, "unexpected temp file writing error") - } - if err := tmpfile.Close(); err != nil { - s.NoError(err, "unexpected temp file close error") - } + _, err = tmpfile.Write([]byte{}) + s.NoError(err, "unexpected temp file writing error") + err = tmpfile.Close() + s.NoError(err, "unexpected temp file close error") // setup reader vfs.File osfs := _os.FileSystem{} @@ -764,12 +755,12 @@ func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { buffer := make([]byte, utils.TouchCopyMinBufferSize) byteCount, err := io.CopyBuffer(writer, reader, buffer) s.NoError(err, "unexpected doing io.Copy") - s.Equal(int64(0), byteCount, "should be no content") + s.Zero(byteCount, "should be no content") // writer file should not exist _, err = os.Stat(writer.Path()) s.Error(err, "should have failed stat") - s.True(os.IsNotExist(err), "should be true: not exists") + s.ErrorIs(err, os.ErrNotExist, "should be not exists") // now with TouchCopyBuffered _, err = reader.Seek(0, 0) // reset reader @@ -788,7 +779,7 @@ func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { fi, err := os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") if fi != nil { - s.Equal(int64(0), fi.Size(), "file should be zero length") + s.Zero(fi.Size(), "file should be zero length") } // TouchCopyBuffered on file that actually has data @@ -801,7 +792,7 @@ func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { s.NoError(err, "unexpected error running TouchCopyBuffered()") fi, err = os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") - s.NotEqual(fi, 0, "file should have a non-zero byte size") + s.NotZero(fi, "file should have a non-zero byte size") // TouchCopyBuffered should fail on a reader.Size() error nonexistentFile := path.Join(writer.Path(), "nonexistent.file") @@ -816,9 +807,7 @@ func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") - if err != nil { - s.NoError(err, "unexpected temp file setup error") - } + s.NoError(err, "unexpected temp file setup error") defer func() { err := os.Remove(tmpfile.Name()) if err != nil { @@ -826,12 +815,10 @@ func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { } }() - if _, err := tmpfile.Write([]byte{}); err != nil { - s.NoError(err, "unexpected temp file writing error") - } - if err := tmpfile.Close(); err != nil { - s.NoError(err, "unexpected temp file close error") - } + _, err = tmpfile.Write([]byte{}) + s.NoError(err, "unexpected temp file writing error") + err = tmpfile.Close() + s.NoError(err, "unexpected temp file close error") // setup reader vfs.File osfs := _os.FileSystem{} @@ -845,12 +832,12 @@ func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { buffer := make([]byte, utils.TouchCopyMinBufferSize) byteCount, err := io.CopyBuffer(writer, reader, buffer) s.NoError(err, "unexpected doing io.Copy") - s.Equal(int64(0), byteCount, "should be no content") + s.Zero(byteCount, "should be no content") // writer file should not exist _, err = os.Stat(writer.Path()) s.Error(err, "should have failed stat") - s.True(os.IsNotExist(err), "should be true: not exists") + s.ErrorIs(err, os.ErrNotExist, "should be not exists") // now with TouchCopyBuffered _, err = reader.Seek(0, 0) // reset reader @@ -869,7 +856,7 @@ func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { fi, err := os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") if fi != nil { - s.Equal(int64(0), fi.Size(), "file should be zero length") + s.Zero(fi.Size(), "file should be zero length") } // TouchCopyBuffered on file that actually has data @@ -882,7 +869,7 @@ func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { s.NoError(err, "unexpected error running TouchCopyBuffered()") fi, err = os.Stat(writer.Path()) s.NoError(err, "file should exist, so no error") - s.NotEqual(fi, 0, "file should have a non-zero byte size") + s.NotZero(fi, "file should have a non-zero byte size") // TouchCopyBuffered should fail on a reader.Size() error nonexistentFile := path.Join(writer.Path(), "nonexistent.file") diff --git a/vfssimple/vfssimple_test.go b/vfssimple/vfssimple_test.go index 116a4165..2178a4f5 100644 --- a/vfssimple/vfssimple_test.go +++ b/vfssimple/vfssimple_test.go @@ -275,34 +275,36 @@ func (s *vfsSimpleSuite) TestParseSupportedURI() { } for _, test := range tests { - fs, authority, path, err := parseSupportedURI(test.uri) - if test.err != nil { - s.Error(err, test.message) - if errors.Is(err, test.err) { - s.True(errors.Is(err, test.err), test.message) + s.Run(test.message, func() { + fs, authority, path, err := parseSupportedURI(test.uri) + if test.err != nil { + s.Error(err, test.message) + if errors.Is(err, test.err) { + s.True(errors.Is(err, test.err), test.message) + } else { + // this is necessary since we can't recreate sentinel errors from url.Parse() to do errors.Is() comparison + s.Contains(err.Error(), test.err.Error(), test.message) + } } else { - // this is necessary since we can't recreate sentinel errors from url.Parse() to do errors.Is() comparison - s.Contains(err.Error(), test.err.Error(), test.message) - } - } else { - s.NoError(err, test.message) - s.Equal(test.scheme, fs.Scheme(), test.message) - s.Equal(test.authority, authority, test.message) - s.Equal(test.path, path, test.message) - // check client for named registered mock - switch fs.Scheme() { - case "s3": - s3api, err := fs.(*s3.FileSystem).Client() s.NoError(err, test.message) - if c, ok := s3api.(*namedS3ClientMock); ok { - s.Equal(c.RegName, test.regFS, test.message) - } else { - s.Fail("should have returned mock", test.message) + s.Equal(test.scheme, fs.Scheme(), test.message) + s.Equal(test.authority, authority, test.message) + s.Equal(test.path, path, test.message) + // check client for named registered mock + switch fs.Scheme() { + case "s3": + s3api, err := fs.(*s3.FileSystem).Client() + s.NoError(err, test.message) + if c, ok := s3api.(*namedS3ClientMock); ok { + s.Equal(c.RegName, test.regFS, test.message) + } else { + s.Fail("should have returned mock", test.message) + } + default: + s.Fail("we should have a case for returned fs type", test.message) } - default: - s.Fail("we should have a case for returned fs type", test.message) } - } + }) } } func (s *vfsSimpleSuite) TestNewFile() {