Start to add tests and fix issues they're throwing up

Adding tests are overdue and it's showing up the areas that I haven't
got right, especially in the packets library

eclipse-paho#5

packets/connack.go

@@ -36,7 +36,19 @@ func (c *Connack) Unpack(r *bytes.Buffer) error {
 
 // Buffers is the implementation of the interface required function for a packet
 func (c *Connack) Buffers() net.Buffers {
-	return nil
+	var header bytes.Buffer
+
+	if c.SessionPresent {
+		header.WriteByte(1)
+	} else {
+		header.WriteByte(0)
+	}
+	header.WriteByte(c.ReasonCode)
+
+	idvp := c.Properties.Pack(CONNACK)
+	propLen := encodeVBI(len(idvp))
+
+	return net.Buffers{header.Bytes(), propLen, idvp}
 }
 
 // WriteTo is the implementation of the interface required function for a packet

packets/packets.go

@@ -176,6 +176,7 @@ func ReadPacket(r io.Reader) (*ControlPacket, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	if n != int64(cp.remainingLength) {
 		return nil, fmt.Errorf("failed to read packet, expected %d bytes, read %d", cp.remainingLength, n)
 	}

packets/packets_test.go

@@ -3,6 +3,7 @@ package packets
 import (
 	"bufio"
 	"bytes"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -142,3 +143,254 @@ func TestReadStringWriteString(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, "Test string", s)
 }
+
+func TestNewControlPacket(t *testing.T) {
+	tests := []struct {
+		name string
+		args PacketType
+		want *ControlPacket
+	}{
+		{
+			name: "connect",
+			args: CONNECT,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: CONNECT},
+				Content: &Connect{
+					ProtocolName:    "MQTT",
+					ProtocolVersion: 5,
+					Properties:      &Properties{User: make(map[string]string)},
+				},
+			},
+		},
+		{
+			name: "connack",
+			args: CONNACK,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: CONNACK},
+				Content:     &Connack{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "publish",
+			args: PUBLISH,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PUBLISH},
+				Content:     &Publish{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "puback",
+			args: PUBACK,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PUBACK},
+				Content:     &Puback{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "pubrec",
+			args: PUBREC,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PUBREC},
+				Content:     &Pubrec{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "pubrel",
+			args: PUBREL,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PUBREL, Flags: 2},
+				Content:     &Pubrel{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "pubcomp",
+			args: PUBCOMP,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PUBCOMP},
+				Content:     &Pubcomp{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "subscribe",
+			args: SUBSCRIBE,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: SUBSCRIBE, Flags: 2},
+				Content: &Subscribe{
+					Properties:    &Properties{User: make(map[string]string)},
+					Subscriptions: make(map[string]SubOptions),
+				},
+			},
+		},
+		{
+			name: "suback",
+			args: SUBACK,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: SUBACK},
+				Content:     &Suback{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "unsubscribe",
+			args: UNSUBSCRIBE,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: UNSUBSCRIBE, Flags: 2},
+				Content:     &Unsubscribe{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "unsuback",
+			args: UNSUBACK,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: UNSUBACK},
+				Content:     &Unsuback{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "pingreq",
+			args: PINGREQ,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PINGREQ},
+				Content:     &Pingreq{},
+			},
+		},
+		{
+			name: "pingresp",
+			args: PINGRESP,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: PINGRESP},
+				Content:     &Pingresp{},
+			},
+		},
+		{
+			name: "disconnect",
+			args: DISCONNECT,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: DISCONNECT},
+				Content:     &Disconnect{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "auth",
+			args: AUTH,
+			want: &ControlPacket{
+				FixedHeader: FixedHeader{Type: AUTH, Flags: 1},
+				Content:     &Auth{Properties: &Properties{User: make(map[string]string)}},
+			},
+		},
+		{
+			name: "dummy",
+			args: 20,
+			want: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := NewControlPacket(tt.args); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("NewControlPacket() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestControlPacket_PacketID(t *testing.T) {
+	type fields struct {
+		Content     Packet
+		FixedHeader FixedHeader
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   uint16
+	}{
+		{
+			name: "publish",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: PUBLISH},
+				Content:     &Publish{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "puback",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: PUBACK},
+				Content:     &Puback{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "pubrel",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: PUBREL},
+				Content:     &Pubrel{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "pubrec",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: PUBREC},
+				Content:     &Pubrec{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "pubcomp",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: PUBCOMP},
+				Content:     &Pubcomp{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "subscribe",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: SUBSCRIBE},
+				Content:     &Subscribe{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "suback",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: SUBACK},
+				Content:     &Suback{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "unsubscribe",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: UNSUBSCRIBE},
+				Content:     &Unsubscribe{PacketID: 123},
+			},
+			want: 123,
+		},
+		{
+			name: "unsuback",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: UNSUBACK},
+				Content:     &Unsuback{PacketID: 123},
+			},
+			want: 123,
+		}, {
+			name: "connect",
+			fields: fields{
+				FixedHeader: FixedHeader{Type: CONNECT},
+				Content:     &Connect{},
+			},
+			want: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &ControlPacket{
+				Content:     tt.fields.Content,
+				FixedHeader: tt.fields.FixedHeader,
+			}
+			if got := c.PacketID(); got != tt.want {
+				t.Errorf("ControlPacket.PacketID() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

packets/subscribe.go

@@ -36,6 +36,20 @@ func (s *SubOptions) Pack() byte {
 	return ret
 }
 
+func (s *SubOptions) Unpack(r *bytes.Buffer) error {
+	b, err := r.ReadByte()
+	if err != nil {
+		return err
+	}
+
+	s.QoS = b & 0x03
+	s.NoLocal = (b & 1 << 2) == 1
+	s.RetainAsPublished = (b & 1 << 3) == 1
+	s.RetainHandling = b & 0x30
+
+	return nil
+}
+
 // Unpack is the implementation of the interface required function for a packet
 func (s *Subscribe) Unpack(r *bytes.Buffer) error {
 	var err error
@@ -49,6 +63,18 @@ func (s *Subscribe) Unpack(r *bytes.Buffer) error {
 		return err
 	}
 
+	for r.Len() > 0 {
+		var so SubOptions
+		t, err := readString(r)
+		if err != nil {
+			return err
+		}
+		if err = so.Unpack(r); err != nil {
+			return err
+		}
+		s.Subscriptions[t] = so
+	}
+
 	return nil
 }
 

packets/unsubscribe.go

@@ -15,6 +15,17 @@ type Unsubscribe struct {
 
 // Unpack is the implementation of the interface required function for a packet
 func (u *Unsubscribe) Unpack(r *bytes.Buffer) error {
+	var err error
+	u.PacketID, err = readUint16(r)
+	if err != nil {
+		return err
+	}
+
+	err = u.Properties.Unpack(r, UNSUBSCRIBE)
+	if err != nil {
+		return err
+	}
+
 	for {
 		t, err := readString(r)
 		if err != nil && err != io.EOF {
@@ -33,10 +44,13 @@ func (u *Unsubscribe) Unpack(r *bytes.Buffer) error {
 func (u *Unsubscribe) Buffers() net.Buffers {
 	var b bytes.Buffer
 	writeUint16(u.PacketID, &b)
+	var topics bytes.Buffer
 	for _, t := range u.Topics {
-		writeString(t, &b)
+		writeString(t, &topics)
 	}
-	return net.Buffers{b.Bytes()}
+	idvp := u.Properties.Pack(UNSUBSCRIBE)
+	propLen := encodeVBI(len(idvp))
+	return net.Buffers{b.Bytes(), propLen, idvp, topics.Bytes()}
 }
 
 // WriteTo is the implementation of the interface required function for a packet