diff --git a/pkg/rpdac/dashboard.go b/pkg/rpdac/dashboard.go index cd213ec..d0ea750 100644 --- a/pkg/rpdac/dashboard.go +++ b/pkg/rpdac/dashboard.go @@ -6,11 +6,12 @@ import ( "fmt" "io" "log" - "reflect" + "sort" "strings" "github.com/b1zzu/reportportal-dashboards-as-code/pkg/reportportal" - "github.com/b1zzu/reportportal-dashboards-as-code/pkg/util" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ) type IDashboardService interface { @@ -432,96 +433,29 @@ func (d *Dashboard) GetKind() ObjectKind { return d.Kind } -func (left *Dashboard) Equals(o util.Comparable) bool { +// Compare the two Dashboards ignoring slices order +func (left *Dashboard) Equals(right *Dashboard) bool { - // TODO: Can be substiuted with cmp.Equal() if we can make it order indipendent for slices + opts := cmp.Options{ + cmpopts.IgnoreUnexported(Dashboard{}, Widget{}), - if left == nil || o == nil { - return left == o - } - - right, ok := o.(*Dashboard) - if !ok { - return false - } - - if left.Name != right.Name || left.Description != right.Description { - return false - } + // sort Widgets + cmp.Transformer("SortWidgets", func(in []*Widget) []*Widget { + out := make([]*Widget, len(in)) + copy(out, in) // copy input to avoid mutating it + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out + }), - if left.Widgets == nil || right.Widgets == nil { - return left.Widgets == nil && right.Widgets == nil + // sort strings (Widget.Filters, WidgetContentParameters.ContentFields) + cmp.Transformer("SortStrings", func(in []string) []string { + out := make([]string, len(in)) + copy(out, in) // copy input to avoid mutating it + sort.Strings(out) + return out + }), } - - leftWidgets := make([]util.Comparable, len(left.Widgets)) - for i := range left.Widgets { - leftWidgets[i] = left.Widgets[i] - } - - rightWidgets := make([]util.Comparable, len(right.Widgets)) - for i := range right.Widgets { - rightWidgets[i] = right.Widgets[i] - } - - return util.CompareSlices(leftWidgets, rightWidgets) -} - -func (d *Dashboard) Key() string { - return d.Name -} - -func (left *Widget) Equals(o util.Comparable) bool { - if left == nil || o == nil { - return left == o - } - - right, ok := o.(*Widget) - if !ok { - return false - } - - return left.Name == right.Name && - left.Description == right.Description && - left.WidgetType == right.WidgetType && - left.WidgetSize.Equals(&right.WidgetSize) && - util.CompareStringSlices(left.Filters, right.Filters) && - left.WidgetPosition.Equals(&right.WidgetPosition) && - left.ContentParameters.Equals(&right.ContentParameters) -} - -func (d *Widget) Key() string { - return d.Name -} - -func (left *WidgetSize) Equals(right *WidgetSize) bool { - - if left == nil || right == nil { - return left == right - } - - return left.Height == right.Height && - left.Width == right.Width -} - -func (left *WidgetPosition) Equals(right *WidgetPosition) bool { - - if left == nil || right == nil { - return left == right - } - - return left.PositionX == right.PositionX && - left.PositionY == right.PositionY -} - -func (left *WidgetContentParameters) Equals(right *WidgetContentParameters) bool { - - if left == nil || right == nil { - return left == right - } - - return util.CompareStringSlices(left.ContentFields, right.ContentFields) && - left.ItemsCount == right.ItemsCount && - reflect.DeepEqual(right.WidgetOptions, left.WidgetOptions) + return cmp.Equal(left, right, opts) } func (d *Dashboard) HashName() string { diff --git a/pkg/rpdac/dashboard_test.go b/pkg/rpdac/dashboard_test.go index 49a511e..21b4301 100644 --- a/pkg/rpdac/dashboard_test.go +++ b/pkg/rpdac/dashboard_test.go @@ -1840,14 +1840,14 @@ func TestDashboardEquals(t *testing.T) { expexct: false, }, { - description: "Compare dashboards wehre one with nil widgets should return false", + description: "Compare dashboards wehre one with nil widgets should return true", left: &Dashboard{ Widgets: []*Widget{}, }, right: &Dashboard{ Widgets: nil, }, - expexct: false, + expexct: true, }, { description: "Compare dashboards with different widgets should return false", @@ -2078,7 +2078,11 @@ func TestWidgetEquals(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { - r := test.left.Equals(test.right) + + left := &Dashboard{Widgets: []*Widget{test.left}} + right := &Dashboard{Widgets: []*Widget{test.right}} + + r := left.Equals(right) if r != test.expexct { t.Errorf("expected '%t' but got '%t'", test.expexct, r) } diff --git a/pkg/rpdac/filter.go b/pkg/rpdac/filter.go index e2065ff..c2e7439 100644 --- a/pkg/rpdac/filter.go +++ b/pkg/rpdac/filter.go @@ -3,9 +3,11 @@ package rpdac import ( "fmt" "log" + "sort" "github.com/b1zzu/reportportal-dashboards-as-code/pkg/reportportal" - "github.com/b1zzu/reportportal-dashboards-as-code/pkg/util" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "gopkg.in/yaml.v2" ) @@ -209,60 +211,29 @@ func (f *Filter) GetKind() ObjectKind { return f.Kind } -func conditionsToStringSlice(conditions []FilterCondition) []string { - s := make([]string, len(conditions)) - for i, c := range conditions { - s[i] = c.Condition + c.FilteringField + c.Value - } - return s -} - -func ordersToStringSlice(orders []FilterOrder) []string { - s := make([]string, len(orders)) - for i, o := range orders { - sort := "dsc" - if o.IsAsc { - sort = "asc" - } - - s[i] = o.SortingColumn + sort - } - return s -} - func (left *Filter) Equals(right *Filter) bool { - if left == nil || right == nil { - return left == right - } - - if left.Name != right.Name || - left.Type != right.Type || - left.Description != right.Description { - - return false - } - - // compare conditions - if left.Conditions != nil && right.Conditions != nil { - - leftConditions := conditionsToStringSlice(left.Conditions) - rightCondition := conditionsToStringSlice(right.Conditions) - - if !util.CompareStringSlices(leftConditions, rightCondition) { - return false - } - - } else if !(left.Conditions == nil && right.Conditions == nil) { - return false - } - - // compare orders - if left.Orders == nil || right.Orders == nil { - return left.Orders == nil && right.Orders == nil - } - - leftOrders := ordersToStringSlice(left.Orders) - rightOrders := ordersToStringSlice(right.Orders) - - return util.CompareStringSlices(leftOrders, rightOrders) + opts := cmp.Options{ + cmpopts.IgnoreUnexported(Filter{}), + + // sort FilterConditions + cmp.Transformer("SortConditions", func(in []FilterCondition) []FilterCondition { + out := make([]FilterCondition, len(in)) + copy(out, in) // copy input to avoid mutating it + sort.Slice(out, func(i, j int) bool { + return fmt.Sprintf("%+v", out[i]) < fmt.Sprintf("%+v", out[j]) + }) + return out + }), + + // sort FilterOrders + cmp.Transformer("SortOrders", func(in []FilterOrder) []FilterOrder { + out := make([]FilterOrder, len(in)) + copy(out, in) // copy input to avoid mutating it + sort.Slice(out, func(i, j int) bool { + return fmt.Sprintf("%+v", out[i]) < fmt.Sprintf("%+v", out[j]) + }) + return out + }), + } + return cmp.Equal(left, right, opts) } diff --git a/pkg/rpdac/filter_test.go b/pkg/rpdac/filter_test.go index 29ae9bf..e716235 100644 --- a/pkg/rpdac/filter_test.go +++ b/pkg/rpdac/filter_test.go @@ -707,24 +707,24 @@ func TestFilterEquals(t *testing.T) { expexct: false, }, { - description: "Compare filters where one has nil conditions should return false", + description: "Compare filters where one has nil conditions should return true", left: &Filter{ Conditions: []FilterCondition{}, }, right: &Filter{ Conditions: nil, }, - expexct: false, + expexct: true, }, { - description: "Compare filters where one has nil orders should return false", + description: "Compare filters where one has nil orders should return true", left: &Filter{ Orders: []FilterOrder{}, }, right: &Filter{ Orders: nil, }, - expexct: false, + expexct: true, }, { description: "Compare filters with different condtions should return false", diff --git a/pkg/util/equal.go b/pkg/util/equal.go deleted file mode 100644 index 2341c16..0000000 --- a/pkg/util/equal.go +++ /dev/null @@ -1,93 +0,0 @@ -package util - -import ( - "sort" -) - -type Comparable interface { - Equals(c Comparable) bool - - // key is used sorting before compare slices and must be a unique identifier in the slice - Key() string -} - -func copySlice(s []Comparable) []Comparable { - c := make([]Comparable, len(s)) - copy(c, s) - return c -} - -func sortSlice(s []Comparable) { - sort.Slice(s, func(i, j int) bool { - return s[i].Key() < s[j].Key() - }) -} - -func CompareSlices(l []Comparable, r []Comparable) bool { - - // if one of the two is nil return true if both are nil otherwise return false - if l == nil || r == nil { - return l == nil && r == nil - } - - // if they are not of the same lenght return false - if len(l) != len(r) { - return false - } - - // copy the slice so we can sort them without changing the original slices - left := copySlice(l) - right := copySlice(r) - - sortSlice(left) - sortSlice(right) - - for i, l := range left { - r := right[i] - - // if one of the two is nil continue if both are nil otherwise return false - if l == nil || r == nil { - if l == nil && r == nil { - continue - } else { - return false - } - } - - if !l.Equals(r) { - return false - } - } - return true -} - -func CompareStringSlices(l []string, r []string) bool { - - // if one of the two is nil return true if both are nil otherwise return false - if l == nil || r == nil { - return l == nil && r == nil - } - - // if they are not of the same lenght return false - if len(l) != len(r) { - return false - } - - left := make([]string, len(l)) - copy(left, l) - - right := make([]string, len(r)) - copy(right, r) - - sort.StringSlice(left).Sort() - sort.StringSlice(right).Sort() - - for i, l := range left { - r := right[i] - - if l != r { - return false - } - } - return true -} diff --git a/pkg/util/equal_test.go b/pkg/util/equal_test.go deleted file mode 100644 index 8a8aafe..0000000 --- a/pkg/util/equal_test.go +++ /dev/null @@ -1,149 +0,0 @@ -package util - -import ( - "testing" -) - -type Object struct { - Name string -} - -type ComparableObject struct { - Name string -} - -func (left *ComparableObject) Equals(r Comparable) bool { - - // left can not be nil so if right is nil they are for sure different - if r == nil { - return false - } - - right, ok := r.(*ComparableObject) - if !ok { - return false - } - - return left.Name == right.Name -} - -func (o *ComparableObject) Key() string { - return o.Name -} - -func TestCompareSlices(t *testing.T) { - - tests := []*struct { - description string - left []*ComparableObject - right []*ComparableObject - expexct bool - }{ - { - description: "Compare equal slices should return true", - left: []*ComparableObject{{Name: "One"}, {Name: "Two"}}, - right: []*ComparableObject{{Name: "One"}, {Name: "Two"}}, - expexct: true, - }, - { - description: "Compare equal slices but with differnt order should return true", - left: []*ComparableObject{{Name: "One"}, {Name: "Two"}}, - right: []*ComparableObject{{Name: "Two"}, {Name: "One"}}, - expexct: true, - }, - { - description: "Compare different slices should return false", - left: []*ComparableObject{{Name: "One"}}, - right: []*ComparableObject{{Name: "Two"}, {Name: "One"}}, - expexct: false, - }, - { - description: "Compare nil slices should return true", - left: nil, - right: nil, - expexct: true, - }, - { - description: "Compare a nil slice with a normal slice should return false", - left: []*ComparableObject{{Name: "One"}}, - right: nil, - expexct: false, - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - - left := make([]Comparable, len(test.left)) - for i := range test.left { - left[i] = test.left[i] - } - - right := make([]Comparable, len(test.right)) - for i := range test.right { - right[i] = test.right[i] - } - - r := CompareSlices(left, right) - if r != test.expexct { - t.Errorf("expected '%t' but got '%t'", test.expexct, r) - } - }) - } -} - -func TestCompareStringSlices(t *testing.T) { - - tests := []*struct { - description string - left []string - right []string - expexct bool - }{ - { - description: "Compare equal slices should return true", - left: []string{"One", "Two"}, - right: []string{"One", "Two"}, - expexct: true, - }, - { - description: "Compare equal slices but with differnt order should return true", - left: []string{"One", "Two"}, - right: []string{"Two", "One"}, - expexct: true, - }, - { - description: "Compare different slices should return false", - left: []string{"One", "Two"}, - right: []string{"Three", "Two"}, - expexct: false, - }, - { - description: "Compare different slices with different number lengths should return false", - left: []string{"One", "Two"}, - right: []string{"Two"}, - expexct: false, - }, - { - description: "Compare nil slices should return true", - left: nil, - right: nil, - expexct: true, - }, - { - description: "Compare a nil slice with a normal slice should return false", - left: []string{"One"}, - right: nil, - expexct: false, - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - r := CompareStringSlices(test.left, test.right) - if r != test.expexct { - t.Errorf("expected '%t' but got '%t'", test.expexct, r) - } - }) - } -}