From c1c22c8701da170f7ed32dccf927265a680ae431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mudrini=C4=87?= Date: Thu, 24 Aug 2023 17:25:15 +0200 Subject: [PATCH 1/2] Fix version comparison in VerifyLatestUpdate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marko Mudrinić --- pkg/release/publish.go | 12 ++- pkg/release/publish_test.go | 178 ++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) diff --git a/pkg/release/publish.go b/pkg/release/publish.go index 4b50bd498e1..87fb3f2b6c4 100644 --- a/pkg/release/publish.go +++ b/pkg/release/publish.go @@ -282,7 +282,17 @@ func (p *Publisher) VerifyLatestUpdate( return false, fmt.Errorf("invalid GCS version format %s", gcsVersion) } - if sv.LTE(gcsSemverVersion) { + // The version format that we use is: + // - Stable: "v..-+" + // - Prerelease: "v..-.+" + // blang/semver library considers "-" part as the prerelease part even though + // the version is stable. This is causing issues when we're supposed to bump the version marker + // from rc to stable. + // blang/semver models prerelease as a slice of prereleases, so stable releases + // are going to have one element (the number of commits), and prereleases are going to have three elements + // (alpha/beta/rc, number of prerelease, number of commits). + // We can use this to determine when we're switching from rc to stable and act accordingly. + if sv.LTE(gcsSemverVersion) && len(sv.Pre) >= len(gcsSemverVersion.Pre) { logrus.Infof( "Not updating version, because %s <= %s", version, gcsVersion, ) diff --git a/pkg/release/publish_test.go b/pkg/release/publish_test.go index f76d1709cbd..ed41c0d9580 100644 --- a/pkg/release/publish_test.go +++ b/pkg/release/publish_test.go @@ -325,3 +325,181 @@ func TestPublishReleaseNotesIndex(t *testing.T) { } } } + +func TestVerifyLatestUpdate(t *testing.T) { + // err := errors.New("") + for _, tc := range []struct { + prepare func(*releasefakes.FakePublisherClient, string) + version string + gcsVersion string + needsUpdate bool + shouldError bool + }{ + { // success same version + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.24.0", + gcsVersion: "v1.24.0", + needsUpdate: false, + shouldError: false, + }, + + { // success version > gcsVersion (patch) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.24.1", + gcsVersion: "v1.24.0", + needsUpdate: true, + shouldError: false, + }, + { // success version < gcsVersion (patch) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.24.0", + gcsVersion: "v1.24.1", + needsUpdate: false, + shouldError: false, + }, + + { // success version > gcsVersion (minor) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.25.0", + gcsVersion: "v1.24.0", + needsUpdate: true, + shouldError: false, + }, + { // success version < gcsVersion (minor) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.23.0", + gcsVersion: "v1.24.0", + needsUpdate: false, + shouldError: false, + }, + + { // success version = gcsVersion (with build version) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-7+c4e17abb04728e", + gcsVersion: "v1.28.0-7+c4e17abb04728e", + needsUpdate: false, + shouldError: false, + }, + { // success version > gcsVersion (with build version) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-9+aaaaaabb04728e", + gcsVersion: "v1.28.0-7+c4e17abb04728e", + needsUpdate: true, + shouldError: false, + }, + { // success version < gcsVersion (with build version) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-7+c4e17abb04728e", + gcsVersion: "v1.28.0-9+aaaaaabb04728e", + needsUpdate: false, + shouldError: false, + }, + { // success version > gcsVersion (with build version) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.1-1+aaaaaabb04728e", + gcsVersion: "v1.28.0-7+c4e17abb04728e", + needsUpdate: true, + shouldError: false, + }, + { // success version = gcsVersion (with build version, prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-rc.1.9+3fb5377b25ec51", + gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51", + needsUpdate: false, + shouldError: false, + }, + { // success version > gcsVersion (with build version, prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-rc.1.10+3fb5377b25ec51", + gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51", + needsUpdate: true, + shouldError: false, + }, + { // success version < gcsVersion (with build version, prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-rc.1.9+3fb5377b25ec51", + gcsVersion: "v1.28.0-rc.1.10+3fb5377b25ec51", + needsUpdate: false, + shouldError: false, + }, + { // success version < gcsVersion (with build version, prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-beta.1.9+3fb5377b25ec51", + gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51", + needsUpdate: false, + shouldError: false, + }, + { // success version > gcsVersion (with build version, prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-rc.1.9+3fb5377b25ec51", + gcsVersion: "v1.28.0-beta.1.9+3fb5377b25ec51", + needsUpdate: true, + shouldError: false, + }, + { // success version > gcsVersion (with build version, stable and prerelease) + prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { + mock.NormalizePath("", "") + mock.GSUtilOutputReturns(gcsVersion, nil) + }, + version: "v1.28.0-7+c4e17abb04728e", + gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51", + needsUpdate: true, + shouldError: false, + }, + } { + sut := release.NewPublisher() + clientMock := &releasefakes.FakePublisherClient{} + sut.SetClient(clientMock) + tc.prepare(clientMock, tc.gcsVersion) + + needsUpdate, err := sut.VerifyLatestUpdate("", "", tc.version) + if tc.shouldError { + require.NotNil(t, err) + } else { + require.Nil(t, err) + require.Equal(t, tc.needsUpdate, needsUpdate) + } + } +} From 23c723f1cc195959b4cb76b4434439ed1ad7e379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mudrini=C4=87?= Date: Fri, 25 Aug 2023 11:40:33 +0200 Subject: [PATCH 2/2] Fix lint errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marko Mudrinić --- pkg/release/publish_test.go | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/release/publish_test.go b/pkg/release/publish_test.go index ed41c0d9580..eabdca5a98d 100644 --- a/pkg/release/publish_test.go +++ b/pkg/release/publish_test.go @@ -327,7 +327,6 @@ func TestPublishReleaseNotesIndex(t *testing.T) { } func TestVerifyLatestUpdate(t *testing.T) { - // err := errors.New("") for _, tc := range []struct { prepare func(*releasefakes.FakePublisherClient, string) version string @@ -337,7 +336,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }{ { // success same version prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.24.0", @@ -348,7 +347,7 @@ func TestVerifyLatestUpdate(t *testing.T) { { // success version > gcsVersion (patch) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.24.1", @@ -358,7 +357,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version < gcsVersion (patch) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.24.0", @@ -369,7 +368,7 @@ func TestVerifyLatestUpdate(t *testing.T) { { // success version > gcsVersion (minor) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.25.0", @@ -379,7 +378,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version < gcsVersion (minor) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.23.0", @@ -390,7 +389,7 @@ func TestVerifyLatestUpdate(t *testing.T) { { // success version = gcsVersion (with build version) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-7+c4e17abb04728e", @@ -400,7 +399,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version > gcsVersion (with build version) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-9+aaaaaabb04728e", @@ -410,7 +409,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version < gcsVersion (with build version) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-7+c4e17abb04728e", @@ -420,7 +419,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version > gcsVersion (with build version) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.1-1+aaaaaabb04728e", @@ -430,7 +429,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version = gcsVersion (with build version, prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-rc.1.9+3fb5377b25ec51", @@ -440,7 +439,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version > gcsVersion (with build version, prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-rc.1.10+3fb5377b25ec51", @@ -450,7 +449,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version < gcsVersion (with build version, prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-rc.1.9+3fb5377b25ec51", @@ -460,7 +459,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version < gcsVersion (with build version, prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-beta.1.9+3fb5377b25ec51", @@ -470,7 +469,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version > gcsVersion (with build version, prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-rc.1.9+3fb5377b25ec51", @@ -480,7 +479,7 @@ func TestVerifyLatestUpdate(t *testing.T) { }, { // success version > gcsVersion (with build version, stable and prerelease) prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) { - mock.NormalizePath("", "") + mock.NormalizePathReturns("", nil) mock.GSUtilOutputReturns(gcsVersion, nil) }, version: "v1.28.0-7+c4e17abb04728e",