Merge branch 'main' into typos

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

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

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)

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

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

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

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

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("[email protected]: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,17 +161,17 @@ 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")
 	fakeDataConn.AssertWriteErr(myWriteErr)
 	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())
 }

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