Refine test assertion output (#423)

- allow colorized diffs, use `COLOR_DIFF=true` to force on
- clarify what is unexpected or missing
- reference specific expected field and index when appropriate
- hide "for config X" for the default config, continue to show for extra
  configs

Signed-off-by: Scott Andrews <[email protected]>

README.md

@@ -684,6 +684,8 @@ The tests make extensive use of given and mutated resources. It is recommended t
 
 There are three test suites: for [testing reconcilers](#reconcilertests), an optimized harness for [testing sub reconcilers](#subreconcilertests), and for [testing admission webhooks](#admissionwebhooktests).
 
+Colorized diffs are available in assertion error messages by setting the environment variable `COLOR_DIFF=true`
+
 <a name="reconcilertestsuite" />
 
 ### ReconcilerTests

go.mod

@@ -5,6 +5,7 @@ go 1.20
 require (
 	dies.dev v0.9.0
 	github.com/evanphx/json-patch/v5 v5.6.0
+	github.com/fatih/color v1.15.0
 	github.com/go-logr/logr v1.2.4
 	github.com/google/go-cmp v0.5.9
 	golang.org/x/net v0.14.0
@@ -38,6 +39,8 @@ require (
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
+	github.com/mattn/go-colorable v0.1.13 // indirect
+	github.com/mattn/go-isatty v0.0.17 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect

go.sum

@@ -22,6 +22,8 @@ github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCv
 github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
 github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
+github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs=
+github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
 github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
@@ -79,6 +81,11 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
+github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
+github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
+github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
+github.com/mattn/go-isatty v0.0.17 h1:BTarxUcIeDqL27Mc+vyvdWYSL28zpIhv3RoTdsLMPng=
+github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
 github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo=
 github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
@@ -160,6 +167,7 @@ golang.org/x/sync v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
 golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

testing/color.go

@@ -0,0 +1,41 @@
+/*
+Copyright 2023 VMware, Inc.
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package testing
+
+import (
+	"os"
+	"strings"
+
+	"github.com/fatih/color"
+)
+
+func init() {
+	if _, ok := os.LookupEnv("COLOR_DIFF"); ok {
+		DiffAddedColor.EnableColor()
+		DiffRemovedColor.EnableColor()
+	}
+}
+
+var (
+	DiffAddedColor   = color.New(color.FgGreen)
+	DiffRemovedColor = color.New(color.FgRed)
+)
+
+func ColorizeDiff(diff string) string {
+	var b strings.Builder
+	for _, line := range strings.Split(diff, "\n") {
+		switch {
+		case strings.HasPrefix(line, "+"):
+			b.WriteString(DiffAddedColor.Sprint(line))
+		case strings.HasPrefix(line, "-"):
+			b.WriteString(DiffRemovedColor.Sprint(line))
+		default:
+			b.WriteString(line)
+		}
+		b.WriteString("\n")
+	}
+	return b.String()
+}

testing/config.go

@@ -117,6 +117,13 @@ func (c *ExpectConfig) init() {
 	})
 }
 
+func (c *ExpectConfig) configNameMsg() string {
+	if c.Name == "" || c.Name == "default" {
+		return ""
+	}
+	return fmt.Sprintf(" for config %q", c.Name)
+}
+
 func (c *ExpectConfig) createClient(objs []client.Object, statusSubResourceTypes []client.Object) *clientWrapper {
 	builder := fake.NewClientBuilder()
 
@@ -184,7 +191,7 @@ func (c *ExpectConfig) AssertClientCreateExpectations(t *testing.T) {
 	}
 	c.init()
 
-	c.compareActions(t, "create", c.ExpectCreates, c.client.CreateActions, IgnoreLastTransitionTime, SafeDeployDiff, IgnoreTypeMeta, IgnoreCreationTimestamp, IgnoreResourceVersion, cmpopts.EquateEmpty())
+	c.compareActions(t, "Create", c.ExpectCreates, c.client.CreateActions, IgnoreLastTransitionTime, SafeDeployDiff, IgnoreTypeMeta, IgnoreCreationTimestamp, IgnoreResourceVersion, cmpopts.EquateEmpty())
 }
 
 // AssertClientUpdateExpectations asserts observed reconciler client update behavior matches the expected client update behavior
@@ -194,7 +201,7 @@ func (c *ExpectConfig) AssertClientUpdateExpectations(t *testing.T) {
 	}
 	c.init()
 
-	c.compareActions(t, "update", c.ExpectUpdates, c.client.UpdateActions, IgnoreLastTransitionTime, SafeDeployDiff, IgnoreTypeMeta, IgnoreCreationTimestamp, IgnoreResourceVersion, cmpopts.EquateEmpty())
+	c.compareActions(t, "Update", c.ExpectUpdates, c.client.UpdateActions, IgnoreLastTransitionTime, SafeDeployDiff, IgnoreTypeMeta, IgnoreCreationTimestamp, IgnoreResourceVersion, cmpopts.EquateEmpty())
 }
 
 // AssertClientPatchExpectations asserts observed reconciler client patch behavior matches the expected client patch behavior
@@ -206,18 +213,18 @@ func (c *ExpectConfig) AssertClientPatchExpectations(t *testing.T) {
 
 	for i, exp := range c.ExpectPatches {
 		if i >= len(c.client.PatchActions) {
-			c.errorf(t, "Missing patch for config %q: %#v", c.Name, exp)
+			c.errorf(t, "ExpectPatches[%d] not observed%s: %#v", i, c.configNameMsg(), exp)
 			continue
 		}
 		actual := NewPatchRef(c.client.PatchActions[i])
 
 		if diff := cmp.Diff(exp, actual); diff != "" {
-			c.errorf(t, "Unexpected patch for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectPatches[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, expected := len(c.client.PatchActions), len(c.ExpectPatches); actual > expected {
 		for _, extra := range c.client.PatchActions[expected:] {
-			c.errorf(t, "Extra patch for config %q: %#v", c.Name, extra)
+			c.errorf(t, "Unexpected Patch observed%s: %#v", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -231,18 +238,18 @@ func (c *ExpectConfig) AssertClientDeleteExpectations(t *testing.T) {
 
 	for i, exp := range c.ExpectDeletes {
 		if i >= len(c.client.DeleteActions) {
-			c.errorf(t, "Missing delete for config %q: %#v", c.Name, exp)
+			c.errorf(t, "ExpectDeletes[%d] not observed%s: %#v", i, c.configNameMsg(), exp)
 			continue
 		}
 		actual := NewDeleteRef(c.client.DeleteActions[i])
 
 		if diff := cmp.Diff(exp, actual); diff != "" {
-			c.errorf(t, "Unexpected delete for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectDeletes[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, expected := len(c.client.DeleteActions), len(c.ExpectDeletes); actual > expected {
 		for _, extra := range c.client.DeleteActions[expected:] {
-			c.errorf(t, "Extra delete for config %q: %#v", c.Name, extra)
+			c.errorf(t, "Unexpected Delete observed%s: %#v", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -256,18 +263,18 @@ func (c *ExpectConfig) AssertClientDeleteCollectionExpectations(t *testing.T) {
 
 	for i, exp := range c.ExpectDeleteCollections {
 		if i >= len(c.client.DeleteCollectionActions) {
-			c.errorf(t, "Missing delete collection for config %q: %#v", c.Name, exp)
+			c.errorf(t, "ExpectDeleteCollections[%d] not observed%s: %#v", i, c.configNameMsg(), exp)
 			continue
 		}
 		actual := NewDeleteCollectionRef(c.client.DeleteCollectionActions[i])
 
 		if diff := cmp.Diff(exp, actual, NormalizeLabelSelector, NormalizeFieldSelector); diff != "" {
-			c.errorf(t, "Unexpected delete collection for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectDeleteCollections[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, expected := len(c.client.DeleteCollectionActions), len(c.ExpectDeleteCollections); actual > expected {
 		for _, extra := range c.client.DeleteCollectionActions[expected:] {
-			c.errorf(t, "Extra delete collection for config %q: %#v", c.Name, extra)
+			c.errorf(t, "Unexpected DeleteCollection observed%s: %#v", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -279,7 +286,7 @@ func (c *ExpectConfig) AssertClientStatusUpdateExpectations(t *testing.T) {
 	}
 	c.init()
 
-	c.compareActions(t, "status update", c.ExpectStatusUpdates, c.client.StatusUpdateActions, statusSubresourceOnly, IgnoreLastTransitionTime, SafeDeployDiff, cmpopts.EquateEmpty())
+	c.compareActions(t, "StatusUpdate", c.ExpectStatusUpdates, c.client.StatusUpdateActions, statusSubresourceOnly, IgnoreLastTransitionTime, SafeDeployDiff, cmpopts.EquateEmpty())
 }
 
 // AssertClientStatusPatchExpectations asserts observed reconciler client status patch behavior matches the expected client status patch behavior
@@ -291,18 +298,18 @@ func (c *ExpectConfig) AssertClientStatusPatchExpectations(t *testing.T) {
 
 	for i, exp := range c.ExpectStatusPatches {
 		if i >= len(c.client.StatusPatchActions) {
-			c.errorf(t, "Missing status patch for config %q: %#v", c.Name, exp)
+			c.errorf(t, "ExpectStatusPatches[%d] not observed%s: %#v", i, c.configNameMsg(), exp)
 			continue
 		}
 		actual := NewPatchRef(c.client.StatusPatchActions[i])
 
 		if diff := cmp.Diff(exp, actual); diff != "" {
-			c.errorf(t, "Unexpected status patch for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectStatusPatches[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, expected := len(c.client.StatusPatchActions), len(c.ExpectStatusPatches); actual > expected {
 		for _, extra := range c.client.StatusPatchActions[expected:] {
-			c.errorf(t, "Extra status patch for config %q: %#v", c.Name, extra)
+			c.errorf(t, "Unexpected StatusPatch observed%s: %#v", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -317,17 +324,17 @@ func (c *ExpectConfig) AssertRecorderExpectations(t *testing.T) {
 	actualEvents := c.recorder.events
 	for i, exp := range c.ExpectEvents {
 		if i >= len(actualEvents) {
-			c.errorf(t, "Missing recorded event for config %q: %s", c.Name, exp)
+			c.errorf(t, "ExpectEvents[%d] not observed%s: %s", i, c.configNameMsg(), exp)
 			continue
 		}
 
 		if diff := cmp.Diff(exp, actualEvents[i]); diff != "" {
-			c.errorf(t, "Unexpected recorded event for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectEvents[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, exp := len(actualEvents), len(c.ExpectEvents); actual > exp {
 		for _, extra := range actualEvents[exp:] {
-			c.errorf(t, "Extra recorded event for config %q: %s", c.Name, extra)
+			c.errorf(t, "Unexpected Event observed%s: %s", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -344,17 +351,17 @@ func (c *ExpectConfig) AssertTrackerExpectations(t *testing.T) {
 		exp.normalize()
 
 		if i >= len(actualTracks) {
-			c.errorf(t, "Missing tracking request for config %q: %v", c.Name, exp)
+			c.errorf(t, "ExpectTracks[%d] not observed%s: %v", i, c.configNameMsg(), exp)
 			continue
 		}
 
 		if diff := cmp.Diff(exp, actualTracks[i], NormalizeLabelSelector); diff != "" {
-			c.errorf(t, "Unexpected tracking request for config %q (-expected, +actual): %s", c.Name, diff)
+			c.errorf(t, "ExpectTracks[%d] differs%s (%s, %s):\n%s", i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, exp := len(actualTracks), len(c.ExpectTracks); actual > exp {
 		for _, extra := range actualTracks[exp:] {
-			c.errorf(t, "Extra tracking request for config %q: %v", c.Name, extra)
+			c.errorf(t, "Unexpected Track observed%s: %v", c.configNameMsg(), extra)
 		}
 	}
 }
@@ -367,18 +374,18 @@ func (c *ExpectConfig) compareActions(t *testing.T, actionName string, expectedA
 
 	for i, exp := range expectedActionFactories {
 		if i >= len(actualActions) {
-			c.errorf(t, "Missing %s for config %q: %#v", actionName, c.Name, exp.DeepCopyObject())
+			c.errorf(t, "Expect%ss[%d] not observed%s: %#v", actionName, i, c.configNameMsg(), exp.DeepCopyObject())
 			continue
 		}
 		actual := actualActions[i].GetObject()
 
 		if diff := cmp.Diff(exp.DeepCopyObject(), actual, diffOptions...); diff != "" {
-			c.errorf(t, "Unexpected %s for config %q (-expected, +actual): %s", actionName, c.Name, diff)
+			c.errorf(t, "Expect%ss[%d] differs%s (%s, %s):\n%s", actionName, i, c.configNameMsg(), DiffRemovedColor.Sprint("-expected"), DiffAddedColor.Sprint("+actual"), ColorizeDiff(diff))
 		}
 	}
 	if actual, expected := len(actualActions), len(expectedActionFactories); actual > expected {
 		for _, extra := range actualActions[expected:] {
-			c.errorf(t, "Extra %s for config %q: %#v", actionName, c.Name, extra)
+			c.errorf(t, "Unexpected %s observed%s: %#v", actionName, c.configNameMsg(), extra)
 		}
 	}
 }