Skip to content

Commit

Permalink
Addressed PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
killianmuldoon committed Sep 17, 2021
1 parent d24e1f4 commit 7d049e7
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 31 deletions.
1 change: 0 additions & 1 deletion controllers/topology/blueprint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ func TestGetBlueprint(t *testing.T) {
g.Expect(tt.want.InfrastructureClusterTemplate).To(EqualObject(got.InfrastructureClusterTemplate))
g.Expect(got.ControlPlane).To(Equal(tt.want.ControlPlane))
g.Expect(tt.want.MachineDeployments).To(Equal(got.MachineDeployments))

})
}
}
1 change: 0 additions & 1 deletion controllers/topology/current_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,6 @@ func TestGetCurrentState(t *testing.T) {
g.Expect(got.InfrastructureCluster).To(EqualObject(tt.want.InfrastructureCluster))
g.Expect(got.ControlPlane).To(Equal(tt.want.ControlPlane))
g.Expect(got.MachineDeployments).To(Equal(tt.want.MachineDeployments))

})
}
}
57 changes: 28 additions & 29 deletions internal/testtypes/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -45,7 +44,6 @@ var (
{"metadata", "managedFields"},
{"metadata", "deletionGracePeriodSeconds"},
{"metadata", "deletionTimestamp"},
{"metadata", "ownerReferences"},
{"metadata", "selfLink"},
{"metadata", "generateName"},
}
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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:])
}
Expand Down

0 comments on commit 7d049e7

Please sign in to comment.