Refactoring agent/utils/utils_test.go

- Use subtests for organization / improved output - Use testify/assert for clarity
aws · Aug 1, 2017 · 4a6cda3 · 4a6cda3
1 parent 7f3d187
commit 4a6cda3
Showing 1 changed file with 64 additions and 103 deletions.
diff --git a/agent/utils/utils_test.go b/agent/utils/utils_test.go
@@ -21,94 +21,72 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/utils/ttime"
 	"github.com/aws/amazon-ecs-agent/agent/utils/ttime/mocks"
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestDefaultIfBlank(t *testing.T) {
 	const defaultValue = "a boring default"
 	const specifiedValue = "new value"
 	result := DefaultIfBlank(specifiedValue, defaultValue)
-
-	if result != specifiedValue {
-		t.Error("Expected " + specifiedValue + ", got " + result)
-	}
+	assert.Equal(t, specifiedValue, result)
 
 	result = DefaultIfBlank("", defaultValue)
-	if result != defaultValue {
-		t.Error("Expected " + defaultValue + ", got " + result)
-	}
+	assert.Equal(t, defaultValue, result)
 }
 
 func TestZeroOrNil(t *testing.T) {
-
-	if !ZeroOrNil(nil) {
-		t.Error("Nil is nil")
-	}
-
-	if !ZeroOrNil(0) {
-		t.Error("0 is 0")
-	}
-
-	if !ZeroOrNil("") {
-		t.Error("\"\" is the string zerovalue")
-	}
-
 	type ZeroTest struct {
 		testInt int
 		TestStr string
 	}
 
-	if !ZeroOrNil(ZeroTest{}) {
-		t.Error("ZeroTest zero-value should be zero")
-	}
-
-	if ZeroOrNil(ZeroTest{TestStr: "asdf"}) {
-		t.Error("ZeroTest with a field populated isn't zero")
-	}
-
-	if ZeroOrNil(1) {
-		t.Error("1 is not 0")
-	}
-
-	uintSlice := []uint16{1, 2, 3}
-	if ZeroOrNil(uintSlice) {
-		t.Error("[1,2,3] is not zero")
-	}
-
-	uintSlice = []uint16{}
-	if !ZeroOrNil(uintSlice) {
-		t.Error("[] is Zero")
-	}
-
-	if ZeroOrNil(struct{ uncomparable []uint16 }{uncomparable: []uint16{1, 2, 3}}) {
-		t.Error("Uncomparable structs are never zero")
-	}
-
-	if ZeroOrNil(struct{ uncomparable []uint16 }{uncomparable: nil}) {
-		t.Error("Uncomparable structs are never zero")
-	}
-
 	var strMap map[string]string
-	if !ZeroOrNil(strMap) {
-		t.Error("map[string]string is not zero or nil")
-	}
 
-	if ZeroOrNil(map[string]string{"foo": "bar"}) {
-		t.Error("map[string]string{foo:bar} is zero or nil")
+	testCases := []struct {
+		param    interface{}
+		expected bool
+		name     string
+	}{
+		{nil, true, "Nil is nil"},
+		{0, true, "0 is 0"},
+		{"", true, "\"\" is the string zerovalue"},
+		{ZeroTest{}, true, "ZeroTest zero-value should be zero"},
+		{ZeroTest{TestStr: "asdf"}, false, "ZeroTest with a field populated isn't zero"},
+		{1, false, "1 is not 0"},
+		{[]uint16{1, 2, 3}, false, "[1,2,3] is not zero"},
+		{[]uint16{}, true, "[] is zero"},
+		{struct{ uncomparable []uint16 }{uncomparable: []uint16{1, 2, 3}}, false, "Uncomparable structs are never zero"},
+		{struct{ uncomparable []uint16 }{uncomparable: nil}, false, "Uncomparable structs are never zero"},
+		{strMap, true, "map[string]string is zero or nil"},
+		{make(map[string]string), true, "empty map[string]string is zero or nil"},
+		{map[string]string{"foo": "bar"}, false, "map[string]string{foo:bar} is not zero or nil"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, ZeroOrNil(tc.param), tc.name)
+		})
 	}
 }
 
 func TestSlicesDeepEqual(t *testing.T) {
-	if !SlicesDeepEqual([]string{}, []string{}) {
-		t.Error("Empty slices should be equal")
-	}
-	if SlicesDeepEqual([]string{"cat"}, []string{}) {
-		t.Error("Should not be equal")
-	}
-	if !SlicesDeepEqual([]string{"cat"}, []string{"cat"}) {
-		t.Error("Should be equal")
-	}
-	if !SlicesDeepEqual([]string{"cat", "dog", "cat"}, []string{"dog", "cat", "cat"}) {
-		t.Error("Should be equal")
+	testCases := []struct {
+		left     []string
+		right    []string
+		expected bool
+		name     string
+	}{
+		{[]string{}, []string{}, true, "Two empty slices"},
+		{[]string{"cat"}, []string{}, false, "One empty slice"},
+		{[]string{}, []string{"cat"}, false, "Another empty slice"},
+		{[]string{"cat"}, []string{"cat"}, true, "Two slices with one element each"},
+		{[]string{"cat", "dog", "cat"}, []string{"dog", "cat", "cat"}, true, "Two slices with multiple elements"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, SlicesDeepEqual(tc.left, tc.right))
+		})
 	}
 }
 
@@ -128,9 +106,7 @@ func TestRetryWithBackoff(t *testing.T) {
 		counter--
 		return errors.New("err")
 	})
-	if counter != 0 {
-		t.Error("Counter didn't go to 0; didn't get retried enough")
-	}
+	assert.Equal(t, 0, counter, "Counter didn't go to 0; didn't get retried enough")
 
 	// no sleeps
 	RetryWithBackoff(NewSimpleBackoff(10*time.Second, 20*time.Second, 0, 2), func() error {
@@ -152,12 +128,8 @@ func TestRetryNWithBackoff(t *testing.T) {
 		counter--
 		return errors.New("err")
 	})
-	if counter != 1 {
-		t.Error("Should have stopped after two tries")
-	}
-	if err == nil {
-		t.Error("Should have returned appropriate error")
-	}
+	assert.Equal(t, 1, counter, "Should have stopped after two tries")
+	assert.Error(t, err)
 
 	// 3 tries, 2 sleeps
 	mocktime.EXPECT().Sleep(100 * time.Millisecond).Times(2)
@@ -170,44 +142,33 @@ func TestRetryNWithBackoff(t *testing.T) {
 		return errors.New("err")
 	})
 
-	if counter != 0 {
-		t.Errorf("Counter expected to be 0, was %v", counter)
-	}
-	if err != nil {
-		t.Errorf("Expected no error, got %v", err)
-	}
+	assert.Equal(t, 0, counter)
+	assert.Nil(t, err)
 }
 
 func TestParseBool(t *testing.T) {
-
 	truthyStrings := []string{"true", "1", "t", "true\r", "true ", "true \r"}
 	falsyStrings := []string{"false", "0", "f", "false\r", "false ", "false \r"}
 	neitherStrings := []string{"apple", " ", "\r", "orange", "maybe"}
 
-	for ndx, str := range truthyStrings {
-		if !ParseBool(str, false) {
-			t.Fatal("Truthy string should be truthy: ", ndx)
-		}
-		if !ParseBool(str, true) {
-			t.Fatal("Truthy string should be truthy (regardless of default): ", ndx)
-		}
+	for _, str := range truthyStrings {
+		t.Run("truthy", func(t *testing.T) {
+			assert.True(t, ParseBool(str, false), "Truthy string should be truthy")
+			assert.True(t, ParseBool(str, true), "Truthy string should be truthy (regardless of default)")
+		})
 	}
 
-	for ndx, str := range falsyStrings {
-		if ParseBool(str, false) {
-			t.Fatal("Falsy string should be falsy: ", ndx)
-		}
-		if ParseBool(str, true) {
-			t.Fatal("falsy string should be falsy (regardless of default): ", ndx)
-		}
+	for _, str := range falsyStrings {
+		t.Run("falsy", func(t *testing.T) {
+			assert.False(t, ParseBool(str, false), "Falsy string should be falsy")
+			assert.False(t, ParseBool(str, true), "Falsy string should be falsy (regardless of default)")
+		})
 	}
 
-	for ndx, str := range neitherStrings {
-		if ParseBool(str, false) {
-			t.Fatal("Should default to false", ndx)
-		}
-		if !ParseBool(str, true) {
-			t.Fatal("Should default to true", ndx)
-		}
+	for _, str := range neitherStrings {
+		t.Run("defaults", func(t *testing.T) {
+			assert.False(t, ParseBool(str, false), "Should default to false")
+			assert.True(t, ParseBool(str, true), "Should default to true")
+		})
 	}
 }