Address Aaron's comments

1) Clean up unit tests to check against serialization directly
instead of checking refs.
2) Switch argument order of Encode functions to be dst, src
3) Misc other stuff

enc/blob_leaf_codec.go

@@ -11,28 +11,23 @@ var (
 	blobTag = []byte("b ")
 )
 
-func blobLeafEncode(b io.Reader, w io.Writer) (err error) {
-	if _, err = w.Write(blobTag); err != nil {
+func blobLeafEncode(dst io.Writer, src io.Reader) (err error) {
+	if _, err = dst.Write(blobTag); err != nil {
 		return
 	}
-	if _, err = io.Copy(w, b); err != nil {
+	if _, err = io.Copy(dst, src); err != nil {
 		return
 	}
 	return
 }
 
-func blobLeafDecode(r io.Reader) ([]byte, error) {
+func blobLeafDecode(src io.Reader) (io.Reader, error) {
 	buf := &bytes.Buffer{}
-	_, err := io.CopyN(buf, r, int64(len(blobTag)))
+	_, err := io.CopyN(buf, src, int64(len(blobTag)))
 	if err != nil {
 		return nil, err
 	}
-	dbg.Chk.True(bytes.Equal(buf.Bytes(), blobTag))
+	dbg.Chk.True(bytes.Equal(buf.Bytes(), blobTag), "Cannot blobLeafDecode - invalid prefix")
 
-	buf.Truncate(0)
-	_, err = io.Copy(buf, r)
-	if err != nil {
-		return nil, err
-	}
-	return buf.Bytes(), nil
+	return src, nil
 }

enc/blob_leaf_codec_test.go

@@ -2,43 +2,42 @@ package enc
 
 import (
 	"bytes"
+	"io"
 	"testing"
 
-	"github.com/attic-labs/noms/chunks"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestBlobLeafEncode(t *testing.T) {
 	assert := assert.New(t)
-	cs := chunks.NopStore{}
 
-	buf := bytes.NewBuffer([]byte{})
-	w := cs.Put()
-	assert.NoError(blobLeafEncode(buf, w))
-	r, err := w.Ref()
-	assert.NoError(err)
-	// echo -n 'b ' | sha1sum
-	assert.Equal("sha1-e1bc846440ec2fb557a5a271e785cd4c648883fa", r.String())
+	src := &bytes.Buffer{}
+	dst := &bytes.Buffer{}
+	assert.NoError(blobLeafEncode(dst, src))
+	assert.EqualValues(blobTag, dst.Bytes())
 
-	buf = bytes.NewBufferString("Hello, World!")
-	w = cs.Put()
-	assert.NoError(blobLeafEncode(buf, w))
-	r, err = w.Ref()
-	assert.NoError(err)
-	// echo -n 'b Hello, World!' | sha1sum
-	assert.Equal("sha1-135fe1453330547994b2ce8a1b238adfbd7df87e", r.String())
+	src = bytes.NewBufferString("Hello, World!")
+	dst = &bytes.Buffer{}
+	assert.NoError(blobLeafEncode(dst, src))
+	assert.EqualValues(append(blobTag, []byte("Hello, World!")...), dst.Bytes())
 }
 
 func TestBlobLeafDecode(t *testing.T) {
 	assert := assert.New(t)
 
-	reader := bytes.NewBufferString("b ")
-	v1, err := blobLeafDecode(reader)
+	out := &bytes.Buffer{}
+	inputReader := bytes.NewBuffer(blobTag)
+	decoded, err := blobLeafDecode(inputReader)
 	assert.NoError(err)
-	assert.EqualValues([]byte{}, v1)
+	_, err = io.Copy(out, decoded)
+	assert.NoError(err)
+	assert.EqualValues([]byte(nil), out.Bytes())
 
-	reader = bytes.NewBufferString("b Hello World!")
-	v2, err := blobLeafDecode(reader)
+	out.Truncate(0)
+	inputReader = bytes.NewBufferString("b Hello World!")
+	decoded, err = blobLeafDecode(inputReader)
 	assert.NoError(err)
-	assert.EqualValues([]byte("Hello World!"), v2)
+	_, err = io.Copy(out, decoded)
+	assert.NoError(err)
+	assert.EqualValues([]byte("Hello World!"), out.Bytes())
 }

enc/codec.go

@@ -1,3 +1,22 @@
+// Package enc contains a very low-level JSON encoder/decoder. Serializes from interface{} to an io.Writer and deserializes from an io.Reader into an interface{}. Does not recursively process nested compound types; ref.Refs are treated like any other value.
+// Supported types include:
+//   - bool
+//   - int16
+//   - int32
+//   - int64
+//   - uint16
+//   - uint32
+//   - uint64
+//   - float32
+//   - float64
+//   - string
+//   - ref.Ref
+//   - io.Reader, for blobs
+//   - enc.List of any encodeable type
+//   - enc.Map of any encodeable type -> any encodeable type
+//   - enc.Set of any encodeable type
+//   - enc.CompoundBlob, a struct containing metadata for encoding a chunked blob.
+//   - TODO: Add support for structs, and make CompoundBlob use that. BUG #165
 package enc
 
 import (
@@ -9,38 +28,18 @@ import (
 	"github.com/attic-labs/noms/dbg"
 )
 
-// Encode serializes v into w, and panics on unsupported types.
-// Supported types include:
-//   - Primitives
-//   - Strings
-//   - ref.Ref
-//   - io.Reader, for blobs
-//   - enc.List of any encodeable type
-//   - enc.Map of any encodeable type -> any encodeable type
-//   - enc.Set of any encodeable type
-//   - enc.CompoundBlob, a struct containing metadata for encoding a chunked blob.
-//   - TODO: Add support for structs, and make CompoundBlob use that. BUG #165
-func Encode(v interface{}, w io.Writer) error {
-	dbg.Chk.NotNil(w)
+// Encode serializes v into dst, and panics on unsupported types.
+func Encode(dst io.Writer, v interface{}) error {
+	dbg.Chk.NotNil(dst)
 	switch v := v.(type) {
 	case io.Reader:
-		return blobLeafEncode(v, w)
+		return blobLeafEncode(dst, v)
 	default:
-		return jsonEncode(v, w)
+		return jsonEncode(dst, v)
 	}
 }
 
 // Decode deserializes data from r into an interface{}, and panics on unsupported encoded types.
-// Supported types include:
-//   - Primitives
-//   - Strings
-//   - ref.Ref
-//   - io.Reader, for blobs
-//   - enc.List of any encodeable type
-//   - enc.Map of any encodeable type -> any encodeable type
-//   - enc.Set of any encodeable type
-//   - enc.CompoundBlob, a struct containing metadata for encoding a chunked blob.
-//   - TODO: Add support for structs, and make CompoundBlob use that. BUG #165
 func Decode(r io.Reader) (interface{}, error) {
 	dbg.Chk.NotNil(r)
 

enc/codec_test.go

@@ -2,34 +2,24 @@ package enc
 
 import (
 	"bytes"
-	"crypto/sha1"
+	"io"
 	"testing"
 
-	"github.com/attic-labs/noms/chunks"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestEncode(t *testing.T) {
 	assert := assert.New(t)
-	s := chunks.NopStore{}
-
-	testEncode := func(expected string, v interface{}) {
-		w := s.Put()
-		err := Encode(v, w)
-		assert.NoError(err)
-
-		// Assuming that MemoryStore works correctly, we don't need to check the actual serialization, only the hash. Neat.
-		r, err := w.Ref()
-		assert.NoError(err)
-		assert.EqualValues(sha1.Sum([]byte(expected)), r.Digest(), "Incorrect ref serializing %+v. Got: %#x", v, r.Digest())
-		return
-	}
 
 	// Encoding details for each codec are tested elsewhere.
 	// Here we just want to make sure codecs are selected correctly.
-	b := bytes.NewBuffer([]byte{0x00, 0x01, 0x02})
-	testEncode(string([]byte{'b', ' ', 0x00, 0x01, 0x02}), b)
-	testEncode(string("j \"foo\"\n"), "foo")
+	dst := &bytes.Buffer{}
+	assert.NoError(Encode(dst, bytes.NewBuffer([]byte{0x00, 0x01, 0x02})))
+	assert.Equal([]byte{'b', ' ', 0x00, 0x01, 0x02}, dst.Bytes())
+
+	dst.Reset()
+	assert.NoError(Encode(dst, "foo"))
+	assert.Equal("j \"foo\"\n", string(dst.Bytes()))
 }
 
 func TestInvalidDecode(t *testing.T) {
@@ -55,7 +45,10 @@ func TestSelectJSONDecoder(t *testing.T) {
 func TestSelectBlobDecoder(t *testing.T) {
 	assert := assert.New(t)
 
-	v, err := Decode(bytes.NewBuffer([]byte{'b', ' ', 0x2B}))
+	decoded, err := Decode(bytes.NewBuffer([]byte{'b', ' ', 0x2B}))
 	assert.NoError(err)
-	assert.EqualValues([]byte{0x2B}, v)
+	out := &bytes.Buffer{}
+	_, err = io.Copy(out, decoded.(io.Reader))
+	assert.NoError(err)
+	assert.EqualValues([]byte{0x2B}, out.Bytes())
 }

enc/json_decode.go

@@ -19,7 +19,7 @@ func jsonDecode(reader io.Reader) (interface{}, error) {
 		return nil, err
 	}
 
-	// Since jsonDecode is private, and ReadValue() should have checked this, it is invariant that the prefix will match.
+	// Since jsonDecode is private, and Decode() should have checked this, it is invariant that the prefix will match.
 	dbg.Chk.EqualValues(jsonTag[:], prefix, "Cannot jsonDecode - invalid prefix")
 
 	var v interface{}

enc/json_encode.go

@@ -40,15 +40,15 @@ func MapFromItems(e ...interface{}) (m Map) {
 	return
 }
 
-func jsonEncode(v interface{}, w io.Writer) (err error) {
+func jsonEncode(dst io.Writer, v interface{}) (err error) {
 	var j interface{}
 	j, err = getJSON(v)
 	if err != nil {
 		return
 	}
-	_, err = w.Write(jsonTag)
+	_, err = dst.Write(jsonTag)
 	if err == nil {
-		err = json.NewEncoder(w).Encode(j)
+		err = json.NewEncoder(dst).Encode(j)
 	}
 	return
 }

enc/json_encode_test.go

@@ -1,7 +1,7 @@
 package enc
 
 import (
-	"crypto/sha1"
+	"bytes"
 	"fmt"
 	"testing"
 
@@ -21,20 +21,26 @@ func (w *logChunkWriter) Write(data []byte) (int, error) {
 
 func TestJsonEncode(t *testing.T) {
 	assert := assert.New(t)
-	s := chunks.NopStore{}
 
-	testEncode := func(expected string, v interface{}) ref.Ref {
+	getRef := func(v interface{}) ref.Ref {
+		s := chunks.NopStore{}
 		w := s.Put()
-		err := jsonEncode(v, w)
-		assert.NoError(err)
-
-		// Assuming that NopStore works correctly, we don't need to check the actual serialization, only the hash. Neat.
+		assert.NoError(jsonEncode(w, v))
 		r, err := w.Ref()
 		assert.NoError(err)
-		assert.EqualValues(sha1.Sum([]byte(expected)), r.Digest(), "Incorrect ref serializing %+v. Got: %#x", v, r.Digest())
 		return r
 	}
 
+	// Empty compound types
+	emptyMapRef := getRef(Map{})
+	emptyListRef := getRef([]interface{}{})
+
+	testEncode := func(expected string, v interface{}) {
+		dst := &bytes.Buffer{}
+		assert.NoError(jsonEncode(dst, v))
+		assert.Equal(expected, string(dst.Bytes()), "Failed to serialize %+v. Got: %s", v, dst.Bytes())
+	}
+
 	// booleans
 	testEncode(`j false
 `, false)
@@ -67,23 +73,21 @@ func TestJsonEncode(t *testing.T) {
 	testEncode(`j "Hello, World!"
 `, "Hello, World!")
 
-	// Empty compound types
-	emptyMapRef := testEncode(`j {"map":[]}
-`, Map{})
-	emptyListRef := testEncode(`j {"list":[]}
-`, []interface{}{})
-	testEncode(`j {"set":[]}
-`, Set{})
-
 	// Lists
+	testEncode(`j {"list":[]}
+`, []interface{}{})
 	testEncode(`j {"list":["foo",true,{"uint16":42},{"ref":"sha1-58bdf8e374b39f9b1e8a64784cf5c09601f4b7ea"},{"ref":"sha1-dca2a4be23d4455487bb588c6a0ab1b9ee07757e"}]}
 `, []interface{}{"foo", true, uint16(42), emptyListRef, emptyMapRef})
 
 	// Maps
+	testEncode(`j {"map":[]}
+`, Map{})
 	testEncode(`j {"map":["string","hotdog","list",{"ref":"sha1-58bdf8e374b39f9b1e8a64784cf5c09601f4b7ea"},"int32",{"int32":42},"bool",false,"map",{"ref":"sha1-dca2a4be23d4455487bb588c6a0ab1b9ee07757e"}]}
 `, MapFromItems("string", "hotdog", "list", emptyListRef, "int32", int32(42), "bool", false, "map", emptyMapRef))
 
 	// Sets
+	testEncode(`j {"set":[]}
+`, Set{})
 	testEncode(`j {"set":["foo",true,{"uint16":42},{"ref":"sha1-58bdf8e374b39f9b1e8a64784cf5c09601f4b7ea"},{"ref":"sha1-dca2a4be23d4455487bb588c6a0ab1b9ee07757e"}]}
 `, Set{"foo", true, uint16(42), emptyListRef, emptyMapRef})