posix: Re-do tests for readDir(). (#1996)

posix-list-dir-others.go

@@ -21,6 +21,7 @@ package main
 import (
 	"io"
 	"os"
+	"path"
 	"strings"
 )
 
@@ -42,6 +43,7 @@ func readDir(dirPath string) (entries []string, err error) {
 	defer d.Close()
 
 	for {
+		// Read 1000 entries.
 		fis, err := d.Readdir(1000)
 		if err != nil {
 			if err == io.EOF {
@@ -54,6 +56,22 @@ func readDir(dirPath string) (entries []string, err error) {
 			if hasPosixReservedPrefix(fi.Name()) {
 				continue
 			}
+			// Stat symbolic link and follow to get the final value.
+			if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
+				var st os.FileInfo
+				st, err = os.Stat(preparePath(path.Join(dirPath, fi.Name())))
+				if err != nil {
+					errorIf(err, "Unable to stat path %s", path.Join(dirPath, fi.Name()))
+					continue
+				}
+				// Append to entries if symbolic link exists and is valid.
+				if st.IsDir() {
+					entries = append(entries, fi.Name()+slashSeparator)
+				} else if st.Mode().IsRegular() {
+					entries = append(entries, fi.Name())
+				}
+				continue
+			}
 			if fi.Mode().IsDir() {
 				// Append "/" instead of "\" so that sorting is achieved as expected.
 				entries = append(entries, fi.Name()+slashSeparator)
@@ -62,5 +80,5 @@ func readDir(dirPath string) (entries []string, err error) {
 			}
 		}
 	}
-	return
+	return entries, nil
 }

posix-list-dir_test.go

@@ -21,12 +21,13 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"runtime"
 	"sort"
 	"testing"
 )
 
 // Test to check for different input arguments.
-func TestReadDir0(t *testing.T) {
+func TestReadDirFail(t *testing.T) {
 	// Check non existent directory.
 	if _, err := readDir("/tmp/non-existent-directory"); err != errFileNotFound {
 		t.Fatalf("expected = %s, got: %s", errFileNotFound, err)
@@ -37,85 +38,133 @@ func TestReadDir0(t *testing.T) {
 		t.Fatalf("expected = %s, got: %s", errFileNotFound, err)
 	}
 
-	// Check if permission denied.
-	if _, err := readDir("/proc/1/fd"); err == nil {
-		t.Fatalf("expected = an error, got: nil")
+	// Only valid for linux.
+	if runtime.GOOS == "linux" {
+		// Check if permission denied.
+		if _, err := readDir("/proc/1/fd"); err == nil {
+			t.Fatalf("expected = an error, got: nil")
+		}
 	}
 }
 
+// Represents data type for all the test results.
 type result struct {
 	dir     string
 	entries []string
 }
 
-var testResults = []result{}
-
-// Test to read empty directory.
-func setupTestReadDir1(t *testing.T) {
+func mustSetupDir(t *testing.T) string {
 	// Create unique test directory.
-	if dir, err := ioutil.TempDir("", "minio-posix-list-dir"); err != nil {
-		t.Fatal("failed to setup.", err)
-	} else {
-		// Add empty entry slice for this test directory.
-		testResults = append(testResults, result{dir, []string{}})
+	dir, err := ioutil.TempDir("", "minio-posix-list-dir")
+	if err != nil {
+		t.Fatalf("Unable to setup directory, %s", err)
 	}
+	return dir
 }
 
-// Test to read empty directory with reserved names.
-func setupTestReadDir2(t *testing.T) {
-	// Create unique test directory.
-	if dir, err := ioutil.TempDir("", "minio-posix-list-dir"); err != nil {
-		t.Fatal("failed to setup.", err)
-	} else {
-		entries := []string{}
-		// Create a file with reserved name.
-		for _, reservedName := range posixReservedPrefix {
-			if err := ioutil.WriteFile(filepath.Join(dir, reservedName), []byte{}, os.ModePerm); err != nil {
-				// For cleanup, its required to add these entries into test results.
-				sort.Strings(entries)
-				testResults = append(testResults, result{dir, entries})
-				t.Fatal("failed to setup.", err)
-			}
-		}
-
-		sort.Strings(entries)
+// Test to read empty directory.
+func setupTestReadDirEmpty(t *testing.T) (testResults []result) {
+	// Add empty entry slice for this test directory.
+	testResults = append(testResults, result{mustSetupDir(t), []string{}})
+	return testResults
+}
 
-		// Add entries slice for this test directory.
-		testResults = append(testResults, result{dir, entries})
+// Test to read empty directory with only reserved names.
+func setupTestReadDirReserved(t *testing.T) (testResults []result) {
+	dir := mustSetupDir(t)
+	entries := []string{}
+	// Create a file with reserved name.
+	for _, reservedName := range posixReservedPrefix {
+		if err := ioutil.WriteFile(filepath.Join(dir, reservedName), []byte{}, os.ModePerm); err != nil {
+			// For cleanup, its required to add these entries into test results.
+			testResults = append(testResults, result{dir, entries})
+			t.Fatalf("Unable to create file, %s", err)
+		}
+		// entries = append(entries, reservedName) - reserved files are skipped.
 	}
+	sort.Strings(entries)
+
+	// Add entries slice for this test directory.
+	testResults = append(testResults, result{dir, entries})
+	return testResults
 }
 
-// Test to read non-empty directory.
-func setupTestReadDir3(t *testing.T) {
-	if dir, err := ioutil.TempDir("", "minio-posix-list-dir"); err != nil {
-		t.Fatal("failed to setup.", err)
-	} else {
-		entries := []string{}
-		for i := 0; i < 10; i++ {
-			name := fmt.Sprintf("file-%d", i)
-			entries = append(entries, name)
-			if err := ioutil.WriteFile(filepath.Join(dir, name), []byte{}, os.ModePerm); err != nil {
-				// Keep entries sorted for easier comparison.
-				sort.Strings(entries)
-				// For cleanup, its required to add these entries into test results.
-				testResults = append(testResults, result{dir, entries})
-				t.Fatal("failed to setup.", err)
-			}
+// Test to read non-empty directory with only files.
+func setupTestReadDirFiles(t *testing.T) (testResults []result) {
+	dir := mustSetupDir(t)
+	entries := []string{}
+	for i := 0; i < 10; i++ {
+		name := fmt.Sprintf("file-%d", i)
+		if err := ioutil.WriteFile(filepath.Join(dir, name), []byte{}, os.ModePerm); err != nil {
+			// For cleanup, its required to add these entries into test results.
+			testResults = append(testResults, result{dir, entries})
+			t.Fatalf("Unable to create file, %s", err)
 		}
+		entries = append(entries, name)
+	}
 
-		// Keep entries sorted for easier comparison.
-		sort.Strings(entries)
+	// Keep entries sorted for easier comparison.
+	sort.Strings(entries)
 
-		// Add entries slice for this test directory.
-		testResults = append(testResults, result{dir, entries})
+	// Add entries slice for this test directory.
+	testResults = append(testResults, result{dir, entries})
+	return testResults
+}
+
+// Test to read non-empty directory with directories and files.
+func setupTestReadDirGeneric(t *testing.T) (testResults []result) {
+	dir := mustSetupDir(t)
+	if err := os.MkdirAll(filepath.Join(dir, "mydir"), 0777); err != nil {
+		t.Fatalf("Unable to create prefix directory \"mydir\", %s", err)
+	}
+	entries := []string{"mydir/"}
+	for i := 0; i < 10; i++ {
+		name := fmt.Sprintf("file-%d", i)
+		if err := ioutil.WriteFile(filepath.Join(dir, "mydir", name), []byte{}, os.ModePerm); err != nil {
+			// For cleanup, its required to add these entries into test results.
+			testResults = append(testResults, result{dir, entries})
+			t.Fatalf("Unable to write file, %s", err)
+		}
 	}
+	// Keep entries sorted for easier comparison.
+	sort.Strings(entries)
+
+	// Add entries slice for this test directory.
+	testResults = append(testResults, result{dir, entries})
+	return testResults
 }
 
-// teardown - cleans up test directories.
-func teardown() {
-	for _, r := range testResults {
-		os.RemoveAll(r.dir)
+// Test to read non-empty directory with symlinks.
+func setupTestReadDirSymlink(t *testing.T) (testResults []result) {
+	dir := mustSetupDir(t)
+	entries := []string{}
+	for i := 0; i < 10; i++ {
+		name1 := fmt.Sprintf("file-%d", i)
+		name2 := fmt.Sprintf("file-%d", i+10)
+		if err := ioutil.WriteFile(filepath.Join(dir, name1), []byte{}, os.ModePerm); err != nil {
+			// For cleanup, its required to add these entries into test results.
+			testResults = append(testResults, result{dir, entries})
+			t.Fatalf("Unable to create a file, %s", err)
+		}
+		// Symlink will not be added to entries.
+		if err := os.Symlink(filepath.Join(dir, name1), filepath.Join(dir, name2)); err != nil {
+			t.Fatalf("Unable to create a symlink, %s", err)
+		}
+		// Add to entries.
+		entries = append(entries, name1)
+		entries = append(entries, name2)
 	}
+	if err := os.MkdirAll(filepath.Join(dir, "mydir"), 0777); err != nil {
+		t.Fatalf("Unable to create \"mydir\", %s", err)
+	}
+	entries = append(entries, "mydir/")
+
+	// Keep entries sorted for easier comparison.
+	sort.Strings(entries)
+
+	// Add entries slice for this test directory.
+	testResults = append(testResults, result{dir, entries})
+	return testResults
 }
 
 // checkResult - checks whether entries are got are same as expected entries.
@@ -136,13 +185,32 @@ func checkResult(expected []string, got []string) bool {
 	return true
 }
 
+// teardown - cleans up test directories.
+func teardown(testResults []result) {
+	for _, r := range testResults {
+		os.RemoveAll(r.dir)
+	}
+}
+
 // TestReadDir - test function to run various readDir() tests.
 func TestReadDir(t *testing.T) {
-	defer teardown()
-	setupTestReadDir1(t)
-	setupTestReadDir2(t)
-	setupTestReadDir3(t)
-
+	var testResults []result
+
+	// Setup and capture test results for empty directory.
+	testResults = append(testResults, setupTestReadDirEmpty(t)...)
+	// Setup and capture test results for reserved files.
+	testResults = append(testResults, setupTestReadDirReserved(t)...)
+	// Setup and capture test results for directory with only files.
+	testResults = append(testResults, setupTestReadDirFiles(t)...)
+	// Setup and capture test results for directory with files and directories.
+	testResults = append(testResults, setupTestReadDirGeneric(t)...)
+	// Setup and capture test results for directory with files and symlink.
+	testResults = append(testResults, setupTestReadDirSymlink(t)...)
+
+	// Remove all dirs once tests are over.
+	defer teardown(testResults)
+
+	// Validate all the results.
 	for _, r := range testResults {
 		if entries, err := readDir(r.dir); err != nil {
 			t.Fatal("failed to run test.", err)