diff --git a/backend/mem/file_test.go b/backend/mem/file_test.go index b36b82a7..9426cc40 100644 --- a/backend/mem/file_test.go +++ b/backend/mem/file_test.go @@ -369,6 +369,7 @@ func (s *memFileTest) TestCopyToLocationOS() { s.NoError(err, "unexpected error creating osFile") _, err = osFile.Write(make([]byte, 0)) s.NoError(err, "unexpected error writing zero bytes to osFile") + s.NoError(osFile.Close()) exists, err := osFile.Exists() s.NoError(err, "unexpected existence error") diff --git a/backend/os/file.go b/backend/os/file.go index 668293cb..41fc81d9 100644 --- a/backend/os/file.go +++ b/backend/os/file.go @@ -52,12 +52,19 @@ func (f *File) LastModified() (*time.Time, error) { return &statsTime, err } -// Name returns the full name of the File relative to Location.Name(). +// Name returns the base name of the file path. +// +// For `file:///some/path/to/file.txt`, it would return `file.txt` func (f *File) Name() string { return path.Base(f.name) } -// Path returns the the path of the File relative to Location.Name(). +// Path returns absolute path, including filename, +// For `file:///some/path/to/file.txt`, it would return `/some/path/to/file.txt` +// +// If the directory portion of a file is desired, call +// +// someFile.Location().Path() func (f *File) Path() string { return filepath.Join(f.Location().Path(), f.Name()) } @@ -143,7 +150,7 @@ func (f *File) Read(p []byte) (int, error) { // the file, 1 means relative to the current offset, and 2 means relative to the end. It returns the new offset and // an error, if any. func (f *File) Seek(offset int64, whence int) (int64, error) { - // when writing, we first write to a temp file which ensures a file isn't created before we call clase. + // when writing, we first write to a temp file which ensures a file isn't created before we call close. // However, if we've never written AND the original file doesn't exist, we can't seek. exists, err := f.Exists() if err != nil { @@ -386,7 +393,7 @@ func openOSFile(filePath string) (*os.File, error) { // Ensure the path exists before opening the file, NoOp if dir already exists. var fileMode os.FileMode = 0666 - if err := os.MkdirAll(path.Dir(filePath), os.ModeDir|0777); err != nil { + if err := os.MkdirAll(path.Dir(filePath), os.ModeDir|0750); err != nil { return nil, err } @@ -457,7 +464,7 @@ func (f *File) copyToLocalTempReader() (*os.File, error) { // So imagine we have a file with content "hello world" and we call Seek(6, 0) and then Write([]byte("there")), the // temp file should have "hello there" and not "there". Then finally when Close is called, the temp file is renamed // to the original file. This code ensures that scenario works as expected. - if exists { + if exists && (f.seekCalled || f.readCalled) { openFunc := openOSFile if f.fileOpener != nil { openFunc = f.fileOpener @@ -467,10 +474,8 @@ func (f *File) copyToLocalTempReader() (*os.File, error) { if err != nil { return nil, err } - if f.seekCalled || f.readCalled { - if _, err := io.Copy(tmpFile, actualFile); err != nil { - return nil, err - } + if _, err := io.Copy(tmpFile, actualFile); err != nil { + return nil, err } if f.cursorPos > 0 { diff --git a/backend/os/file_test.go b/backend/os/file_test.go index d7992011..bd36e727 100644 --- a/backend/os/file_test.go +++ b/backend/os/file_test.go @@ -507,7 +507,8 @@ func (s *osFileTest) TestWrite() { f, err := s.tmploc.NewFile("test_files/writeFileFail.txt") s.NoError(err) s.NoError(f.Touch()) - f.(*File).useTempFile = true + _, err = f.Seek(0, 0) + s.NoError(err) f.(*File).fileOpener = func(filePath string) (*os.File, error) { return nil, errors.New("bad opener") } data = make([]byte, 4) _, err = f.Write(data) @@ -545,7 +546,7 @@ func (s *osFileTest) TestCursor() { s.Equal(int64(8), file.(*File).cursorPos) s.Equal(3, sz) - _, serr = file.Seek(5, 0) // cursor 0 - in temp file + _, serr = file.Seek(5, 0) // cursor 5 - in temp file s.Equal(int64(5), file.(*File).cursorPos) s.NoError(serr) diff --git a/utils/authority_test.go b/utils/authority_test.go index ebc26dc5..513f7288 100644 --- a/utils/authority_test.go +++ b/utils/authority_test.go @@ -258,6 +258,6 @@ func (a *authoritySuite) TestAuthority() { } } -func TestUtils(t *testing.T) { +func TestAuthority(t *testing.T) { suite.Run(t, new(authoritySuite)) } diff --git a/utils/utils_test.go b/utils/utils_test.go index 8e3270d1..8eb0f7ee 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -19,7 +19,7 @@ import ( ************TESTS***************** **********************************/ -type utilsTest struct { +type utilsSuite struct { suite.Suite } @@ -29,7 +29,7 @@ type slashTest struct { message string } -func (s *utilsTest) TestEnsureTrailingSlash() { +func (s *utilsSuite) TestEnsureTrailingSlash() { tests := []slashTest{ { path: "/some/path", @@ -59,11 +59,13 @@ func (s *utilsTest) TestEnsureTrailingSlash() { } for _, slashtest := range tests { - s.Equal(slashtest.expected, utils.EnsureTrailingSlash(slashtest.path), slashtest.message) + s.Run(slashtest.message, func() { + s.Equal(slashtest.expected, utils.EnsureTrailingSlash(slashtest.path), slashtest.message) + }) } } -func (s *utilsTest) TestEnsureLeadingSlash() { +func (s *utilsSuite) TestEnsureLeadingSlash() { tests := []slashTest{ { path: "some/path/", @@ -88,11 +90,13 @@ func (s *utilsTest) TestEnsureLeadingSlash() { } for _, slashtest := range tests { - s.Equal(slashtest.expected, utils.EnsureLeadingSlash(slashtest.path), slashtest.message) + s.Run(slashtest.message, func() { + s.Equal(slashtest.expected, utils.EnsureLeadingSlash(slashtest.path), slashtest.message) + }) } } -func (s *utilsTest) TestRemoveTrailingSlash() { +func (s *utilsSuite) TestRemoveTrailingSlash() { tests := []slashTest{ { path: "/some/path", @@ -122,11 +126,13 @@ func (s *utilsTest) TestRemoveTrailingSlash() { } for _, slashtest := range tests { - s.Equal(slashtest.expected, utils.RemoveTrailingSlash(slashtest.path), slashtest.message) + s.Run(slashtest.message, func() { + s.Equal(slashtest.expected, utils.RemoveTrailingSlash(slashtest.path), slashtest.message) + }) } } -func (s *utilsTest) TestRemoveLeadingSlash() { +func (s *utilsSuite) TestRemoveLeadingSlash() { tests := []slashTest{ { path: "some/path/", @@ -156,7 +162,9 @@ func (s *utilsTest) TestRemoveLeadingSlash() { } for _, slashtest := range tests { - s.Equal(slashtest.expected, utils.RemoveLeadingSlash(slashtest.path), slashtest.message) + s.Run(slashtest.message, func() { + s.Equal(slashtest.expected, utils.RemoveLeadingSlash(slashtest.path), slashtest.message) + }) } } @@ -166,7 +174,7 @@ type pathValidationTest struct { message string } -func (s *utilsTest) TestValidateAbsFilePath() { +func (s *utilsSuite) TestValidateAbsFilePath() { tests := []pathValidationTest{ { path: "/some/path/", @@ -221,16 +229,18 @@ func (s *utilsTest) TestValidateAbsFilePath() { } for _, validationTest := range tests { - err := utils.ValidateAbsoluteFilePath(validationTest.path) - if !validationTest.passExpected { - s.EqualError(err, utils.ErrBadAbsFilePath, validationTest.message) - } else { - s.NoError(err, validationTest.message) - } + s.Run(validationTest.message, func() { + err := utils.ValidateAbsoluteFilePath(validationTest.path) + if !validationTest.passExpected { + s.EqualError(err, utils.ErrBadAbsFilePath, validationTest.message) + } else { + s.NoError(err, validationTest.message) + } + }) } } -func (s *utilsTest) TestValidateAbsLocationPath() { +func (s *utilsSuite) TestValidateAbsLocationPath() { tests := []pathValidationTest{ { path: "/some/path/", @@ -285,16 +295,18 @@ func (s *utilsTest) TestValidateAbsLocationPath() { } for _, validationTest := range tests { - err := utils.ValidateAbsoluteLocationPath(validationTest.path) - if !validationTest.passExpected { - s.EqualError(err, utils.ErrBadAbsLocationPath, validationTest.message) - } else { - s.NoError(err, validationTest.message) - } + s.Run(validationTest.message, func() { + err := utils.ValidateAbsoluteLocationPath(validationTest.path) + if !validationTest.passExpected { + s.EqualError(err, utils.ErrBadAbsLocationPath, validationTest.message) + } else { + s.NoError(err, validationTest.message) + } + }) } } -func (s *utilsTest) TestValidateRelFilePath() { +func (s *utilsSuite) TestValidateRelFilePath() { tests := []pathValidationTest{ { path: "/some/path/", @@ -349,16 +361,18 @@ func (s *utilsTest) TestValidateRelFilePath() { } for _, validationTest := range tests { - err := utils.ValidateRelativeFilePath(validationTest.path) - if !validationTest.passExpected { - s.EqualError(err, utils.ErrBadRelFilePath, validationTest.message) - } else { - s.NoError(err, validationTest.message) - } + s.Run(validationTest.message, func() { + err := utils.ValidateRelativeFilePath(validationTest.path) + if !validationTest.passExpected { + s.EqualError(err, utils.ErrBadRelFilePath, validationTest.message) + } else { + s.NoError(err, validationTest.message) + } + }) } } -func (s *utilsTest) TestValidateRelLocationPath() { +func (s *utilsSuite) TestValidateRelLocationPath() { tests := []pathValidationTest{ { path: "/some/path/", @@ -413,16 +427,18 @@ func (s *utilsTest) TestValidateRelLocationPath() { } for _, validationTest := range tests { - err := utils.ValidateRelativeLocationPath(validationTest.path) - if !validationTest.passExpected { - s.EqualError(err, utils.ErrBadRelLocationPath, validationTest.message) - } else { - s.NoError(err, validationTest.message) - } + s.Run(validationTest.message, func() { + err := utils.ValidateRelativeLocationPath(validationTest.path) + if !validationTest.passExpected { + s.EqualError(err, utils.ErrBadRelLocationPath, validationTest.message) + } else { + s.NoError(err, validationTest.message) + } + }) } } -func (s *utilsTest) TestValidatePrefix() { +func (s *utilsSuite) TestValidatePrefix() { tests := []struct { prefix string passExpected bool @@ -501,12 +517,14 @@ func (s *utilsTest) TestValidatePrefix() { } for _, validationTest := range tests { - err := utils.ValidatePrefix(validationTest.prefix) - if !validationTest.passExpected { - s.EqualError(err, utils.ErrBadPrefix, validationTest.message) - } else { - s.NoError(err, validationTest.message) - } + s.Run(validationTest.message, func() { + err := utils.ValidatePrefix(validationTest.prefix) + if !validationTest.passExpected { + s.EqualError(err, utils.ErrBadPrefix, validationTest.message) + } else { + s.NoError(err, validationTest.message) + } + }) } } @@ -517,7 +535,7 @@ type URITest struct { isRegex bool } -func (s *utilsTest) TestPathToURI() { +func (s *utilsSuite) TestPathToURI() { tests := []URITest{ { path: "/absolute/path/", @@ -575,24 +593,27 @@ func (s *utilsTest) TestPathToURI() { } for _, slashtest := range tests { - uri, err := utils.PathToURI(slashtest.path) - s.Require().NoError(err, "no error expected") - if slashtest.isRegex { - s.Regexp(slashtest.expected, uri, slashtest.message) - } else { - s.Equal(slashtest.expected, uri, slashtest.message) - } + s.Run(slashtest.message, func() { + uri, err := utils.PathToURI(slashtest.path) + s.Require().NoError(err, "no error expected") + if slashtest.isRegex { + s.Regexp(slashtest.expected, uri, slashtest.message) + } else { + s.Equal(slashtest.expected, uri, slashtest.message) + } + }) } // test error return from bad uri.parse const nullChar = '\u0000' + // 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") } -func (s *utilsTest) TestGetURI() { +func (s *utilsSuite) TestGetURI() { // set up mocks mockFs1 := new(mocks.FileSystem) @@ -628,7 +649,7 @@ func (s *utilsTest) TestGetURI() { s.Equal("s3://mybucket/this/path/to/", utils.GetLocationURI(mockLoc2), "s3 location uri matches ") } -func (s *utilsTest) TestTouchCopy() { +func (s *utilsSuite) TestTouchCopy() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") @@ -679,6 +700,7 @@ func (s *utilsTest) TestTouchCopy() { panic(err) } }() + s.NoError(writer.Close()) // writer file should exist fi, err := os.Stat(writer.Path()) @@ -708,7 +730,7 @@ func (s *utilsTest) TestTouchCopy() { } -func (s *utilsTest) TestTouchCopyBufferedDefaultBufferSize() { +func (s *utilsSuite) TestTouchCopyBufferedDefaultBufferSize() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") @@ -759,6 +781,7 @@ func (s *utilsTest) TestTouchCopyBufferedDefaultBufferSize() { panic(err) } }() + s.NoError(writer.Close()) // writer file should exist fi, err := os.Stat(writer.Path()) @@ -788,7 +811,7 @@ func (s *utilsTest) TestTouchCopyBufferedDefaultBufferSize() { } -func (s *utilsTest) TestTouchCopyBufferedNonDefaultBufferSize() { +func (s *utilsSuite) TestTouchCopyBufferedNonDefaultBufferSize() { // write out blank file tmpfile, err := os.CreateTemp("", "utils_test") @@ -839,6 +862,7 @@ func (s *utilsTest) TestTouchCopyBufferedNonDefaultBufferSize() { panic(err) } }() + s.NoError(writer.Close()) // writer file should exist fi, err := os.Stat(writer.Path()) @@ -869,5 +893,5 @@ func (s *utilsTest) TestTouchCopyBufferedNonDefaultBufferSize() { } func TestUtils(t *testing.T) { - suite.Run(t, new(utilsTest)) + suite.Run(t, new(utilsSuite)) }