Improve error message for test assertions (kudobuilder#380)

* Bump controller-runtime to pick up a fake client fix. We need [this commit](kubernetes-sigs/controller-runtime@3a037ef) for the subsequent changes which invoke List() on the fake client. * Improve error message for error assertions and add test. In the test cases for multiple objects we're using NewV1Pod, which returns an actual Pod struct, rather than NewPod (which returns an Unstructured). This is because the fake client is unable to List() pods which were supplied to builder as Unstructureds. Instead it errors out with: item[0]: can't assign or convert unstructured.Unstructured into v1.Pod Co-authored-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com> Signed-off-by: Israel Blancas <iblancasa@gmail.com>
iblancasa · Nov 17, 2022 · 16eff8e · 16eff8e
1 parent d98ae7b
commit 16eff8e
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 15 deletions.
diff --git a/go.sum b/go.sum
@@ -76,7 +76,6 @@ github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMo
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/PuerkitoBio/purell v1.1.1 h1:WEQqlqaGbrPkxLJWfBwQmfEAE1Z7ONdDLqrN38tNFfI=
 github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
-github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M=
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=

diff --git a/pkg/test/step.go b/pkg/test/step.go
@@ -355,13 +355,20 @@ func (s *Step) CheckResourceAbsent(expected runtime.Object, namespace string) er
 		return err
 	}
 
+	var unexpectedObjects []unstructured.Unstructured
 	for _, actual := range actuals {
 		if err := testutils.IsSubset(expectedObj, actual.UnstructuredContent()); err == nil {
-			return fmt.Errorf("resource matched of kind: %s", gvk.String())
+			unexpectedObjects = append(unexpectedObjects, actual)
 		}
 	}
 
-	return nil
+	if len(unexpectedObjects) == 0 {
+		return nil
+	}
+	if len(unexpectedObjects) == 1 {
+		return fmt.Errorf("resource %s %s matched error assertion", unexpectedObjects[0].GroupVersionKind(), unexpectedObjects[0].GetName())
+	}
+	return fmt.Errorf("resource %s %s (and %d other resources) matched error assertion", unexpectedObjects[0].GroupVersionKind(), unexpectedObjects[0].GetName(), len(unexpectedObjects)-1)
 }
 
 // CheckAssertCommands Runs the commands provided in `commands` and check if have been run successfully.

diff --git a/pkg/test/step_test.go b/pkg/test/step_test.go
@@ -202,24 +202,46 @@ func TestCheckResource(t *testing.T) {
 func TestCheckResourceAbsent(t *testing.T) {
 	for _, test := range []struct {
 		name        string
-		actual      runtime.Object
+		actual      []runtime.Object
 		expected    runtime.Object
 		shouldError bool
+		expectedErr string
 	}{
 		{
 			name:        "resource matches",
-			actual:      testutils.NewPod("hello", ""),
+			actual:      []runtime.Object{testutils.NewPod("hello", "")},
 			expected:    testutils.NewPod("hello", ""),
 			shouldError: true,
 		},
+		{
+			name: "one of more resources matches",
+			actual: []runtime.Object{
+				testutils.NewV1Pod("pod1", "", "val1"),
+				testutils.NewV1Pod("pod2", "", "val2"),
+			},
+			expected:    testutils.WithSpec(t, testutils.NewPod("", ""), map[string]interface{}{"serviceAccountName": "val1"}),
+			shouldError: true,
+			expectedErr: "resource /v1, Kind=Pod pod1 matched error assertion",
+		},
+		{
+			name: "multiple of more resources matches",
+			actual: []runtime.Object{
+				testutils.NewV1Pod("pod1", "", "val1"),
+				testutils.NewV1Pod("pod2", "", "val1"),
+				testutils.NewV1Pod("pod3", "", "val2"),
+			},
+			expected:    testutils.WithSpec(t, testutils.NewPod("", ""), map[string]interface{}{"serviceAccountName": "val1"}),
+			shouldError: true,
+			expectedErr: "resource /v1, Kind=Pod pod1 (and 1 other resources) matched error assertion",
+		},
 		{
 			name:     "resource mis-match",
-			actual:   testutils.NewPod("hello", ""),
+			actual:   []runtime.Object{testutils.NewPod("hello", "")},
 			expected: testutils.WithSpec(t, testutils.NewPod("hello", ""), map[string]interface{}{"invalid": "key"}),
 		},
 		{
 			name:     "resource does not exist",
-			actual:   testutils.NewPod("other", ""),
+			actual:   []runtime.Object{testutils.NewPod("other", "")},
 			expected: testutils.NewPod("hello", ""),
 		},
 	} {
@@ -228,23 +250,28 @@ func TestCheckResourceAbsent(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			fakeDiscovery := testutils.FakeDiscoveryClient()
 
-			_, _, err := testutils.Namespaced(fakeDiscovery, test.actual, testNamespace)
-			assert.Nil(t, err)
+			for _, object := range test.actual {
+				_, _, err := testutils.Namespaced(fakeDiscovery, object, testNamespace)
+				assert.NoError(t, err)
+			}
 
 			step := Step{
 				Logger: testutils.NewTestLogger(t, ""),
 				Client: func(bool) (client.Client, error) {
-					return fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(test.actual).Build(), nil
+					return fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(test.actual...).Build(), nil
 				},
 				DiscoveryClient: func() (discovery.DiscoveryInterface, error) { return fakeDiscovery, nil },
 			}
 
 			error := step.CheckResourceAbsent(test.expected, testNamespace)
 
 			if test.shouldError {
-				assert.NotNil(t, error)
+				assert.Error(t, error)
+				if test.expectedErr != "" {
+					assert.EqualError(t, error, test.expectedErr)
+				}
 			} else {
-				assert.Nil(t, error)
+				assert.NoError(t, error)
 			}
 		})
 	}

diff --git a/pkg/test/utils/kubernetes.go b/pkg/test/utils/kubernetes.go
@@ -637,11 +637,13 @@ func ObjectKey(obj runtime.Object) client.ObjectKey {
 }
 
 // NewResource generates a Kubernetes object using the provided apiVersion, kind, name, and namespace.
+// The name and namespace are omitted if empty.
 func NewResource(apiVersion, kind, name, namespace string) *unstructured.Unstructured {
-	meta := map[string]interface{}{
-		"name": name,
-	}
+	meta := map[string]interface{}{}
 
+	if name != "" {
+		meta["name"] = name
+	}
 	if namespace != "" {
 		meta["namespace"] = namespace
 	}
@@ -690,6 +692,27 @@ func NewPod(name, namespace string) *unstructured.Unstructured {
 	return NewResource("v1", "Pod", name, namespace)
 }
 
+// NewV1Pod returns a new corev1.Pod object.
+// Each of name, namespace and serviceAccountName are set if non-empty.
+func NewV1Pod(name, namespace, serviceAccountName string) *corev1.Pod {
+	pod := corev1.Pod{}
+	pod.SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "",
+		Version: "v1",
+		Kind:    "Pod",
+	})
+	if name != "" {
+		pod.SetName(name)
+	}
+	if namespace != "" {
+		pod.SetNamespace(namespace)
+	}
+	if serviceAccountName != "" {
+		pod.Spec.ServiceAccountName = serviceAccountName
+	}
+	return &pod
+}
+
 // WithNamespace naively applies the namespace to the object. Used mainly in tests, otherwise
 // use Namespaced.
 func WithNamespace(obj *unstructured.Unstructured, namespace string) *unstructured.Unstructured {