*: Expand tests covering fs.Rename

Ahead of making changes to Rename, ensures that the behaviour across the built-in
implementations are aligned by expanding tests, so that we avoid future regression.

Signed-off-by: Paulo Gomes <[email protected]>

Author: pjbgf

Makefile

@@ -14,7 +14,7 @@ $(GOLANGCI):
 
 .PHONY: test
 test:
-	$(GOTEST) -race ./...
+	$(GOTEST) -race -timeout 60s ./...
 
 test-coverage:
 	echo "" > $(COVERAGE_REPORT); \

fs.go

@@ -8,9 +8,11 @@ import (
 )
 
 var (
-	ErrReadOnly        = errors.New("read-only filesystem")
-	ErrNotSupported    = errors.New("feature not supported")
-	ErrCrossedBoundary = errors.New("chroot boundary crossed")
+	ErrReadOnly               = errors.New("read-only filesystem")
+	ErrNotSupported           = errors.New("feature not supported")
+	ErrCrossedBoundary        = errors.New("chroot boundary crossed")
+	ErrBaseDirCannotBeRemoved = errors.New("base dir cannot be removed")
+	ErrBaseDirCannotBeRenamed = errors.New("base dir cannot be renamed")
 )
 
 // Capability holds the supported features of a billy filesystem. This does

osfs/os_bound.go

@@ -20,7 +20,6 @@
 package osfs
 
 import (
-	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -32,9 +31,6 @@ import (
 )
 
 var (
-	ErrBaseDirCannotBeRemoved = errors.New("base dir cannot be removed")
-	ErrBaseDirCannotBeRenamed = errors.New("base dir cannot be renamed")
-
 	dotPrefixes = []string{"./", ".\\"}
 )
 
@@ -84,15 +80,21 @@ func (fs *BoundOS) ReadDir(path string) ([]os.FileInfo, error) {
 
 func (fs *BoundOS) Rename(from, to string) error {
 	if from == "." || from == fs.baseDir {
-		return ErrBaseDirCannotBeRenamed
+		return billy.ErrBaseDirCannotBeRenamed
 	}
 
 	from = fs.expandDot(from)
-	to = fs.expandDot(to)
+	_, err := fs.Lstat(from)
+	if err != nil {
+		return err
+	}
+
 	f, err := fs.abs(from)
 	if err != nil {
 		return err
 	}
+
+	to = fs.expandDot(to)
 	t, err := fs.abs(to)
 	if err != nil {
 		return err
@@ -130,7 +132,7 @@ func (fs *BoundOS) Stat(filename string) (os.FileInfo, error) {
 
 func (fs *BoundOS) Remove(filename string) error {
 	if filename == "." || filename == fs.baseDir {
-		return ErrBaseDirCannotBeRemoved
+		return billy.ErrBaseDirCannotBeRemoved
 	}
 
 	fn, err := fs.abs(filename)
@@ -161,7 +163,7 @@ func (fs *BoundOS) Join(elem ...string) string {
 
 func (fs *BoundOS) RemoveAll(path string) error {
 	if path == "." || path == fs.baseDir {
-		return ErrBaseDirCannotBeRemoved
+		return billy.ErrBaseDirCannotBeRemoved
 	}
 
 	path = fs.expandDot(path)

osfs/os_bound_test.go

@@ -1293,10 +1293,10 @@ func TestRename(t *testing.T) {
 	assert.NotNil(fi)
 
 	err = fs.Rename(filepath.FromSlash("/tmp/outside/cwd/file1"), newFile)
-	require.ErrorContains(t, err, notFoundError())
+	require.ErrorIs(t, err, os.ErrNotExist)
 
 	err = fs.Rename(oldFile, filepath.FromSlash("/tmp/outside/cwd/file2"))
-	require.ErrorContains(t, err, notFoundError())
+	require.ErrorIs(t, err, os.ErrNotExist)
 }
 
 func mustExist(filename string) {

osfs/os_chroot.go

@@ -52,7 +52,12 @@ func (fs *ChrootOS) ReadDir(dir string) ([]os.FileInfo, error) {
 }
 
 func (fs *ChrootOS) Rename(from, to string) error {
-	if err := fs.createDir(to); err != nil {
+	_, err := fs.Lstat(from)
+	if err != nil {
+		return err
+	}
+
+	if err = fs.createDir(to); err != nil {
 		return err
 	}
 

test/basic_test.go

@@ -4,11 +4,17 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	stdfs "io/fs"
 	"os"
 	"path/filepath"
+	"reflect"
+	"slices"
+	"strings"
 	"testing"
 
+	"github.com/go-git/go-billy/v6"
 	. "github.com/go-git/go-billy/v6" //nolint
+	"github.com/go-git/go-billy/v6/osfs"
 	"github.com/go-git/go-billy/v6/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -469,24 +475,101 @@ func TestStatNonExistent(t *testing.T) {
 }
 
 func TestRename(t *testing.T) {
-	eachBasicFS(t, func(t *testing.T, fs Basic) {
-		t.Helper()
-		err := util.WriteFile(fs, "foo", nil, 0644)
-		require.NoError(t, err)
-
-		err = fs.Rename("foo", "bar")
-		require.NoError(t, err)
-
-		foo, err := fs.Stat("foo")
-		assert.Nil(t, foo)
-		assert.ErrorIs(t, err, os.ErrNotExist)
+	tests := []struct {
+		name      string
+		before    func(*testing.T, billy.Filesystem)
+		from      string
+		to        string
+		wantErr   *error
+		wantFiles []string
+	}{
+		{
+			name:    "from not found",
+			from:    "foo",
+			to:      "bar",
+			wantErr: &os.ErrNotExist,
+		},
+		{
+			name: "file rename",
+			before: func(t *testing.T, fs billy.Filesystem) {
+				root := fsRoot(fs)
+				_, err := fs.Create(fs.Join(root, "foo"))
+				require.NoError(t, err)
+			},
+			from:      "foo",
+			to:        "bar",
+			wantFiles: []string{"/bar"},
+		},
+		{
+			name: "dir rename",
+			before: func(t *testing.T, fs billy.Filesystem) {
+				root := fsRoot(fs)
+				_, err := fs.Create(fs.Join(root, "foo", "bar1"))
+				require.NoError(t, err)
+				_, err = fs.Create(fs.Join(root, "foo", "bar2"))
+				require.NoError(t, err)
+			},
+			from:      "foo",
+			to:        "bar",
+			wantFiles: []string{"/bar/bar1", "/bar/bar2"},
+		},
+	}
 
-		bar, err := fs.Stat("bar")
-		require.NoError(t, err)
-		assert.NotNil(t, bar)
+	eachFS(t, func(t *testing.T, fs Filesystem) {
+		t.Helper()
+
+		root := fsRoot(fs)
+		for _, tc := range tests {
+			t.Run(tc.name, func(t *testing.T) {
+				if tc.before != nil {
+					tc.before(t, fs)
+				}
+
+				err := fs.Rename(fs.Join(root, tc.from), fs.Join(root, tc.to))
+				if tc.wantErr == nil {
+					require.NoError(t, err)
+				} else {
+					require.ErrorIs(t, err, *tc.wantErr)
+				}
+
+				err = util.Walk(fs, root, func(path string, fi stdfs.FileInfo, err error) error {
+					if err != nil {
+						return err
+					}
+
+					if fi.IsDir() {
+						return nil
+					}
+
+					if root != "/" {
+						path = strings.TrimPrefix(path, root)
+					}
+					if !slices.Contains(tc.wantFiles, path) {
+						assert.Fail(t, "file not found", "name", path)
+					}
+
+					return nil
+				})
+				require.NoError(t, err)
+
+				fis, _ := fs.ReadDir(root)
+				for _, fi := range fis {
+					cpath := fs.Join(root, fi.Name())
+					err := util.RemoveAll(fs, cpath)
+					require.NoError(t, err)
+				}
+			})
+		}
 	})
 }
 
+func fsRoot(fs billy.Filesystem) string {
+	if reflect.TypeOf(fs) == reflect.TypeOf(&osfs.BoundOS{}) {
+		return fs.Root()
+	}
+	return "/"
+}
+
 func TestOpenAndWrite(t *testing.T) {
 	eachBasicFS(t, func(t *testing.T, fs Basic) {
 		t.Helper()