chore: improve test clarity (#675)

* read-only-port defaults are correct

* Tests that should catch good read-only-port

* Rework checks & tests

* Linting on issue template YAML

* More explicit test for 4.2.4

* Remove verbosity for ease of reading results

* Use subtests

* Tidy more test cases

check/check_test.go

@@ -1,4 +1,4 @@
-// Copyright © 2017-2019 Aqua Security Software Ltd. <info@aquasec.com>
+// Copyright © 2017-2020 Aqua Security Software Ltd. <info@aquasec.com>
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -53,8 +53,8 @@ func TestCheck_Run(t *testing.T) {
 				Tests: &tests{TestItems: []*testItem{{
 					Flag: "hello",
 					Set:  false,
-				}},
+				}}},
-				}},
+			},
 			Expected: FAIL,
 		},
 		{
@@ -70,97 +70,58 @@ func TestCheck_Run(t *testing.T) {
 			Expected: PASS,
 		},
 	}
+
 	for _, testCase := range testCases {
-
+		t.Run(testCase.name, func(t *testing.T) {
 			testCase.check.run()
-
 			if testCase.check.State != testCase.Expected {
-			t.Errorf("%s: expected %s, actual %s\n", testCase.name, testCase.Expected, testCase.check.State)
+				t.Errorf("expected %s, actual %s", testCase.Expected, testCase.check.State)
 			}
+		})
 	}
 }
 
 func TestCheckAuditConfig(t *testing.T) {
 
-	cases := []struct {
+	passingCases := []*Check{
-		*Check
-		expected State
-	}{
-		{
 		controls.Groups[1].Checks[0],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[1],
-			"FAIL",
-		},
-		{
-			controls.Groups[1].Checks[2],
-			"FAIL",
-		},
-		{
 		controls.Groups[1].Checks[3],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[4],
-			"FAIL",
-		},
-		{
 		controls.Groups[1].Checks[5],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[6],
-			"FAIL",
-		},
-		{
 		controls.Groups[1].Checks[7],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[8],
-			"FAIL",
-		},
-		{
 		controls.Groups[1].Checks[9],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[10],
-			"FAIL",
-		},
-		{
-			controls.Groups[1].Checks[11],
-			"FAIL",
-		},
-		{
-			controls.Groups[1].Checks[12],
-			"FAIL",
-		},
-		{
-			controls.Groups[1].Checks[13],
-			"FAIL",
-		},
-		{
-			controls.Groups[1].Checks[14],
-			"FAIL",
-		},
-		{
 		controls.Groups[1].Checks[15],
-			"PASS",
-		},
-		{
-			controls.Groups[1].Checks[16],
-			"FAIL",
-		},
 	}
 
-	for _, c := range cases {
+	failingCases := []*Check{
-		c.run()
+		controls.Groups[1].Checks[1],
-		if c.State != c.expected {
+		controls.Groups[1].Checks[2],
-			t.Errorf("%s, expected:%v, got:%v\n", c.Text, c.expected, c.State)
+		controls.Groups[1].Checks[4],
+		controls.Groups[1].Checks[6],
+		controls.Groups[1].Checks[8],
+		controls.Groups[1].Checks[10],
+		controls.Groups[1].Checks[11],
+		controls.Groups[1].Checks[12],
+		controls.Groups[1].Checks[13],
+		controls.Groups[1].Checks[14],
+		controls.Groups[1].Checks[16],
 	}
+
+	for _, c := range passingCases {
+		t.Run(c.Text, func(t *testing.T) {
+			c.run()
+			if c.State != "PASS" {
+				t.Errorf("Should PASS, got: %v", c.State)
+			}
+		})
+	}
+
+	for _, c := range failingCases {
+		t.Run(c.Text, func(t *testing.T) {
+			c.run()
+			if c.State != "FAIL" {
+				t.Errorf("Should FAIL, got: %v", c.State)
+			}
+		})
 	}
 }
 

check/test_test.go

@@ -1,4 +1,4 @@
-// Copyright © 2017 Aqua Security Software Ltd. <info@aquasec.com>
+// Copyright © 2017-2020 Aqua Security Software Ltd. <info@aquasec.com>
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
 package check
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"strings"
@@ -216,6 +217,7 @@ func TestTestExecute(t *testing.T) {
 	}
 
 	for _, c := range cases {
+		t.Run(c.Text, func(t *testing.T) {
 			c.Check.AuditOutput = c.str
 			c.Check.AuditConfigOutput = c.strConfig
 			res, err := c.Check.execute()
@@ -223,8 +225,9 @@ func TestTestExecute(t *testing.T) {
 				t.Errorf(err.Error())
 			}
 			if !res.testResult {
-			t.Errorf("%s, expected:%v, got:%v\n", c.Text, true, res)
+				t.Errorf("expected:%v, got:%v", true, res)
 			}
+		})
 	}
 }
 
@@ -257,14 +260,16 @@ func TestTestExecuteExceptions(t *testing.T) {
 	}
 
 	for _, c := range cases {
+		t.Run(c.Text, func(t *testing.T) {
 			c.Check.AuditConfigOutput = c.str
 			res, err := c.Check.execute()
 			if err != nil {
 				t.Errorf(err.Error())
 			}
 			if res.testResult {
-			t.Errorf("%s, expected:%v, got:%v\n", c.Text, false, res)
+				t.Errorf("expected:%v, got:%v", false, res)
 			}
+		})
 	}
 }
 
@@ -318,17 +323,19 @@ apiVersion: kubelet.config.k8s.io/v1beta
 		},
 	}
 
-	for _, c := range cases {
+	for id, c := range cases {
+		t.Run(fmt.Sprintf("%d", id), func(t *testing.T) {
 			err := unmarshal(c.content, &c.jsonInterface)
 			if err != nil {
 				if !c.expectedToFail {
-				t.Errorf("%s, expectedToFail:%v, got:%v\n", c.content, c.expectedToFail, err)
+					t.Errorf("should pass, got error:%v", err)
 				}
 			} else {
 				if c.expectedToFail {
-				t.Errorf("%s, expectedToFail:%v, got:Did not fail\n", c.content, c.expectedToFail)
+					t.Errorf("should fail, but passed")
 				}
 			}
+		})
 	}
 }
 
@@ -339,13 +346,14 @@ func TestExecuteJSONPath(t *testing.T) {
 		Address    string
 	}
 	cases := []struct {
+		name           string
 		jsonPath       string
 		jsonInterface  kubeletConfig
 		expectedResult string
 		expectedToFail bool
 	}{
 		{
-			// JSONPath parse works, results don't match
+			"JSONPath parse works, results don't match",
 			"{.Kind}",
 			kubeletConfig{
 				Kind:       "KubeletConfiguration",
@@ -356,7 +364,7 @@ func TestExecuteJSONPath(t *testing.T) {
 			true,
 		},
 		{
-			// JSONPath parse works, results match
+			"JSONPath parse works, results match",
 			"{.Kind}",
 			kubeletConfig{
 				Kind:       "KubeletConfiguration",
@@ -367,7 +375,7 @@ func TestExecuteJSONPath(t *testing.T) {
 			false,
 		},
 		{
-			// JSONPath parse fails
+			"JSONPath parse fails",
 			"{.ApiVersion",
 			kubeletConfig{
 				Kind:       "KubeletConfiguration",
@@ -379,13 +387,15 @@ func TestExecuteJSONPath(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
 			result, err := executeJSONPath(c.jsonPath, c.jsonInterface)
 			if err != nil && !c.expectedToFail {
-			t.Fatalf("jsonPath:%q, expectedResult:%q got:%v\n", c.jsonPath, c.expectedResult, err)
+				t.Fatalf("jsonPath:%q, expectedResult:%q got:%v", c.jsonPath, c.expectedResult, err)
 			}
 			if c.expectedResult != result && !c.expectedToFail {
-			t.Errorf("jsonPath:%q, expectedResult:%q got:%q\n", c.jsonPath, c.expectedResult, result)
+				t.Errorf("jsonPath:%q, expectedResult:%q got:%q", c.jsonPath, c.expectedResult, result)
 			}
+		})
 	}
 }
 
@@ -438,10 +448,12 @@ func TestAllElementsValid(t *testing.T) {
 			valid: false,
 		},
 	}
-	for _, c := range cases {
+	for id, c := range cases {
+		t.Run(fmt.Sprintf("%d", id), func(t *testing.T) {
 			if !allElementsValid(c.source, c.target) && c.valid {
-			t.Errorf("Not All Elements in %q are found in %q \n", c.source, c.target)
+				t.Errorf("Not All Elements in %q are found in %q", c.source, c.target)
 			}
+		})
 	}
 }
 
@@ -478,16 +490,17 @@ func TestSplitAndRemoveLastSeparator(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for id, c := range cases {
+		t.Run(fmt.Sprintf("%d", id), func(t *testing.T) {
 			as := splitAndRemoveLastSeparator(c.source, defaultArraySeparator)
 			if len(as) == 0 && c.valid {
-			t.Errorf("Split did not work with %q \n", c.source)
+				t.Errorf("Split did not work with %q", c.source)
 			}
 
 			if c.elementCnt != len(as) {
-			t.Errorf("Split did not work with %q expected: %d got: %d\n", c.source, c.elementCnt, len(as))
+				t.Errorf("Split did not work with %q expected: %d got: %d", c.source, c.elementCnt, len(as))
 			}
-
+		})
 	}
 }
 
@@ -743,15 +756,16 @@ func TestCompareOp(t *testing.T) {
 	}
 
 	for _, c := range cases {
+		t.Run(c.label, func(t *testing.T) {
 			expectedResultPattern, testResult := compareOp(c.op, c.flagVal, c.compareValue)
-
 			if expectedResultPattern != c.expectedResultPattern {
-			t.Errorf("'expectedResultPattern' did not match - label: %q op: %q expected 'expectedResultPattern':%q  got:%q\n", c.label, c.op, c.expectedResultPattern, expectedResultPattern)
+				t.Errorf("'expectedResultPattern' did not match - op: %q expected:%q  got:%q", c.op, c.expectedResultPattern, expectedResultPattern)
 			}
 
 			if testResult != c.testResult {
-			t.Errorf("'testResult' did not match - label: %q op: %q expected 'testResult':%t  got:%t\n", c.label, c.op, c.testResult, testResult)
+				t.Errorf("'testResult' did not match - lop: %q expected:%t  got:%t", c.op, c.testResult, testResult)
 			}
+		})
 	}
 }
 
@@ -778,14 +792,16 @@ func TestToNumeric(t *testing.T) {
 		},
 	}
 
-	for _, c := range cases {
+	for id, c := range cases {
+		t.Run(fmt.Sprintf("%d", id), func(t *testing.T) {
 			f, s, err := toNumeric(c.firstValue, c.secondValue)
 			if c.expectedToFail && err == nil {
-			t.Errorf("TestToNumeric - Expected error while converting %s and %s", c.firstValue, c.secondValue)
+				t.Errorf("Expected error while converting %s and %s", c.firstValue, c.secondValue)
 			}
 
 			if !c.expectedToFail && (f != 5 || s != 6) {
-			t.Errorf("TestToNumeric - Expected to return %d,%d , but instead got %d,%d", 5, 6, f, s)
+				t.Errorf("Expected to return %d,%d - got %d,%d", 5, 6, f, s)
 			}
+		})
 	}
 }

makefile

@@ -33,7 +33,7 @@ build-docker:
              -t $(IMAGE_NAME) .
 
 tests:
-	GO111MODULE=on go test -v -short -race -timeout 30s -coverprofile=coverage.txt -covermode=atomic ./...
+	GO111MODULE=on go test -short -race -timeout 30s -coverprofile=coverage.txt -covermode=atomic ./...
 
 integration-tests: build-docker
 	GO111MODULE=on go test ./integration/... -v -tags integration -timeout 1200s -args -kubebenchImg=$(IMAGE_NAME)