Added several tests for metrics package

pkg/metrics/controller_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"go.opencensus.io/metric/metricdata"
 	"go.opencensus.io/metric/metricexport"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
@@ -396,3 +397,46 @@ func TestCalcDuration(t *testing.T) {
 	}
 	assert.Len(t, c.gameServerStateLastChange.Keys(), 0, "We should not have any keys after the test")
 }
+
+func TestIsSystemNode(t *testing.T) {
+	cases := []struct {
+		desc     string
+		node     *corev1.Node
+		expected bool
+	}{
+		{
+			desc: "Is system node, true expected",
+			node: &corev1.Node{
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{{Key: "agones.dev/test"}},
+				},
+			},
+			expected: true,
+		},
+		{
+			desc: "Not a system node, false expected",
+			node: &corev1.Node{
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{{Key: "qwerty.dev/test"}},
+				},
+			},
+			expected: false,
+		},
+		{
+			desc: "Empty taints, false expected",
+			node: &corev1.Node{
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{},
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			res := isSystemNode(tc.node)
+			assert.Equal(t, tc.expected, res)
+		})
+	}
+}

pkg/metrics/util.go

@@ -68,12 +68,23 @@ func parseLabels(s string) (*stackdriver.Labels, error) {
 		}
 		key := strings.TrimSpace(keyValue[0])
 		value := strings.TrimSpace(keyValue[1])
-		if !utf8.ValidString(key) || !utf8.ValidString(value) {
-			return nil, errors.New("invalid labels: must be a valid utf-8 string")
+
+		if key == "" {
+			return nil, errors.New("invalid key: can not be empty")
+		}
+
+		if value == "" {
+			return nil, fmt.Errorf("invalid value for key %s: can not be empty", key)
 		}
-		if key == "" || value == "" {
-			return nil, errors.New("invalid labels: must not be empty string")
+
+		if !utf8.ValidString(key) {
+			return nil, fmt.Errorf("invalid key: %s, must be a valid utf-8 string", key)
 		}
+
+		if !utf8.ValidString(value) {
+			return nil, fmt.Errorf("invalid value: %s, must be a valid utf-8 string", value)
+		}
+
 		res.Set(key, value, "")
 	}
 	return res, nil

pkg/metrics/util_test.go

@@ -16,12 +16,14 @@ package metrics
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1"
 	agtesting "agones.dev/agones/pkg/testing"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -186,3 +188,93 @@ func fleetAutoScaler(fleetName string, fasName string) *autoscalingv1.FleetAutos
 		},
 	}
 }
+
+func TestParseLabels(t *testing.T) {
+	cases := []struct {
+		desc     string
+		labels   string
+		expected map[string]string
+		err      string
+	}{
+		{
+			desc:     "Two valid labels, no error",
+			labels:   "l1=1,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "",
+		},
+		{
+			desc:     "Empty input string, empty struct expected",
+			labels:   "",
+			expected: map[string]string{},
+			err:      "",
+		},
+		{
+			desc:     "Valid labels, invalid separator, error expected",
+			labels:   "l1=1|l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid labels: l1=1|l2=2, expect key=value,key2=value2",
+		},
+		{
+			desc:     "Two valid labels with extra spaces, error expected",
+			labels:   "   l1=1,   l2=2   ",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "",
+		},
+		{
+			desc:     "Two invalid labels, error expected",
+			labels:   "l1-1,l2-2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid labels: l1-1,l2-2, expect key=value,key2=value2",
+		},
+		{
+			desc:     "Invalid key utf8 string, error expected",
+			labels:   "\xF4=1,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid key: \xF4, must be a valid utf-8 string",
+		},
+		{
+			desc:     "Invalid value utf8 string, error expected",
+			labels:   "l1=\xF4,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid value: \xF4, must be a valid utf-8 string",
+		},
+		{
+			desc:     "Empty key, error expected",
+			labels:   " =1,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid key: can not be empty",
+		},
+		{
+			desc:     "Empty value, error expected",
+			labels:   "l1= ,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid value for key l1: can not be empty",
+		},
+		{
+			desc:     "Empty key and value, key err excpected",
+			labels:   " = ,l2=2",
+			expected: map[string]string{"l1": "1", "l2": "2"},
+			err:      "invalid key: can not be empty",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			res, err := parseLabels(tc.labels)
+
+			if tc.err != "" {
+				require.Error(t, err)
+				assert.Equal(t, tc.err, err.Error())
+			} else {
+				require.NoError(t, err)
+				// retrieve inner map
+				m := reflect.ValueOf(res).Elem().FieldByName("m").MapRange()
+				for m.Next() {
+					val, ok := tc.expected[m.Key().String()]
+					require.True(t, ok)
+					assert.Equal(t, val, m.Value().FieldByName("val").String())
+				}
+			}
+		})
+	}
+}