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.

10 files changed, 108 insertions, 115 deletions

diff --git a/fixtures/fixtures.go b/fixtures/fixtures.go
index 72b4f1c..15f0e02 100644
--- a/fixtures/fixtures.go
+++ b/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
diff --git a/remote_test.go b/remote_test.go
index 12e4625..cb5cca5 100644
--- a/remote_test.go
+++ b/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})
diff --git a/repository.go b/repository.go
index 4d6b4e3..2e2e57e 100644
--- a/repository.go
+++ b/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
 	}
diff --git a/storage/filesystem/internal/dotgit/dotgit_test.go b/storage/filesystem/internal/dotgit/dotgit_test.go
index ebd8596..9e46639 100644
--- a/storage/filesystem/internal/dotgit/dotgit_test.go
+++ b/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)
diff --git a/storage/filesystem/internal/dotgit/writers_test.go b/storage/filesystem/internal/dotgit/writers_test.go
index ebecbb4..2dff4e9 100644
--- a/storage/filesystem/internal/dotgit/writers_test.go
+++ b/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()
diff --git a/utils/fs/fs.go b/utils/fs/fs.go
index 9be0f88..4eaad15 100644
--- a/utils/fs/fs.go
+++ b/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
-}
diff --git a/utils/fs/os.go b/utils/fs/os/os.go
index db45a1a..0b0173f 100644
--- a/utils/fs/os.go
+++ b/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
+}
diff --git a/utils/fs/os/os_test.go b/utils/fs/os/os_test.go
new file mode 100644
index 0000000..02a0a6c
--- /dev/null
+++ b/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)
+}
diff --git a/utils/fs/os_test.go b/utils/fs/os_test.go
deleted file mode 100644
index e70c19b..0000000
--- a/utils/fs/os_test.go
+++ /dev/null
@@ -1,24 +0,0 @@
-package fs
-
-import (
-	"io/ioutil"
-	"os"
-
-	. "gopkg.in/check.v1"
-)
-
-type OSSuite struct {
-	FilesystemSuite
-	path string
-}
-
-var _ = Suite(&OSSuite{})
-
-func (s *OSSuite) SetUpTest(c *C) {
-	s.path, _ = ioutil.TempDir(os.TempDir(), "go-git-os-fs-test")
-	s.FilesystemSuite.fs = NewOS(s.path)
-}
-func (s *OSSuite) TearDownTest(c *C) {
-	err := os.RemoveAll(s.path)
-	c.Assert(err, IsNil)
-}
diff --git a/utils/fs/fs_test.go b/utils/fs/test/fs_suite.go
index 428c26c..109311f 100644
--- a/utils/fs/fs_test.go
+++ b/utils/fs/test/fs_suite.go
@@ -1,54 +1,42 @@
-package fs
+package test
 
 import (
-	"io"
+	"fmt"
 	"io/ioutil"
 	"strings"
 	"testing"
 
 	. "gopkg.in/check.v1"
+	. "gopkg.in/src-d/go-git.v4/utils/fs"
 )
 
 func Test(t *testing.T) { TestingT(t) }
 
 type FilesystemSuite struct {
-	fs Filesystem
+	Fs Filesystem
 }
 
 func (s *FilesystemSuite) TestCreate(c *C) {
-	f, err := s.fs.Create("foo")
+	f, err := s.Fs.Create("foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.Filename(), Equals, "foo")
 }
 
 func (s *FilesystemSuite) TestCreateDepth(c *C) {
-	f, err := s.fs.Create("bar/foo")
+	f, err := s.Fs.Create("bar/foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.Filename(), Equals, "bar/foo")
 }
 
 func (s *FilesystemSuite) TestCreateDepthAbsolute(c *C) {
-	f, err := s.fs.Create("/bar/foo")
+	f, err := s.Fs.Create("/bar/foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.Filename(), Equals, "bar/foo")
 }
 
-func (s *FilesystemSuite) TestCreateAndWrite(c *C) {
-	f, err := s.fs.Create("foo")
-	c.Assert(err, IsNil)
-	l, err := f.Write([]byte("foo"))
-	c.Assert(err, IsNil)
-	c.Assert(l, Equals, 3)
-
-	f.Seek(0, io.SeekStart)
-	wrote, err := ioutil.ReadAll(f)
-	c.Assert(err, IsNil)
-	c.Assert(wrote, DeepEquals, []byte("foo"))
-}
-
 func (s *FilesystemSuite) TestCreateOverwrite(c *C) {
 	for i := 0; i < 2; i++ {
-		f, err := s.fs.Create("foo")
+		f, err := s.Fs.Create("foo")
 		c.Assert(err, IsNil)
 
 		l, err := f.Write([]byte("foo"))
@@ -59,7 +47,7 @@ func (s *FilesystemSuite) TestCreateOverwrite(c *C) {
 		c.Assert(err, IsNil)
 	}
 
-	f, err := s.fs.Open("foo")
+	f, err := s.Fs.Open("foo")
 	c.Assert(err, IsNil)
 
 	wrote, err := ioutil.ReadAll(f)
@@ -68,14 +56,14 @@ func (s *FilesystemSuite) TestCreateOverwrite(c *C) {
 }
 
 func (s *FilesystemSuite) TestCreateClose(c *C) {
-	f, err := s.fs.Create("foo")
+	f, err := s.Fs.Create("foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.IsClosed(), Equals, false)
 
 	f.Write([]byte("foo"))
 	c.Assert(f.Close(), IsNil)
 
-	file, err := s.fs.Open(f.Filename())
+	file, err := s.Fs.Open(f.Filename())
 	c.Assert(err, IsNil)
 
 	wrote, err := ioutil.ReadAll(file)
@@ -88,83 +76,83 @@ func (s *FilesystemSuite) TestCreateClose(c *C) {
 func (s *FilesystemSuite) TestReadDirAndDir(c *C) {
 	files := []string{"foo", "bar", "qux/baz", "qux/qux"}
 	for _, name := range files {
-		f, err := s.fs.Create(name)
+		f, err := s.Fs.Create(name)
 		c.Assert(err, IsNil)
 		c.Assert(f.Close(), IsNil)
 	}
 
-	info, err := s.fs.ReadDir("/")
+	info, err := s.Fs.ReadDir("/")
 	c.Assert(err, IsNil)
 	c.Assert(info, HasLen, 3)
 
-	info, err = s.fs.ReadDir("/qux")
+	info, err = s.Fs.ReadDir("/qux")
 	c.Assert(err, IsNil)
 	c.Assert(info, HasLen, 2)
 
-	qux := s.fs.Dir("/qux")
+	qux := s.Fs.Dir("/qux")
 	info, err = qux.ReadDir("/")
 	c.Assert(err, IsNil)
 	c.Assert(info, HasLen, 2)
 }
 
 func (s *FilesystemSuite) TestRename(c *C) {
-	f, err := s.fs.Create("foo")
+	f, err := s.Fs.Create("foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.Close(), IsNil)
 
-	err = s.fs.Rename("foo", "bar")
+	err = s.Fs.Rename("foo", "bar")
 	c.Assert(err, IsNil)
 
-	foo, err := s.fs.Stat("foo")
+	foo, err := s.Fs.Stat("foo")
 	c.Assert(foo, IsNil)
 	c.Assert(err, NotNil)
 
-	bar, err := s.fs.Stat("bar")
+	bar, err := s.Fs.Stat("bar")
 	c.Assert(bar, NotNil)
 	c.Assert(err, IsNil)
 }
 
 func (s *FilesystemSuite) TestTempFile(c *C) {
-	f, err := s.fs.TempFile("", "bar")
+	f, err := s.Fs.TempFile("", "bar")
 	c.Assert(err, IsNil)
 
 	c.Assert(strings.HasPrefix(f.Filename(), "bar"), Equals, true)
 }
 
 func (s *FilesystemSuite) TestTempFileWithPath(c *C) {
-	f, err := s.fs.TempFile("foo", "bar")
+	f, err := s.Fs.TempFile("foo", "bar")
 	c.Assert(err, IsNil)
-
-	c.Assert(strings.HasPrefix(f.Filename(), s.fs.Join("foo", "bar")), Equals, true)
+	fmt.Printf("f: %s\n", f.Filename())
+	c.Assert(strings.HasPrefix(f.Filename(), s.Fs.Join("foo", "bar")), Equals, true)
 }
 
 func (s *FilesystemSuite) TestTempFileFullWithPath(c *C) {
-	f, err := s.fs.TempFile("/foo", "bar")
+	f, err := s.Fs.TempFile("/foo", "bar")
 	c.Assert(err, IsNil)
-
-	c.Assert(strings.HasPrefix(f.Filename(), s.fs.Join("foo", "bar")), Equals, true)
+	fmt.Printf("f: %s\n", f.Filename())
+	c.Assert(strings.HasPrefix(f.Filename(), s.Fs.Join("foo", "bar")), Equals, true)
 }
 
 func (s *FilesystemSuite) TestOpenAndStat(c *C) {
-	f, err := s.fs.Create("foo")
+	f, err := s.Fs.Create("foo")
 	c.Assert(err, IsNil)
 	c.Assert(f.Close(), IsNil)
 
-	foo, err := s.fs.Open("foo")
+	foo, err := s.Fs.Open("foo")
 	c.Assert(foo, NotNil)
 	c.Assert(foo.Filename(), Equals, "foo")
 	c.Assert(err, IsNil)
 
-	stat, err := s.fs.Stat("foo")
+	stat, err := s.Fs.Stat("foo")
 	c.Assert(stat, NotNil)
 	c.Assert(err, IsNil)
 	c.Assert(stat.Name(), Equals, "foo")
 }
 
 func (s *FilesystemSuite) TestJoin(c *C) {
-	c.Assert(s.fs.Join("foo", "bar"), Equals, "foo/bar")
+	c.Assert(s.Fs.Join("foo", "bar"), Equals, "foo/bar")
 }
 
 func (s *FilesystemSuite) TestBase(c *C) {
-	c.Assert(s.fs.Base(), Not(Equals), "")
+	c.Assert(s.Fs.Base(), Not(Equals), "")
 }