From cf9ba7b88f48afcf4d4ed02315734964342ca2ff Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 28 Jul 2016 21:57:11 -0700 Subject: [PATCH] tests: Add missing unit test for posix.ReadFile. (#2319) --- .travis.yml | 5 +- fs-v1.go | 6 +- posix.go | 9 ++- posix_test.go | 198 ++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 203 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5fe785c99..9cab4601a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,10 +4,9 @@ language: go os: - linux -# Enable OSX builds when travis service is back. -#- osx +- osx -#osx_image: xcode7.2 +osx_image: xcode7.2 env: - ARCH=x86_64 diff --git a/fs-v1.go b/fs-v1.go index f9a7b759d..14470a2ef 100644 --- a/fs-v1.go +++ b/fs-v1.go @@ -83,8 +83,10 @@ func shutdownFS(storage StorageAPI) { os.Exit(1) } if err = storage.DeleteVol(minioMetaBucket); err != nil { - errorIf(err, "Unable to delete minio meta bucket", minioMetaBucket) - os.Exit(1) + if err != errVolumeNotEmpty { + errorIf(err, "Unable to delete minio meta bucket %s", minioMetaBucket) + os.Exit(1) + } } // Successful exit. os.Exit(0) diff --git a/posix.go b/posix.go index a5135a79a..4d5208a07 100644 --- a/posix.go +++ b/posix.go @@ -488,7 +488,7 @@ func (s *posix) ReadFile(volume string, path string, offset int64, buf []byte) ( } else if os.IsPermission(err) { return 0, errFileAccessDenied } else if strings.Contains(err.Error(), "not a directory") { - return 0, errFileNotFound + return 0, errFileAccessDenied } return 0, err } @@ -500,10 +500,12 @@ func (s *posix) ReadFile(volume string, path string, offset int64, buf []byte) ( if err != nil { return 0, err } + // Verify if its not a regular file, since subsequent Seek is undefined. if !st.Mode().IsRegular() { - return 0, errFileNotFound + return 0, errIsNotRegular } + // Seek to requested offset. _, err = file.Seek(offset, os.SEEK_SET) if err != nil { @@ -554,7 +556,7 @@ func (s *posix) AppendFile(volume, path string, buf []byte) (err error) { // Verify if the file already exists and is not of regular type. var st os.FileInfo if st, err = os.Stat(preparePath(filePath)); err == nil { - if st.IsDir() { + if !st.Mode().IsRegular() { return errIsNotRegular } } @@ -565,6 +567,7 @@ func (s *posix) AppendFile(volume, path string, buf []byte) (err error) { if strings.Contains(err.Error(), "not a directory") { return errFileAccessDenied } else if runtime.GOOS == "windows" && strings.Contains(err.Error(), "system cannot find the path specified") { + // Add specific case for windows. return errFileAccessDenied } return err diff --git a/posix_test.go b/posix_test.go index b8843e004..659b2ac4d 100644 --- a/posix_test.go +++ b/posix_test.go @@ -17,6 +17,9 @@ package main import ( + "bytes" + "errors" + "io" "io/ioutil" "os" slashpath "path" @@ -204,7 +207,7 @@ func TestMakeVol(t *testing.T) { for _, testCase := range testCases { if err := posix.MakeVol(testCase.volName); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } @@ -262,7 +265,7 @@ func TestDeleteVol(t *testing.T) { for _, testCase := range testCases { if err := posix.DeleteVol(testCase.volName); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } @@ -310,7 +313,7 @@ func TestStatVol(t *testing.T) { for _, testCase := range testCases { if _, err := posix.StatVol(testCase.volName); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } } @@ -383,7 +386,7 @@ func TestDeleteFile(t *testing.T) { for _, testCase := range testCases { if err := posix.DeleteFile("success-vol", testCase.fileName); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } @@ -401,6 +404,187 @@ func TestDeleteFile(t *testing.T) { } } +// Test posix.ReadFile() +func TestReadFile(t *testing.T) { + // Create temporary directory. + path, err := ioutil.TempDir("", "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path) + + // Initialize posix storage layer. + posix, err := newPosix(path) + if err != nil { + t.Fatalf("Unable to initialize posix, %s", err) + } + + // Setup test environment. + if err = posix.MakeVol("success-vol"); err != nil { + t.Fatalf("Unable to create volume, %s", err) + } + + // Create directory to make errIsNotRegular + if err = os.Mkdir(slashpath.Join(path, "success-vol", "object-as-dir"), 0777); err != nil { + t.Fatalf("Unable to create directory, %s", err) + } + + testCases := []struct { + fileName string + offset int64 + bufSize int + expectedBuf []byte + expectedErr error + }{ + // Successful read at offset 0 and proper buffer size. - 1 + { + "myobject", 0, 5, + []byte("hello"), nil, + }, + // Success read at hierarchy. - 2 + { + "path/to/my/object", 0, 5, + []byte("hello"), nil, + }, + // One path segment length is 255 chars long. - 3 + { + "path/to/my/object000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001", + 0, 5, []byte("hello"), nil}, + // Whole path is 1024 characters long, success case. - 4 + { + "level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001/level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002/level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003/object000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001", + 0, 5, []byte("hello"), + func() error { + // On darwin HFS does not support > 1024 characters. + if runtime.GOOS == "darwin" { + return errFileNameTooLong + } + // On all other platforms return success. + return nil + }(), + }, + // Object is a directory. - 5 + { + "object-as-dir", + 0, 5, nil, errIsNotRegular}, + // One path segment length is > 255 chars long. - 6 + { + "path/to/my/object0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001", + 0, 5, nil, errFileNameTooLong}, + // Path length is > 1024 chars long. - 7 + { + "level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001/level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002/level0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003/object000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001", + 0, 5, nil, errFileNameTooLong}, + // Buffer size greater than object size. - 8 + { + "myobject", 0, 16, + []byte("hello, world"), + io.ErrUnexpectedEOF, + }, + // Reading from an offset success. - 9 + { + "myobject", 7, 5, + []byte("world"), nil, + }, + // Reading from an object but buffer size greater. - 10 + { + "myobject", + 7, 8, + []byte("world"), + io.ErrUnexpectedEOF, + }, + // Seeking into a wrong offset, return PathError. - 11 + { + "myobject", + -1, 5, + nil, + func() error { + if runtime.GOOS == "windows" { + return &os.PathError{ + Op: "seek", + Path: preparePath(slashpath.Join(path, "success-vol", "myobject")), + Err: errors.New("An attempt was made to move the file pointer before the beginning of the file."), + } + } + return &os.PathError{ + Op: "seek", + Path: preparePath(slashpath.Join(path, "success-vol", "myobject")), + Err: os.ErrInvalid, + } + }(), + }, + // Seeking ahead returns io.EOF. - 12 + { + "myobject", 14, 1, nil, io.EOF, + }, + } + + // Create all files needed during testing. + appendFiles := testCases[:4] + + // Create test files for further reading. + for i, appendFile := range appendFiles { + err = posix.AppendFile("success-vol", appendFile.fileName, []byte("hello, world")) + if err != appendFile.expectedErr { + t.Fatalf("Creating file failed: %d %#v, expected: %s, got: %s", i+1, appendFile, appendFile.expectedErr, err) + } + } + + // Following block validates all ReadFile test cases. + for i, testCase := range testCases { + // Common read buffer. + var buf = make([]byte, testCase.bufSize) + n, err := posix.ReadFile("success-vol", testCase.fileName, testCase.offset, buf) + if err != nil && testCase.expectedErr != nil { + // Validate if the type string of the errors are an exact match. + if err.Error() != testCase.expectedErr.Error() { + t.Errorf("Case: %d %#v, expected: %s, got: %s", i+1, testCase, testCase.expectedErr, err) + } + // Err unexpected EOF special case, where we verify we have provided a larger + // buffer than the data itself, but the results are in-fact valid. So we validate + // this error condition specifically treating it as a good condition with valid + // results. In this scenario return 'n' is always lesser than the input buffer. + if err == io.ErrUnexpectedEOF { + if !bytes.Equal(testCase.expectedBuf, buf[:n]) { + t.Errorf("Case: %d %#v, expected: \"%s\", got: \"%s\"", i+1, testCase, string(testCase.expectedBuf), string(buf[:testCase.bufSize])) + } + if n > int64(len(buf)) { + t.Errorf("Case: %d %#v, expected: %d, got: %d", i+1, testCase, testCase.bufSize, n) + } + } + } + // ReadFile has returned success, but our expected error is non 'nil'. + if err == nil && err != testCase.expectedErr { + t.Errorf("Case: %d %#v, expected: %s, got :%s", i+1, testCase, testCase.expectedErr, err) + } + // Expected error retured, proceed further to validate the returned results. + if err == nil && err == testCase.expectedErr { + if !bytes.Equal(testCase.expectedBuf, buf) { + t.Errorf("Case: %d %#v, expected: \"%s\", got: \"%s\"", i+1, testCase, string(testCase.expectedBuf), string(buf[:testCase.bufSize])) + } + if n != int64(testCase.bufSize) { + t.Errorf("Case: %d %#v, expected: %d, got: %d", i+1, testCase, testCase.bufSize, n) + } + } + } + + // Test for permission denied. + if runtime.GOOS == "linux" { + // Initialize posix storage layer for permission denied error. + posix, err := newPosix("/") + if err != nil { + t.Errorf("Unable to initialize posix, %s", err) + } + if err == nil { + // Common read buffer. + var buf = make([]byte, 10) + if _, err = posix.ReadFile("proc", "1/fd", 0, buf); err != errFileAccessDenied { + t.Errorf("expected: %s, got: %s", errFileAccessDenied, err) + } + } + } +} + // Test posix.AppendFile() func TestAppendFile(t *testing.T) { // Create temporary directory. @@ -460,7 +644,7 @@ func TestAppendFile(t *testing.T) { for _, testCase := range testCases { if err := posix.AppendFile("success-vol", testCase.fileName, []byte("hello, world")); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } @@ -531,7 +715,7 @@ func TestRenameFile(t *testing.T) { for _, testCase := range testCases { if err := posix.RenameFile("src-vol", testCase.srcPath, "dest-vol", testCase.destPath); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } } @@ -580,7 +764,7 @@ func TestStatFile(t *testing.T) { for _, testCase := range testCases { if _, err := posix.StatFile("success-vol", testCase.path); err != testCase.expectedErr { - t.Fatalf("case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) + t.Fatalf("Case: %s, expected: %s, got: %s", testCase, testCase.expectedErr, err) } } }