From f8d00cc5355e7d063d5e7cd9128ff838e7a91caa Mon Sep 17 00:00:00 2001
From: Nick Turner <nic@amazon.com>
Date: Mon, 12 Oct 2020 10:24:41 -0700
Subject: [PATCH 1/3] Add test for Addon Menu and fix merge functionality

---
 channels/pkg/channels/addons_test.go | 64 ++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/channels/pkg/channels/addons_test.go b/channels/pkg/channels/addons_test.go
index 990df7604f3d7..095f37c6ed4cf 100644
--- a/channels/pkg/channels/addons_test.go
+++ b/channels/pkg/channels/addons_test.go
@@ -17,7 +17,9 @@ limitations under the License.
 package channels
 
 import (
+	"fmt"
 	"net/url"
+	"reflect"
 	"testing"
 
 	"github.com/blang/semver/v4"
@@ -197,6 +199,68 @@ func Test_UnparseableVersion(t *testing.T) {
 	assert.EqualError(t, err, "addon \"testaddon\" has unparseable version \"1.0-kops\": Short version cannot contain PreRelease/Build meta data", "detected invalid version")
 }
 
+func Test_MergeAddons(t *testing.T) {
+	merges := []struct {
+		LeftSide           *AddonMenu
+		RightSide          *AddonMenu
+		ExpectedAfterMerge *AddonMenu
+	}{
+		{
+			LeftSide:           addonMenu(addon(t, "a", "1.0.0", ">=1.18.0", "k8s-1.18")),
+			RightSide:          addonMenu(addon(t, "a", "1.0.1", ">=1.18.0", "k8s-1.18")),
+			ExpectedAfterMerge: addonMenu(addon(t, "a", "1.0.1", ">=1.18.0", "k8s-1.18")),
+		},
+		{
+			LeftSide:           addonMenu(addon(t, "a", "1.0.1", ">=1.18.0", "k8s-1.18")),
+			RightSide:          addonMenu(addon(t, "a", "1.0.0", ">=1.18.0", "k8s-1.18")),
+			ExpectedAfterMerge: addonMenu(addon(t, "a", "1.0.1", ">=1.18.0", "k8s-1.18")),
+		},
+	}
+
+	for _, m := range merges {
+		m.LeftSide.MergeAddons(m.RightSide)
+		if !reflect.DeepEqual(m.LeftSide, m.ExpectedAfterMerge) {
+			t.Errorf("Unexpected AddonMenu merge result,\nMerged:\n%s\nExpected:\n%s\n", addonMenuString(m.LeftSide), addonMenuString(m.ExpectedAfterMerge))
+		}
+	}
+}
+
 func s(v string) *string {
 	return &v
 }
+
+func addon(t *testing.T, name, version, kubernetesVersion, Id string) *Addon {
+	location, err := url.Parse("file://x/y/z")
+	require.NoError(t, err, "parsing file url")
+	return &Addon{
+		Name:            name,
+		ChannelName:     "test",
+		ChannelLocation: *location,
+		Spec: &api.AddonSpec{
+			Name:              &name,
+			Version:           &version,
+			KubernetesVersion: kubernetesVersion,
+			Id:                Id,
+		},
+	}
+}
+
+func addonMenu(addons ...*Addon) *AddonMenu {
+	addonMenu := NewAddonMenu()
+	for _, addon := range addons {
+		addonMenu.Addons[addon.Name] = addon
+	}
+	return addonMenu
+}
+
+func addonString(addon *Addon) string {
+	return fmt.Sprintf("  Addon{Name: %s, Version: %s, KubernetesVersion: %s, Id: %s},\n", *addon.Spec.Name, *addon.Spec.Version, addon.Spec.KubernetesVersion, addon.Spec.Id)
+}
+
+func addonMenuString(addonMenu *AddonMenu) string {
+	addonMenuString := fmt.Sprintf("AddonMenu{\n")
+	for _, addon := range addonMenu.Addons {
+		addonMenuString += addonString(addon)
+	}
+	return addonMenuString + fmt.Sprintf("}\n")
+}

From 61ac9a5b071214c6a4a8202f361b5b8cd5dc6def Mon Sep 17 00:00:00 2001
From: Nick Turner <nic@amazon.com>
Date: Tue, 13 Oct 2020 10:55:31 -0700
Subject: [PATCH 2/3] Fix bug in MergeAddons

* Replace existing addon if v (new) replaces() existing, instead
  of the other way around.
---
 channels/pkg/channels/addon.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/channels/pkg/channels/addon.go b/channels/pkg/channels/addon.go
index 35426a84e8168..327cf479efe7c 100644
--- a/channels/pkg/channels/addon.go
+++ b/channels/pkg/channels/addon.go
@@ -59,7 +59,7 @@ func (m *AddonMenu) MergeAddons(o *AddonMenu) {
 		if existing == nil {
 			m.Addons[k] = v
 		} else {
-			if existing.ChannelVersion().replaces(v.ChannelVersion()) {
+			if v.ChannelVersion().replaces(existing.ChannelVersion()) {
 				m.Addons[k] = v
 			}
 		}

From 0e442aaaddfd92af4630ad8ed3f81e826d8a0177 Mon Sep 17 00:00:00 2001
From: Nicholas Turner <1205393+nckturner@users.noreply.github.com>
Date: Wed, 14 Oct 2020 21:30:35 -0700
Subject: [PATCH 3/3] Fix staticcheck and gofmt

Co-authored-by: Ciprian Hacman <ciprianhacman@gmail.com>
---
 channels/pkg/channels/addons_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/channels/pkg/channels/addons_test.go b/channels/pkg/channels/addons_test.go
index 095f37c6ed4cf..db79094f68a6a 100644
--- a/channels/pkg/channels/addons_test.go
+++ b/channels/pkg/channels/addons_test.go
@@ -258,9 +258,9 @@ func addonString(addon *Addon) string {
 }
 
 func addonMenuString(addonMenu *AddonMenu) string {
-	addonMenuString := fmt.Sprintf("AddonMenu{\n")
+	addonMenuString := "AddonMenu{\n"
 	for _, addon := range addonMenu.Addons {
 		addonMenuString += addonString(addon)
 	}
-	return addonMenuString + fmt.Sprintf("}\n")
+	return addonMenuString + "}\n"
 }