Address feedback and add archive_test

archive/archive.go

@@ -3,6 +3,7 @@ package archive
 import (
 	"archive/tar"
 	"archive/zip"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -61,7 +62,7 @@ func Copy(dst Writer, src Reader) error {
 		if dst, ok := dst.(zipWriter); ok {
 			return copyZip(dst, src)
 		}
-	// Switch on concrete type since there's no special gzip methods.
+	// Switch on concrete type because gzip has no special methods
 	case *gzip.Reader:
 		if dst, ok := dst.(*gzip.Writer); ok {
 			_, err := io.Copy(dst, src)
@@ -91,22 +92,33 @@ func copyBuffer(dst Writer, src Reader) (err error) {
 			return err
 		case nil: // Proceed below
 		}
+
 		// Buffer the file
 		if _, err := io.Copy(buf, src); err != nil {
+			return fmt.Errorf("buffer to disk: %v", err)
+		}
+
+		// Seek to the start of the file for full file copy
+		if _, err := buf.Seek(0, os.SEEK_SET); err != nil {
+			return err
+		}
+
+		// Set desired file permissions
+		if err := os.Chmod(buf.Name(), defaultFileMode); err != nil {
 			return err
 		}
 		fi, err := buf.Stat()
 		if err != nil {
 			return err
 		}
-		os.Chmod(buf.Name(), defaultFileMode)
 
 		// Write the buffered file
 		if err := dst.NextFile(name, fi); err != nil {
+			fmt.Println("Failed to prep writer for next file")
 			return err
 		}
 		if _, err := io.Copy(dst, buf); err != nil {
-			return err
+			return fmt.Errorf("copy to dst: %v", err)
 		}
 		if err := buf.Truncate(0); err != nil {
 			return err

archive/archive_test.go

@@ -0,0 +1,195 @@
+package archive_test
+
+import (
+	"bytes"
+	"io"
+	"testing"
+
+	"github.com/jonsyu1/seelog/archive"
+	"github.com/jonsyu1/seelog/archive/gzip"
+	"github.com/jonsyu1/seelog/archive/tar"
+	"github.com/jonsyu1/seelog/archive/zip"
+	"github.com/jonsyu1/seelog/io/iotest"
+)
+
+const (
+	gzipType = iota
+	tarType
+	zipType
+)
+
+type file struct {
+	name     string
+	contents []byte
+}
+
+var (
+	oneFile = []file{
+		{
+			name:     "file1",
+			contents: []byte("This is a single log."),
+		},
+	}
+	twoFiles = []file{
+		{
+			name:     "file1",
+			contents: []byte("This is a log."),
+		},
+		{
+			name:     "file2",
+			contents: []byte("This is another log."),
+		},
+	}
+)
+
+var copyTests = map[string]struct {
+	srcType, dstType int
+	in               []file
+}{
+	"zip to zip": {
+		srcType: zipType,
+		dstType: zipType,
+		in:      twoFiles,
+	},
+	"zip to tar": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      twoFiles,
+	},
+	"zip to gzip": {
+		srcType: zipType,
+		dstType: gzipType,
+		in:      oneFile,
+	},
+	"tar to tar": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      twoFiles,
+	},
+	"tar to gzip": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      oneFile,
+	},
+	"tar to zip": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      twoFiles,
+	},
+	"gzip to gzip": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      oneFile,
+	},
+	"gzip to tar": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      oneFile,
+	},
+	"gzip to zip": {
+		srcType: zipType,
+		dstType: tarType,
+		in:      oneFile,
+	},
+}
+
+func TestCopy(t *testing.T) {
+	var srcb, dstb bytes.Buffer
+	for tname, tt := range copyTests {
+		// Reset buffers between tests
+		srcb.Reset()
+		dstb.Reset()
+
+		// Last file name (needed for gzip.NewReader)
+		var fname string
+
+		// Seed the src
+		srcw := writer(t, tname, &srcb, tt.srcType)
+		for _, f := range tt.in {
+			srcw.NextFile(f.name, iotest.FileInfo(t, f.contents))
+			mustCopy(t, tname, srcw, bytes.NewReader(f.contents))
+			fname = f.name
+		}
+		mustClose(t, tname, srcw)
+
+		// Perform the copy
+		srcr := reader(t, tname, &srcb, tt.srcType, fname)
+		dstw := writer(t, tname, &dstb, tt.dstType)
+		if err := archive.Copy(dstw, srcr); err != nil {
+			t.Fatalf("%s: %v", tname, err)
+		}
+		srcr.Close() // Read-only
+		mustClose(t, tname, dstw)
+
+		// Read back dst to confirm our expectations
+		dstr := reader(t, tname, &dstb, tt.dstType, fname)
+		for _, want := range tt.in {
+			var (
+				got file
+				buf bytes.Buffer
+			)
+			got.name, _ = dstr.NextFile()
+			mustCopy(t, tname, &buf, dstr)
+			got.contents = buf.Bytes()
+
+			switch {
+			case got.name != want.name:
+				t.Errorf("%s: got file %q but want file %q",
+					tname, got.name, want.name)
+
+			case !bytes.Equal(got.contents, want.contents):
+				t.Errorf("%s: mismatched contents in %q: got %q but want %q",
+					tname, got.name, got.contents, want.contents)
+			}
+		}
+		dstr.Close()
+	}
+}
+
+func writer(t *testing.T, tname string, w io.Writer, atype int) archive.WriteCloser {
+	switch atype {
+	case gzipType:
+		return gzip.NewWriter(w)
+	case tarType:
+		return tar.NewWriter(w)
+	case zipType:
+		return zip.NewWriter(w)
+	}
+	t.Fatalf("%s: unrecognized archive type: %d", tname, atype)
+	panic("execution continued after (*testing.T).Fatalf")
+}
+
+func reader(t *testing.T, tname string, buf *bytes.Buffer, atype int, fname string) archive.ReadCloser {
+	switch atype {
+	case gzipType:
+		gr, err := gzip.NewReader(buf, fname)
+		if err != nil {
+			t.Fatalf("%s: %v", tname, err)
+		}
+		return gr
+	case tarType:
+		return archive.NopCloser(tar.NewReader(buf))
+	case zipType:
+		zr, err := zip.NewReader(
+			bytes.NewReader(buf.Bytes()),
+			int64(buf.Len()))
+		if err != nil {
+			t.Fatalf("%s: new zip reader: %v", tname, err)
+		}
+		return archive.NopCloser(zr)
+	}
+	t.Fatalf("%s: unrecognized archive type: %d", tname, atype)
+	panic("execution continued after (*testing.T).Fatalf")
+}
+
+func mustCopy(t *testing.T, tname string, dst io.Writer, src io.Reader) {
+	if _, err := io.Copy(dst, src); err != nil {
+		t.Fatalf("%s: copy: %v", tname, err)
+	}
+}
+
+func mustClose(t *testing.T, tname string, c io.Closer) {
+	if err := c.Close(); err != nil {
+		t.Fatalf("%s: close: %v", tname, err)
+	}
+}

archive/gzip/gzip.go

@@ -9,21 +9,24 @@ import (
 	"os"
 )
 
-// A Reader is an io.Reader that can be read to retrieve uncompressed data from
-// a gzip-format compressed file.
+// Reader is an io.Reader that can be read to retrieve uncompressed data from a
+// gzip-format compressed file.
 type Reader struct {
 	gzip.Reader
 	name  string
 	isEOF bool
 }
 
 // NewReader creates a new Reader reading the given reader.
-func NewReader(r io.Reader) (*Reader, error) {
+func NewReader(r io.Reader, name string) (*Reader, error) {
 	gr, err := gzip.NewReader(r)
 	if err != nil {
 		return nil, err
 	}
-	return &Reader{Reader: *gr}, nil
+	return &Reader{
+		Reader: *gr,
+		name:   name,
+	}, nil
 }
 
 // NextFile returns the file name. Calls subsequent to the first call will
@@ -37,14 +40,14 @@ func (r *Reader) NextFile() (name string, err error) {
 	return r.name, nil
 }
 
-// A Writer is an io.WriteCloser. Writes to a Writer are compressed and written to w.
+// Writer is an io.WriteCloser. Writes to a Writer are compressed and written to w.
 type Writer struct {
 	gzip.Writer
 	name        string
 	noMoreFiles bool
 }
 
-// NextFile does nothing, and should never be called more than once.
+// NextFile never returns a next file, and should not be called more than once.
 func (w *Writer) NextFile(name string, _ os.FileInfo) error {
 	if w.noMoreFiles {
 		return fmt.Errorf("gzip: only accepts one file: already received %q and now %q", w.name, name)

archive/tar/tar.go

@@ -6,7 +6,7 @@ import (
 	"os"
 )
 
-// A Reader provides sequential access to the contents of a tar archive.
+// Reader provides sequential access to the contents of a tar archive.
 type Reader struct {
 	tar.Reader
 }
@@ -25,7 +25,7 @@ func (r *Reader) NextFile() (name string, err error) {
 	return hdr.Name, nil
 }
 
-// A Writer provides sequential writing of a tar archive in POSIX.1 format.
+// Writer provides sequential writing of a tar archive in POSIX.1 format.
 type Writer struct {
 	tar.Writer
 	closers []io.Closer

archive/tar/tar_test.go

@@ -1,4 +1,4 @@
-package tar
+package tar_test
 
 import (
 	"bytes"
@@ -7,6 +7,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/jonsyu1/seelog/archive/tar"
 	"github.com/jonsyu1/seelog/io/iotest"
 )
 
@@ -50,7 +51,7 @@ func TestWriterAndReader(t *testing.T) {
 // writeFiles iterates through the files we want and writes them as a tarred
 // file.
 func writeFiles(t *testing.T, f *os.File, tname string, want []file) {
-	w := NewWriter(f)
+	w := tar.NewWriter(f)
 	defer w.Close()
 
 	// Write zipped files
@@ -74,7 +75,7 @@ func writeFiles(t *testing.T, f *os.File, tname string, want []file) {
 
 // readFiles iterates through tarred files and ensures they are the same.
 func readFiles(t *testing.T, f *os.File, tname string, want []file) {
-	r := NewReader(f)
+	r := tar.NewReader(f)
 
 	for _, fwant := range want {
 		fname, err := r.NextFile()

archive/zip/zip.go

@@ -6,7 +6,7 @@ import (
 	"os"
 )
 
-// A Reader provides sequential access to the contents of a zip archive.
+// Reader provides sequential access to the contents of a zip archive.
 type Reader struct {
 	zip.Reader
 	unread []*zip.File
@@ -58,7 +58,7 @@ func (r *Reader) Files() []*zip.File {
 	return r.File
 }
 
-// A Writer provides sequential writing of a zip archive.1 format.
+// Writer provides sequential writing of a zip archive.1 format.
 type Writer struct {
 	zip.Writer
 	w io.Writer

archive/zip/zip_test.go

@@ -1,4 +1,4 @@
-package zip
+package zip_test
 
 import (
 	"bytes"
@@ -7,6 +7,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/jonsyu1/seelog/archive/zip"
 	"github.com/jonsyu1/seelog/io/iotest"
 )
 
@@ -36,7 +37,7 @@ func TestWriterAndReader(t *testing.T) {
 // writeFiles iterates through the files we want and writes them as a zipped
 // file.
 func writeFiles(t *testing.T, f *os.File, tname string, want map[string][]byte) {
-	w := NewWriter(f)
+	w := zip.NewWriter(f)
 	defer w.Close()
 
 	// Write zipped files
@@ -65,7 +66,7 @@ func readFiles(t *testing.T, f *os.File, tname string, want map[string][]byte) {
 	if err != nil {
 		t.Fatalf("%s: stat zipped file: %v", tname, err)
 	}
-	r, err := NewReader(f, fi.Size())
+	r, err := zip.NewReader(f, fi.Size())
 	if err != nil {
 		t.Fatalf("%s: %v", tname, err)
 	}