From 053c9f03df230d34c5aea44ba7279aa14cd6c4d1 Mon Sep 17 00:00:00 2001 From: sebgl Date: Thu, 28 Nov 2019 16:26:25 +0100 Subject: [PATCH 1/2] Fix packages sort algorithm We want to compare resource group first, then fallback to resource version if the group is the same. The previous version of the sort algorithm did not work as expected in some situations, by comparing resources version even though their group does not match. Only the case where groupA < groupB was considered for group comparison, leading to a version comparison if groupA > groupB. This commit fixes it by making sure we evaluate version comparison only if groups are the same. It is verified by a unit test that does not pass with the previous version of the sort algorithm. --- main.go | 18 +++++++----- main_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 main_test.go diff --git a/main.go b/main.go index 37a1a5c..6aa8f47 100644 --- a/main.go +++ b/main.go @@ -271,17 +271,21 @@ func combineAPIPackages(pkgs []*types.Package) ([]*apiPackage, error) { for _, v := range pkgMap { out = append(out, v) } + sortPackages(out) - sort.SliceStable(out, func(i, j int) bool { - a := out[i] - b := out[j] - if a.apiGroup < b.apiGroup { - return true + return out, nil +} + +// sortPackages sorts the given packages in a consistent alphabetical order. +func sortPackages(packages []*apiPackage) { + sort.SliceStable(packages, func(i, j int) bool { + a := packages[i] + b := packages[j] + if a.apiGroup != b.apiGroup { + return a.apiGroup < b.apiGroup } return a.apiVersion < b.apiVersion }) - - return out, nil } // isVendorPackage determines if package is coming from vendor/ dir. diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000..9c226f6 --- /dev/null +++ b/main_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "reflect" + "testing" +) + +func Test_sortPackages(t *testing.T) { + tests := []struct { + name string + packages []*apiPackage + expected []*apiPackage + }{ + { + name: "sort by group then version", + packages: []*apiPackage{ + { + apiGroup: "a", + apiVersion: "v1", + }, + { + apiGroup: "c", + apiVersion: "v1beta1", + }, + { + apiGroup: "b", + apiVersion: "v1beta1", + }, + { + apiGroup: "c", + apiVersion: "v1", + }, + { + apiGroup: "b", + apiVersion: "v1", + }, + { + apiGroup: "a", + apiVersion: "v1beta1", + }, + }, + expected: []*apiPackage{ + { + apiGroup: "a", + apiVersion: "v1", + }, + { + apiGroup: "a", + apiVersion: "v1beta1", + }, + { + apiGroup: "b", + apiVersion: "v1", + }, + { + apiGroup: "b", + apiVersion: "v1beta1", + }, + { + apiGroup: "c", + apiVersion: "v1", + }, + { + apiGroup: "c", + apiVersion: "v1beta1", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sortPackages(tt.packages) + if !reflect.DeepEqual(tt.expected, tt.packages) { + t.Error("Unexpected packages ordering") + } + }) + } +} \ No newline at end of file From 16e1a602816225cb515a9e723fc313c1f1c346e9 Mon Sep 17 00:00:00 2001 From: sebgl Date: Thu, 28 Nov 2019 16:27:00 +0100 Subject: [PATCH 2/2] Don't init flags in init() An issue prevents unit tests to run if flags are initialized in an init() function. More details there: https://github.com/golang/go/issues/31859. A workaround is to init flags another way (here by calling a function from main()). --- main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 6aa8f47..7ab05aa 100644 --- a/main.go +++ b/main.go @@ -75,7 +75,7 @@ type apiPackage struct { func (v *apiPackage) identifier() string { return fmt.Sprintf("%s/%s", v.apiGroup, v.apiVersion) } -func init() { +func initFlags() { klog.InitFlags(nil) flag.Set("alsologtostderr", "true") // for klog flag.Parse() @@ -114,6 +114,8 @@ func resolveTemplateDir(dir string) error { func main() { defer klog.Flush() + initFlags() + f, err := os.Open(*flConfig) if err != nil { klog.Fatalf("failed to open config file: %+v", err)