Merge pull request #109961 from pohly/e2e-framework-log-test-fix

e2e: fix unit test

hack/make-rules/test.sh

@@ -44,7 +44,7 @@ kube::test::find_dirs() {
           -o -path './output/*' \
           -o -path './release/*' \
           -o -path './target/*' \
-          -o -path './test/e2e/*' \
+          -o -path './test/e2e/e2e_test.go' \
           -o -path './test/e2e_node/*' \
           -o -path './test/e2e_kubeadm/*' \
           -o -path './test/integration/*' \

test/e2e/framework/log_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/onsi/ginkgo/config"
 	"github.com/onsi/ginkgo/reporters"
 	"github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
 
 	"k8s.io/kubernetes/test/e2e/framework"
 )
@@ -40,7 +41,6 @@ import (
 //
 //
 //
-//
 
 func runTests(t *testing.T, reporter ginkgo.Reporter) {
 	// This source code line will be part of the stack dump comparison.
@@ -86,46 +86,130 @@ func TestFailureOutput(t *testing.T) {
 	actual := normalizeReport(*reporter)
 
 	// output from AfterEach
-	commonOutput := "\n\nINFO: after\nFAIL: true is never false either\nExpected\n    <bool>: true\nto equal\n    <bool>: false\n\nFull Stack Trace\nk8s.io/kubernetes/test/e2e/framework_test.glob..func1.6()\n\tlog_test.go:71\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47\n\n"
+	commonOutput := `
+
+INFO: after
+FAIL: true is never false either
+Expected
+    <bool>: true
+to equal
+    <bool>: false
+
+Full Stack Trace
+k8s.io/kubernetes/test/e2e/framework_test.glob..func1.6()
+	log_test.go:71
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47
+
+`
 
 	// Sorted by name!
 	expected := suiteResults{
 		testResult{
-			name:    "[Top Level] log asserts",
+			name: "[Top Level] log asserts",
-			output:  "INFO: before\nFAIL: false is never true\nExpected\n    <bool>: false\nto equal\n    <bool>: true\n\nFull Stack Trace\nk8s.io/kubernetes/test/e2e/framework_test.glob..func1.3()\n\tlog_test.go:60\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47" + commonOutput,
+			output: `INFO: before
-			failure: "false is never true\nExpected\n    <bool>: false\nto equal\n    <bool>: true",
+FAIL: false is never true
-			stack:   "k8s.io/kubernetes/test/e2e/framework_test.glob..func1.3()\n\tlog_test.go:60\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47\n",
+Expected
+    <bool>: false
+to equal
+    <bool>: true
+
+Full Stack Trace
+k8s.io/kubernetes/test/e2e/framework_test.glob..func1.3()
+	log_test.go:60
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47` + commonOutput,
+			failure: `false is never true
+Expected
+    <bool>: false
+to equal
+    <bool>: true`,
+			stack: `k8s.io/kubernetes/test/e2e/framework_test.glob..func1.3()
+	log_test.go:60
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47
+`,
 		},
 		testResult{
-			name:    "[Top Level] log equal",
+			name: "[Top Level] log equal",
-			output:  "INFO: before\nFAIL: of course it's not equal...\nExpected\n    <int>: 0\nto equal\n    <int>: 1\n\nFull Stack Trace\nk8s.io/kubernetes/test/e2e/framework_test.glob..func1.5()\n\tlog_test.go:67\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47" + commonOutput,
+			output: `INFO: before
-			failure: "of course it's not equal...\nExpected\n    <int>: 0\nto equal\n    <int>: 1",
+FAIL: of course it's not equal...
-			stack:   "k8s.io/kubernetes/test/e2e/framework_test.glob..func1.5()\n\tlog_test.go:67\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47\n",
+Expected
+    <int>: 0
+to equal
+    <int>: 1
+
+Full Stack Trace
+k8s.io/kubernetes/test/e2e/framework_test.glob..func1.5()
+	log_test.go:67
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47` + commonOutput,
+			failure: `of course it's not equal...
+Expected
+    <int>: 0
+to equal
+    <int>: 1`,
+			stack: `k8s.io/kubernetes/test/e2e/framework_test.glob..func1.5()
+	log_test.go:67
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47
+`,
 		},
 		testResult{
-			name:    "[Top Level] log error",
+			name: "[Top Level] log error",
-			output:  "INFO: before\nFAIL: hard-coded error\nUnexpected error:\n    <*errors.errorString>: {\n        s: \"an error with a long, useless description\",\n    }\n    an error with a long, useless description\noccurred\n\nFull Stack Trace\nk8s.io/kubernetes/test/e2e/framework_test.glob..func1.4()\n\tlog_test.go:64\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47" + commonOutput,
+			output: `INFO: before
-			failure: "hard-coded error\nUnexpected error:\n    <*errors.errorString>: {\n        s: \"an error with a long, useless description\",\n    }\n    an error with a long, useless description\noccurred",
+FAIL: hard-coded error
-			stack:   "k8s.io/kubernetes/test/e2e/framework_test.glob..func1.4()\n\tlog_test.go:64\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47\n",
+Unexpected error:
+    <*errors.errorString>: {
+        s: "an error with a long, useless description",
+    }
+    an error with a long, useless description
+occurred
+
+Full Stack Trace
+k8s.io/kubernetes/test/e2e/framework_test.glob..func1.4()
+	log_test.go:64
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47` + commonOutput,
+			failure: `hard-coded error
+Unexpected error:
+    <*errors.errorString>: {
+        s: "an error with a long, useless description",
+    }
+    an error with a long, useless description
+occurred`,
+			stack: `k8s.io/kubernetes/test/e2e/framework_test.glob..func1.4()
+	log_test.go:64
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47
+`,
 		},
 		testResult{
-			name:    "[Top Level] log fails",
+			name: "[Top Level] log fails",
-			output:  "INFO: before\nFAIL: I'm failing.\n\nFull Stack Trace\nk8s.io/kubernetes/test/e2e/framework_test.glob..func1.2()\n\tlog_test.go:57\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47" + commonOutput,
+			output: `INFO: before
+FAIL: I'm failing.
+
+Full Stack Trace
+k8s.io/kubernetes/test/e2e/framework_test.glob..func1.2()
+	log_test.go:57
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47` + commonOutput,
 			failure: "I'm failing.",
-			stack:   "k8s.io/kubernetes/test/e2e/framework_test.glob..func1.2()\n\tlog_test.go:57\nk8s.io/kubernetes/test/e2e/framework_test.runTests()\n\tlog_test.go:47\n",
+			stack: `k8s.io/kubernetes/test/e2e/framework_test.glob..func1.2()
+	log_test.go:57
+k8s.io/kubernetes/test/e2e/framework_test.runTests()
+	log_test.go:47
+`,
 		},
 	}
-	// Compare individual fields. Comparing the slices leads to unreadable error output when there is any mismatch.
+	// assert.Equal prints a useful diff if the slices are not
-	framework.ExpectEqual(len(actual), len(expected), "%d entries in %v", len(expected), actual)
+	// equal. However, the diff does not show changes inside the
-	for i, a := range actual {
+	// strings. Therefore we also compare the individual fields.
-		b := expected[i]
+	if !assert.Equal(t, expected, actual) {
-		framework.ExpectEqual(a.name, b.name, "name in %d", i)
+		for i := 0; i < len(expected) && i < len(actual); i++ {
-		framework.ExpectEqual(a.output, b.output, "output in %d", i)
+			assert.Equal(t, expected[i].output, actual[i].output, "output from test #%d: %s", i, expected[i].name)
-		framework.ExpectEqual(a.failure, b.failure, "failure in %d", i)
+			assert.Equal(t, expected[i].stack, actual[i].stack, "stack from test #%d: %s", i, expected[i].name)
-		// There may be additional stack entries from the "testing" package at the
+		}
-		// end. We ignore those in the comparison because the line number in them
-		// varies.
-		framework.ExpectEqual(a.stack, b.stack, "stack in %d: %s", i, a.stack)
 	}
 }
 

test/e2e/framework/node/wait_test.go

@@ -168,15 +168,26 @@ func TestCheckReadyForTests(t *testing.T) {
 				return true, nodeList, tc.nodeListErr
 			})
 			checkFunc := CheckReadyForTests(c, tc.nonblockingTaints, tc.allowedNotReadyNodes, testLargeClusterThreshold)
-			out, err := checkFunc()
+			// The check function returns "false, nil" during its
-			if out != tc.expected {
+			// first two calls, therefore we have to try several
-				t.Errorf("Expected %v but got %v", tc.expected, out)
+			// times until we get the expected error.
-			}
+			for attempt := 0; attempt <= 3; attempt++ {
-			switch {
+				out, err := checkFunc()
-			case err == nil && len(tc.expectedErr) > 0:
+				expected := tc.expected
-				t.Errorf("Expected error %q nil", tc.expectedErr)
+				expectedErr := tc.expectedErr
-			case err != nil && err.Error() != tc.expectedErr:
+				if tc.nodeListErr != nil && attempt < 2 {
-				t.Errorf("Expected error %q but got %q", tc.expectedErr, err.Error())
+					expected = false
+					expectedErr = ""
+				}
+				if out != expected {
+					t.Errorf("Expected %v but got %v", expected, out)
+				}
+				switch {
+				case err == nil && expectedErr != "":
+					t.Errorf("attempt #%d: expected error %q nil", attempt, expectedErr)
+				case err != nil && err.Error() != expectedErr:
+					t.Errorf("attempt #%d: expected error %q but got %q", attempt, expectedErr, err.Error())
+				}
 			}
 		})
 	}