format/pktline: review and improving coverage

5 files changed, 67 insertions, 29 deletions

diff --git a/pktline/decoder.go b/formats/pktline/decoder.go
index 76be108..a078475 100644
--- a/pktline/decoder.go
+++ b/formats/pktline/decoder.go
@@ -2,7 +2,6 @@ package pktline
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"strconv"
 )
@@ -13,23 +12,29 @@ var (
 	ErrInvalidLen    = errors.New("invalid length")
 )
 
+// Decoder implements a pkt-line format decoder
 type Decoder struct {
 	r io.Reader
 }
 
+// NewDecoder returns a new Decoder
 func NewDecoder(r io.Reader) *Decoder {
 	return &Decoder{r}
 }
 
+// ReadLine reads and return one pkt-line line from the reader
+func (d *Decoder) ReadLine() (string, error) {
+	return d.readLine()
+}
+
 func (d *Decoder) readLine() (string, error) {
-	raw := make([]byte, HEADER_LENGTH)
+	raw := make([]byte, HeaderLength)
 	if _, err := d.r.Read(raw); err != nil {
 		return "", err
 	}
+
 	header, err := strconv.ParseInt(string(raw), 16, 16)
 	if err != nil {
-		fmt.Println(err)
-
 		return "", ErrInvalidHeader
 	}
 
@@ -37,7 +42,7 @@ func (d *Decoder) readLine() (string, error) {
 		return "", nil
 	}
 
-	exp := int(header - HEADER_LENGTH)
+	exp := int(header - HeaderLength)
 	if exp < 0 {
 		return "", ErrInvalidLen
 	}
@@ -52,12 +57,10 @@ func (d *Decoder) readLine() (string, error) {
 	return string(line), nil
 }
 
-func (d *Decoder) ReadLine() (string, error) {
-	return d.readLine()
-}
-
+// ReadBlock reads and return multiple pkt-line lines, it stops at the end
+// of the reader or if a flush-pkt is reached
 func (d *Decoder) ReadBlock() ([]string, error) {
-	o := make([]string, 0)
+	var o []string
 
 	for {
 		line, err := d.readLine()
@@ -75,10 +78,9 @@ func (d *Decoder) ReadBlock() ([]string, error) {
 
 		o = append(o, line)
 	}
-
-	return o, nil
 }
 
+// ReadAll read and returns all the lines
 func (d *Decoder) ReadAll() ([]string, error) {
 	result, err := d.ReadBlock()
 	if err != nil {
@@ -101,6 +103,4 @@ func (d *Decoder) ReadAll() ([]string, error) {
 
 		result = append(result, lines...)
 	}
-
-	return result, nil
 }
diff --git a/pktline/decoder_test.go b/formats/pktline/decoder_test.go
index 7899cc8..e3cd2f4 100644
--- a/pktline/decoder_test.go
+++ b/formats/pktline/decoder_test.go
@@ -14,23 +14,30 @@ type DecoderSuite struct{}
 var _ = Suite(&DecoderSuite{})
 
 func (s *DecoderSuite) TestReadLine(c *C) {
-	j := &Decoder{strings.NewReader("0006a\n")}
+	j := NewDecoder(strings.NewReader("0006a\n"))
 
 	line, err := j.ReadLine()
 	c.Assert(err, IsNil)
 	c.Assert(line, Equals, "a\n")
 }
 
+func (s *DecoderSuite) TestReadLineInvalidHeader(c *C) {
+	j := NewDecoder(strings.NewReader("foo\n"))
+
+	_, err := j.ReadLine()
+	c.Assert(err, Equals, ErrInvalidHeader)
+}
+
 func (s *DecoderSuite) TestReadLineBufferUnderflow(c *C) {
-	j := &Decoder{strings.NewReader("00e7a\n")}
+	j := NewDecoder(strings.NewReader("00e7a\n"))
 
 	line, err := j.ReadLine()
 	c.Assert(err, Equals, ErrUnderflow)
 	c.Assert(line, Equals, "")
 }
 
-func (s *DecoderSuite) TestReadLineBufferInvalidLen(c *C) {
-	j := &Decoder{strings.NewReader("0001foo\n")}
+func (s *DecoderSuite) TestReadLineInvalidLen(c *C) {
+	j := NewDecoder(strings.NewReader("0001foo\n"))
 
 	line, err := j.ReadLine()
 	c.Assert(err, Equals, ErrInvalidLen)
@@ -38,7 +45,7 @@ func (s *DecoderSuite) TestReadLineBufferInvalidLen(c *C) {
 }
 
 func (s *DecoderSuite) TestReadBlock(c *C) {
-	j := &Decoder{strings.NewReader("0006a\n")}
+	j := NewDecoder(strings.NewReader("0006a\n"))
 
 	lines, err := j.ReadBlock()
 	c.Assert(err, IsNil)
@@ -47,7 +54,7 @@ func (s *DecoderSuite) TestReadBlock(c *C) {
 }
 
 func (s *DecoderSuite) TestReadBlockWithFlush(c *C) {
-	j := &Decoder{strings.NewReader("0006a\n0006b\n00000006c\n")}
+	j := NewDecoder(strings.NewReader("0006a\n0006b\n00000006c\n"))
 
 	lines, err := j.ReadBlock()
 	c.Assert(err, IsNil)
@@ -56,8 +63,16 @@ func (s *DecoderSuite) TestReadBlockWithFlush(c *C) {
 	c.Assert(lines[1], Equals, "b\n")
 }
 
+func (s *DecoderSuite) TestReadBlockInvalidLen(c *C) {
+	j := NewDecoder(strings.NewReader("0001foo\n"))
+
+	lines, err := j.ReadBlock()
+	c.Assert(err, Equals, ErrInvalidLen)
+	c.Assert(lines, HasLen, 0)
+}
+
 func (s *DecoderSuite) TestReadAll(c *C) {
-	j := &Decoder{strings.NewReader("0006a\n0006b\n00000006c\n0006d\n0006e\n")}
+	j := NewDecoder(strings.NewReader("0006a\n0006b\n00000006c\n0006d\n0006e\n"))
 
 	lines, err := j.ReadAll()
 	c.Assert(err, IsNil)
diff --git a/pktline/doc.go b/formats/pktline/doc.go
index a976b54..0ae22e3 100644
--- a/pktline/doc.go
+++ b/formats/pktline/doc.go
@@ -51,6 +51,6 @@ package pktline
 // https://github.com/git/git/blob/master/Documentation/technical/protocol-common.txt
 
 const (
-	HEADER_LENGTH = 4
-	MAX_LENGTH    = 65524
+	HeaderLength = 4
+	MaxLength    = 65524
 )
diff --git a/pktline/encoder.go b/formats/pktline/encoder.go
index 9c00126..dfa53e2 100644
--- a/pktline/encoder.go
+++ b/formats/pktline/encoder.go
@@ -10,14 +10,17 @@ var (
 	ErrOverflow = errors.New("unexpected string length (overflow)")
 )
 
+// Encoder implements a pkt-line format encoder
 type Encoder struct {
 	lines []string
 }
 
+// NewEncoder returns a new Encoder
 func NewEncoder() *Encoder {
 	return &Encoder{make([]string, 0)}
 }
 
+// AddLine encode and adds a line to the encoder
 func (e *Encoder) AddLine(line string) error {
 	le, err := EncodeFromString(line + "\n")
 	if err != nil {
@@ -28,27 +31,31 @@ func (e *Encoder) AddLine(line string) error {
 	return nil
 }
 
+// AddFlush adds a flush-pkt to the encoder
 func (e *Encoder) AddFlush() {
 	e.lines = append(e.lines, "0000")
 }
 
-func (e *Encoder) GetReader() *strings.Reader {
+// Reader returns a string.Reader over the encoder
+func (e *Encoder) Reader() *strings.Reader {
 	data := strings.Join(e.lines, "")
 
 	return strings.NewReader(data)
 }
 
+// EncodeFromString encodes a string to pkt-line format
 func EncodeFromString(line string) (string, error) {
 	return Encode([]byte(line))
 }
 
+// Encode encodes a byte slice to pkt-line format
 func Encode(line []byte) (string, error) {
 	if line == nil {
 		return "0000", nil
 	}
 
-	l := len(line) + HEADER_LENGTH
-	if l > MAX_LENGTH {
+	l := len(line) + HeaderLength
+	if l > MaxLength {
 		return "", ErrOverflow
 	}
 
diff --git a/pktline/encoder_test.go b/formats/pktline/encoder_test.go
index 091ad1c..f718c33 100644
--- a/pktline/encoder_test.go
+++ b/formats/pktline/encoder_test.go
@@ -1,7 +1,9 @@
 package pktline
 
 import (
+	"bytes"
 	"io/ioutil"
+	"strings"
 
 	. "gopkg.in/check.v1"
 )
@@ -16,6 +18,17 @@ func (s *EncoderSuite) TestEncode(c *C) {
 	c.Assert(string(line), Equals, "0006a\n")
 }
 
+func (s *EncoderSuite) TestEncodeNil(c *C) {
+	line, err := Encode(nil)
+	c.Assert(err, IsNil)
+	c.Assert(string(line), Equals, "0000")
+}
+
+func (s *EncoderSuite) TestEncodeOverflow(c *C) {
+	_, err := Encode(bytes.Repeat([]byte{'0'}, MaxLength+1))
+	c.Assert(err, Equals, ErrOverflow)
+}
+
 func (s *EncoderSuite) TestEncodeFromString(c *C) {
 	line, err := EncodeFromString("a\n")
 	c.Assert(err, IsNil)
@@ -24,11 +37,14 @@ func (s *EncoderSuite) TestEncodeFromString(c *C) {
 
 func (s *EncoderSuite) TestEncoder(c *C) {
 	e := NewEncoder()
-	e.AddLine("a")
+	c.Assert(e.AddLine("a"), IsNil)
 	e.AddFlush()
-	e.AddLine("b")
+	c.Assert(e.AddLine("b"), IsNil)
+
+	over := strings.Repeat("0", MaxLength+1)
+	c.Assert(e.AddLine(over), Equals, ErrOverflow)
 
-	r := e.GetReader()
+	r := e.Reader()
 	a, _ := ioutil.ReadAll(r)
 	c.Assert(string(a), Equals, "0006a\n00000006b\n")
 }