more test coverage, responding to comments

cmd/influxd/config.go

@@ -38,18 +38,20 @@ const (
 // Config represents the configuration format for the influxd binary.
 type (
 	Collectd struct {
-		Addr     string `toml:"address"`
+		Addr string `toml:"address"`
+		Port uint16 `toml:"port"`
+
 		Database string `toml:"database"`
 		Enabled  bool   `toml:"enabled"`
-		Port     uint16 `toml:"port"`
 		TypesDB  string `toml:"typesdb"`
 	}
 
 	Graphite struct {
-		Addr          string `toml:"address"`
+		Addr string `toml:"address"`
+		Port uint16 `toml:"port"`
+
 		Database      string `toml:"database"`
 		Enabled       bool   `toml:"enabled"`
-		Port          uint16 `toml:"port"`
 		Protocol      string `toml:"protocol"`
 		NamePosition  string `toml:"name-position"`
 		NameSeparator string `toml:"name-separator"`

cmd/influxd/config_test.go

@@ -310,13 +310,13 @@ func Test_Collectd_ConnectionString(t *testing.T) {
 			config:           main.Collectd{},
 		},
 		{
-			name:             "address provide, no port provided from config",
+			name:             "address provided, no port provided from config",
 			defaultBindAddr:  "192.168.0.1",
 			connectionString: "192.168.0.2:25826",
 			config:           main.Collectd{Addr: "192.168.0.2"},
 		},
 		{
-			name:             "no address provides, port provided from config",
+			name:             "no address provided, port provided from config",
 			defaultBindAddr:  "192.168.0.1",
 			connectionString: "192.168.0.1:25827",
 			config:           main.Collectd{Port: 25827},
@@ -333,7 +333,7 @@ func Test_Collectd_ConnectionString(t *testing.T) {
 		t.Logf("test: %q", test.name)
 		s := test.config.ConnectionString(test.defaultBindAddr)
 		if s != test.connectionString {
-			t.Errorf("connectection string mistmatch, expected: %q, got: %q", test.connectionString, s)
+			t.Errorf("connection string mismatch, expected: %q, got: %q", test.connectionString, s)
 		}
 	}
 }

cmd/influxd/run.go

@@ -97,9 +97,9 @@ func execRun(args []string) {
 		// Spin up the collectd server
 		if config.Collectd.Enabled {
 			c := config.Collectd
-			s := collectd.NewServer(s, c.TypesDB)
-			s.Database = c.Database
-			err := s.ListenAndServe(c.ConnectionString(config.BindAddress))
+			cs := collectd.NewServer(s, c.TypesDB)
+			cs.Database = c.Database
+			err := cs.ListenAndServe(c.ConnectionString(config.BindAddress))
 			if err != nil {
 				log.Println("failed to start collectd Server", err.Error())
 			}

collectd/collectd.go

@@ -28,6 +28,12 @@ var (
 
 	// ErrServerClosed return when closing an already closed graphite server.
 	ErrServerClosed = errors.New("server already closed")
+
+	// ErrResolveUDPAddr returned when we are unable to resolve a udp address
+	ErrResolveUDPAddr = errors.New("Unable to resolve UDP address")
+
+	// ErrListenUDP returned when we are unable to resolve a udp address
+	ErrListenUDP = errors.New("Unable to listen on UDP")
 )
 
 // SeriesWriter defines the interface for the destination of the data.
@@ -58,9 +64,6 @@ func NewServer(w SeriesWriter, typesDBPath string) *Server {
 }
 
 func (s *Server) ListenAndServe(iface string) error {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
 	if iface == "" { // Make sure we have an address
 		return ErrBindAddressRequired
 	} else if s.Database == "" { // Make sure they have a database
@@ -75,12 +78,12 @@ func (s *Server) ListenAndServe(iface string) error {
 
 	addr, err := net.ResolveUDPAddr("udp", iface)
 	if err != nil {
-		return nil
+		return ErrResolveUDPAddr
 	}
 
 	conn, err := net.ListenUDP("udp", addr)
 	if err != nil {
-		return err
+		return ErrListenUDP
 	}
 	s.conn = conn
 
@@ -110,9 +113,7 @@ func (s *Server) serve(conn *net.UDPConn) {
 	for {
 		select {
 		case <-s.done:
-			s.mu.Lock()
 			s.conn.Close()
-			s.mu.Unlock()
 			return
 		default:
 			n, _, err := conn.ReadFromUDP(buffer)

collectd/collectd_test.go

@@ -2,6 +2,7 @@ package collectd_test
 
 import (
 	"fmt"
+	"net"
 	"testing"
 	"time"
 
@@ -72,6 +73,39 @@ func TestServer_ListenAndServe_Success(t *testing.T) {
 	}
 }
 
+func TestServer_ListenAndServe_ErrResolveUDPAddr(t *testing.T) {
+	var (
+		ts  testServer
+		s   = collectd.NewServer(ts, "./collectd_test.conf")
+		err = collectd.ErrResolveUDPAddr
+	)
+
+	s.Database = "counter"
+	e := s.ListenAndServe("foo")
+	if e != err {
+		t.Fatalf("err does not match.  expected %v, got %v", err, e)
+	}
+}
+
+func TestServer_ListenAndServe_ErrListenUDP(t *testing.T) {
+	var (
+		ts  testServer
+		s   = collectd.NewServer(ts, "./collectd_test.conf")
+		err = collectd.ErrListenUDP
+	)
+
+	//Open a udp listener on the port prior to force it to err
+	addr, _ := net.ResolveUDPAddr("udp", "127.0.0.1:25826")
+	conn, _ := net.ListenUDP("udp", addr)
+	defer conn.Close()
+
+	s.Database = "counter"
+	e := s.ListenAndServe("127.0.0.1:25826")
+	if e != err {
+		t.Fatalf("err does not match.  expected %v, got %v", err, e)
+	}
+}
+
 func Test_Unmarshal_Metrics(t *testing.T) {
 	/*
 	   This is a sample of what data can be represented like in json