Addressed PR comments

Author: killianmuldoon

controllers/topology/blueprint_test.go

@@ -262,7 +262,7 @@ func TestGetBlueprint(t *testing.T) {
 				g.Expect(got).To(BeNil())
 				return
 			}
-			// Use EqualObject where an object is created and passed through the fake client. Elsewhere the Equal methoc
+			// Use EqualObject where an object is created and passed through the fake client. Elsewhere the Equal method
 			// is enough to establish inequality.
 			g.Expect(tt.want.ClusterClass).To(EqualObject(got.ClusterClass, IgnoreAutogeneratedMetadata))
 			g.Expect(tt.want.InfrastructureClusterTemplate).To(EqualObject(got.InfrastructureClusterTemplate))

internal/testtypes/matchers.go

@@ -34,7 +34,6 @@ import (
 // These package variables hold pre-created commonly used options that can be used to reduce the manual work involved in
 // identifying the paths that need to be compared for testing equality between objects.
 var (
-
 	// IgnoreAutogeneratedMetadata contains the paths for all the metadata fields that are commonly set by the
 	// client and APIServer. This is used as a MatchOption for situations when only user-provided metadata is relevant.
 	IgnoreAutogeneratedMetadata = IgnorePaths{
@@ -45,7 +44,6 @@ var (
 		{"metadata", "managedFields"},
 		{"metadata", "deletionGracePeriodSeconds"},
 		{"metadata", "deletionTimestamp"},
-		{"metadata", "ownerReferences"},
 		{"metadata", "selfLink"},
 		{"metadata", "generateName"},
 	}
@@ -63,8 +61,8 @@ type Matcher struct {
 	options *MatchOptions
 }
 
-// EqualObject applies the MatchOptions and returns a Matcher for the passed Kubernetes runtime.Object. This function
-// can be used directly as a Gomega Matcher in Gomega Assertions.
+// EqualObject returns a Matcher for the passed Kubernetes runtime.Object with the passed Options. This function can be
+// used as a Gomega Matcher in Gomega Assertions.
 func EqualObject(original runtime.Object, opts ...MatchOption) *Matcher {
 	matchOptions := &MatchOptions{}
 	matchOptions = matchOptions.ApplyOptions(opts)
@@ -79,56 +77,57 @@ func EqualObject(original runtime.Object, opts ...MatchOption) *Matcher {
 	}
 }
 
-// Match compares the object it's based on to the passed object and returns true if the objects are the same according to
-// the Matcher MatchOptions.
-func (m *Matcher) Match(other interface{}) (success bool, err error) {
+// Match compares the current object to the passed object and returns true if the objects are the same according to
+// the Matcher and MatchOptions.
+func (m *Matcher) Match(actual interface{}) (success bool, err error) {
 	// Nil checks required first here for:
 	//     1) Nil equality which returns true
 	//     2) One object nil which returns an error
-	otherIsNil := reflect.ValueOf(other).IsNil()
+	actualIsNil := reflect.ValueOf(actual).IsNil()
 	originalIsNil := reflect.ValueOf(m.original).IsNil()
 
-	if otherIsNil && originalIsNil {
+	if actualIsNil && originalIsNil {
 		return true, nil
 	}
-	if otherIsNil || originalIsNil {
-		return false, fmt.Errorf("can not compare an object with a nil. original :%v , other %v", m.original, other)
+	if actualIsNil || originalIsNil {
+		return false, fmt.Errorf("can not compare an object with a nil. original %v , actual %v", m.original, actual)
 	}
 
 	// Calculate diff returns a json diff between the two objects.
-	m.diff, err = m.calculateDiff(other)
+	m.diff, err = m.calculateDiff(actual)
 	if err != nil {
 		return false, err
 	}
 	return bytes.Equal(m.diff, []byte("{}")), nil
 }
 
 // FailureMessage returns a message comparing the full objects after an unexpected failure to match has occurred.
-func (m *Matcher) FailureMessage(other interface{}) (message string) {
+func (m *Matcher) FailureMessage(actual interface{}) (message string) {
 	return fmt.Sprintf("the following fields were expected to match but did not:\n%s\n%s", string(m.diff),
-		format.Message(other, "expected to match", m.original))
+		format.Message(actual, "expected to match", m.original))
 }
 
 // NegatedFailureMessage returns a string comparing the full objects after an unexpected match has occurred.
-func (m *Matcher) NegatedFailureMessage(other interface{}) (message string) {
-	return format.Message(other, "not to match", m.original)
+func (m *Matcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message("the following fields were not expected to match \n%s\n%s", string(m.diff),
+		format.Message(actual, "expected to match", m.original))
 }
 
-// calculateDiff applies the MatchOptions and identified the diff between the Matcher object and the passed object.
-func (m *Matcher) calculateDiff(other interface{}) ([]byte, error) {
-	// Convert the original and other objects to json.
+// calculateDiff applies the MatchOptions and identifies the diff between the Matcher object and the actual object.
+func (m *Matcher) calculateDiff(actual interface{}) ([]byte, error) {
+	// Convert the original and actual objects to json.
 	originalJSON, err := json.Marshal(m.original)
 	if err != nil {
 		return nil, err
 	}
 
-	modifiedJSON, err := json.Marshal(other)
+	actualJSON, err := json.Marshal(actual)
 	if err != nil {
 		return nil, err
 	}
 
 	// Use a mergePatch to produce a diff between the two objects.
-	originalWithModifiedJSON, err := jsonpatch.MergePatch(originalJSON, modifiedJSON)
+	originalWithModifiedJSON, err := jsonpatch.MergePatch(originalJSON, actualJSON)
 	if err != nil {
 		return nil, err
 	}
@@ -182,16 +181,16 @@ func (i AllowPaths) ApplyToMatcher(opts *MatchOptions) {
 }
 
 // filterDiff limits the diff to allowPaths if given and excludes ignorePaths if given. It returns the altered diff.
-func filterDiff(diff []byte, allowedPaths, ignorePaths [][]string) ([]byte, error) {
+func filterDiff(diff []byte, allowPaths, ignorePaths [][]string) ([]byte, error) {
 	// converts the diff into a Map
 	diffMap := make(map[string]interface{})
 	err := json.Unmarshal(diff, &diffMap)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to unmarshal merge diff")
 	}
 
-	// Removes from diffs everything not in the allowed paths.
-	filterDiffMap(diffMap, allowedPaths)
+	// Removes from diffs everything not in the allowpaths.
+	filterDiffMap(diffMap, allowPaths)
 
 	// Removes from diffs everything in the ignore paths.
 	for _, path := range ignorePaths {
@@ -207,17 +206,17 @@ func filterDiff(diff []byte, allowedPaths, ignorePaths [][]string) ([]byte, erro
 }
 
 // filterDiffMap limits the diffMap to those paths allowed by the MatchOptions.
-func filterDiffMap(diffMap map[string]interface{}, allowedPaths [][]string) {
+func filterDiffMap(diffMap map[string]interface{}, allowPaths [][]string) {
 	// if the allowPaths only contains "*" return the full diffmap.
-	if len(allowedPaths) == 1 && allowedPaths[0][0] == "*" {
+	if len(allowPaths) == 1 && allowPaths[0][0] == "*" {
 		return
 	}
 
 	// Loop through the entries in the map.
 	for k, m := range diffMap {
-		// Check if item is in the allowed paths.
+		// Check if item is in the allowPaths.
 		allowed := false
-		for _, path := range allowedPaths {
+		for _, path := range allowPaths {
 			if k == path[0] {
 				allowed = true
 				break
@@ -234,7 +233,7 @@ func filterDiffMap(diffMap map[string]interface{}, allowedPaths [][]string) {
 			continue
 		}
 		nestedPaths := make([][]string, 0)
-		for _, path := range allowedPaths {
+		for _, path := range allowPaths {
 			if k == path[0] && len(path) > 1 {
 				nestedPaths = append(nestedPaths, path[1:])
 			}