utils/fs: move 'os' and 'test' to separate packages. (#93)

* create utils/fs/test package to expose generic test suite to 3rd party fs implementations.
* move 'os' to its own package to avoid cyclic dependency (test -> fs -> test, becomes test -> fs, os -> test, os -> fs).
* remove TestCreateAndWrite: some of our implementations cannot read a file that was just created, written and not closed yet.

Author: smola

fixtures/fixtures.go

@@ -11,6 +11,7 @@ import (
 	"gopkg.in/check.v1"
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/utils/fs"
+	osfs "gopkg.in/src-d/go-git.v4/utils/fs/os"
 )
 
 var RootFolder = ""
@@ -161,7 +162,7 @@ func (f *Fixture) DotGit() fs.Filesystem {
 	}
 
 	folders = append(folders, path)
-	return fs.NewOS(path)
+	return osfs.NewOS(path)
 }
 
 type Fixtures []*Fixture

remote_test.go

@@ -8,7 +8,7 @@ import (
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/storage/filesystem"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
-	"gopkg.in/src-d/go-git.v4/utils/fs"
+	osfs "gopkg.in/src-d/go-git.v4/utils/fs/os"
 
 	. "gopkg.in/check.v1"
 )
@@ -88,7 +88,7 @@ func (s *RemoteSuite) TestFetchObjectStorageWriter(c *C) {
 	defer os.RemoveAll(dir) // clean up
 
 	var sto Storage
-	sto, err = filesystem.NewStorage(fs.NewOS(dir))
+	sto, err = filesystem.NewStorage(osfs.NewOS(dir))
 	c.Assert(err, IsNil)
 
 	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: RepositoryFixture})

repository.go

@@ -8,7 +8,7 @@ import (
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/storage/filesystem"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
-	"gopkg.in/src-d/go-git.v4/utils/fs"
+	osfs "gopkg.in/src-d/go-git.v4/utils/fs/os"
 )
 
 var (
@@ -33,7 +33,7 @@ func NewMemoryRepository() *Repository {
 // based on a fs.OS, if you want to use a custom one you need to use the function
 // NewRepository and build you filesystem.Storage
 func NewFilesystemRepository(path string) (*Repository, error) {
-	s, err := filesystem.NewStorage(fs.NewOS(path))
+	s, err := filesystem.NewStorage(osfs.NewOS(path))
 	if err != nil {
 		return nil, err
 	}

storage/filesystem/internal/dotgit/dotgit_test.go

@@ -9,7 +9,7 @@ import (
 
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/fixtures"
-	"gopkg.in/src-d/go-git.v4/utils/fs"
+	osfs "gopkg.in/src-d/go-git.v4/utils/fs/os"
 
 	. "gopkg.in/check.v1"
 )
@@ -27,7 +27,7 @@ func (s *SuiteDotGit) TestSetRefs(c *C) {
 	c.Assert(err, IsNil)
 	defer os.RemoveAll(tmp)
 
-	fs := fs.NewOS(tmp)
+	fs := osfs.NewOS(tmp)
 	dir := New(fs)
 
 	err = dir.SetRef(core.NewReferenceFromStrings(
@@ -164,7 +164,7 @@ func (s *SuiteDotGit) TestNewObject(c *C) {
 	c.Assert(err, IsNil)
 	defer os.RemoveAll(tmp)
 
-	fs := fs.NewOS(tmp)
+	fs := osfs.NewOS(tmp)
 	dir := New(fs)
 	w, err := dir.NewObject()
 	c.Assert(err, IsNil)

storage/filesystem/internal/dotgit/writers_test.go

@@ -9,7 +9,7 @@ import (
 	"strconv"
 
 	"gopkg.in/src-d/go-git.v4/fixtures"
-	"gopkg.in/src-d/go-git.v4/utils/fs"
+	osfs "gopkg.in/src-d/go-git.v4/utils/fs/os"
 
 	. "gopkg.in/check.v1"
 )
@@ -24,7 +24,7 @@ func (s *SuiteDotGit) TestNewObjectPack(c *C) {
 
 	defer os.RemoveAll(dir)
 
-	fs := fs.NewOS(dir)
+	fs := osfs.NewOS(dir)
 	dot := New(fs)
 
 	w, err := dot.NewObjectPack()

utils/fs/fs.go

@@ -8,13 +8,15 @@ import (
 )
 
 var (
-	ErrClosed       = errors.New("File: Writing on closed file.")
+	ErrClosed       = errors.New("file: Writing on closed file.")
 	ErrReadOnly     = errors.New("this is a read-only filesystem")
 	ErrNotSupported = errors.New("feature not supported")
 )
 
 type Filesystem interface {
+	//Create opens a file in write-only mode.
 	Create(filename string) (File, error)
+	//Open opens a file in read-only mode.
 	Open(filename string) (File, error)
 	Stat(filename string) (FileInfo, error)
 	ReadDir(path string) ([]FileInfo, error)
@@ -35,18 +37,3 @@ type File interface {
 }
 
 type FileInfo os.FileInfo
-
-type BaseFile struct {
-	filename string
-	closed   bool
-}
-
-//Filename returns the filename from the File
-func (f *BaseFile) Filename() string {
-	return f.filename
-}
-
-//IsClosed returns if te file is closed
-func (f *BaseFile) IsClosed() bool {
-	return f.closed
-}

utils/fs/os.go renamed to utils/fs/os/os.go

@@ -1,10 +1,12 @@
-package fs
+package os
 
 import (
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
+
+	. "gopkg.in/src-d/go-git.v4/utils/fs"
 )
 
 // OS a filesystem base on the os filesystem
@@ -37,10 +39,7 @@ func (fs *OS) Create(filename string) (File, error) {
 		return nil, err
 	}
 
-	return &OSFile{
-		BaseFile: BaseFile{filename: filename},
-		file:     f,
-	}, nil
+	return newOSFile(filename, f), nil
 }
 
 func (fs *OS) createDir(fullpath string) error {
@@ -92,10 +91,7 @@ func (fs *OS) Open(filename string) (File, error) {
 		return nil, err
 	}
 
-	return &OSFile{
-		BaseFile: BaseFile{filename: filename},
-		file:     f,
-	}, nil
+	return newOSFile(filename, f), nil
 }
 
 // Stat returns the FileInfo structure describing file.
@@ -125,10 +121,7 @@ func (fs *OS) TempFile(dir, prefix string) (File, error) {
 		return nil, err
 	}
 
-	return &OSFile{
-		BaseFile: BaseFile{filename: filename},
-		file:     f,
-	}, nil
+	return newOSFile(filename, f), nil
 }
 
 // Join joins the specified elements using the filesystem separator.
@@ -147,30 +140,49 @@ func (fs *OS) Base() string {
 	return fs.base
 }
 
-// OSFile represents a file in the os filesystem
-type OSFile struct {
-	file *os.File
-	BaseFile
+// osFile represents a file in the os filesystem
+type osFile struct {
+	filename string
+	closed   bool
+	file     *os.File
+}
+
+func newOSFile(filename string, file *os.File) File {
+	return &osFile{
+		filename: filename,
+		closed:   false,
+		file:     file,
+	}
 }
 
-func (f *OSFile) Read(p []byte) (int, error) {
+func (f *osFile) Read(p []byte) (int, error) {
 	return f.file.Read(p)
 }
 
-func (f *OSFile) Seek(offset int64, whence int) (int64, error) {
+func (f *osFile) Seek(offset int64, whence int) (int64, error) {
 	return f.file.Seek(offset, whence)
 }
 
-func (f *OSFile) Write(p []byte) (int, error) {
+func (f *osFile) Write(p []byte) (int, error) {
 	return f.file.Write(p)
 }
 
-func (f *OSFile) Close() error {
+func (f *osFile) Close() error {
 	f.closed = true
 
 	return f.file.Close()
 }
 
-func (f *OSFile) ReadAt(p []byte, off int64) (int, error) {
+func (f *osFile) ReadAt(p []byte, off int64) (n int, err error) {
 	return f.file.ReadAt(p, off)
 }
+
+//Filename returns the filename from the File
+func (f *osFile) Filename() string {
+	return f.filename
+}
+
+//IsClosed returns if te file is closed
+func (f *osFile) IsClosed() bool {
+	return f.closed
+}

utils/fs/os/os_test.go

@@ -0,0 +1,29 @@
+package os_test
+
+import (
+	"io/ioutil"
+	stdos "os"
+	"testing"
+
+	. "gopkg.in/check.v1"
+	"gopkg.in/src-d/go-git.v4/utils/fs/os"
+	"gopkg.in/src-d/go-git.v4/utils/fs/test"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type OSSuite struct {
+	test.FilesystemSuite
+	path string
+}
+
+var _ = Suite(&OSSuite{})
+
+func (s *OSSuite) SetUpTest(c *C) {
+	s.path, _ = ioutil.TempDir(stdos.TempDir(), "go-git-os-fs-test")
+	s.FilesystemSuite.Fs = os.NewOS(s.path)
+}
+func (s *OSSuite) TearDownTest(c *C) {
+	err := stdos.RemoveAll(s.path)
+	c.Assert(err, IsNil)
+}