From e2007733f4abeff9a9a931b099fb396814eb3792 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 19 Nov 2018 11:20:44 +0100 Subject: [PATCH 1/5] build: table-driven tests for publisher. --- .../build/release_publisher/publisher_test.go | 75 +++++++++++-------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/scripts/build/release_publisher/publisher_test.go b/scripts/build/release_publisher/publisher_test.go index 76a1446406a..a61f6ef432d 100644 --- a/scripts/build/release_publisher/publisher_test.go +++ b/scripts/build/release_publisher/publisher_test.go @@ -4,44 +4,57 @@ import "testing" func TestPreparingReleaseFromRemote(t *testing.T) { - var builder releaseBuilder - - versionIn := "v5.2.0-beta1" - expectedVersion := "5.2.0-beta1" - whatsNewUrl := "https://whatsnews.foo/" - relNotesUrl := "https://relnotes.foo/" - expectedArch := "amd64" - expectedOs := "linux" - buildArtifacts := []buildArtifact{{expectedOs, expectedArch, ".linux-amd64.tar.gz"}} - - builder = releaseFromExternalContent{ - getter: mockHttpGetter{}, - rawVersion: versionIn, - artifactConfigurations: buildArtifactConfigurations, + cases := []struct { + version string + expectedVersion string + whatsNewUrl string + relNotesUrl string + expectedArch string + expectedOs string + buildArtifacts []buildArtifact + }{ + { + version: "v5.2.0-beta1", + expectedVersion: "5.2.0-beta1", + whatsNewUrl: "https://whatsnews.foo/", + relNotesUrl: "https://relnotes.foo/", + expectedArch: "amd64", + expectedOs: "linux", + buildArtifacts: []buildArtifact{{"linux", "amd64", ".linux-amd64.tar.gz"}}, + }, } - rel, _ := builder.prepareRelease("https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", whatsNewUrl, relNotesUrl, false) + for _, test := range cases { + var builder releaseBuilder + builder = releaseFromExternalContent{ + getter: mockHttpGetter{}, + rawVersion: test.version, + artifactConfigurations: test.buildArtifacts, + } - if !rel.Beta || rel.Stable { - t.Errorf("%s should have been tagged as beta (not stable), but wasn't .", versionIn) - } + rel, _ := builder.prepareRelease("https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", test.whatsNewUrl, test.relNotesUrl, false) - if rel.Version != expectedVersion { - t.Errorf("Expected version to be %s, but it was %s.", expectedVersion, rel.Version) - } + if !rel.Beta || rel.Stable { + t.Errorf("%s should have been tagged as beta (not stable), but wasn't .", test.version) + } - expectedBuilds := len(buildArtifacts) - if len(rel.Builds) != expectedBuilds { - t.Errorf("Expected %v builds, but got %v.", expectedBuilds, len(rel.Builds)) - } + if rel.Version != test.expectedVersion { + t.Errorf("Expected version to be %s, but it was %s.", test.expectedVersion, rel.Version) + } - build := rel.Builds[0] - if build.Arch != expectedArch { - t.Errorf("Expected arch to be %v, but it was %v", expectedArch, build.Arch) - } + expectedBuilds := len(test.buildArtifacts) + if len(rel.Builds) != expectedBuilds { + t.Errorf("Expected %v builds, but got %v.", expectedBuilds, len(rel.Builds)) + } - if build.Os != expectedOs { - t.Errorf("Expected arch to be %v, but it was %v", expectedOs, build.Os) + build := rel.Builds[0] + if build.Arch != test.expectedArch { + t.Errorf("Expected arch to be %v, but it was %v", test.expectedArch, build.Arch) + } + + if build.Os != test.expectedOs { + t.Errorf("Expected arch to be %v, but it was %v", test.expectedOs, build.Os) + } } } From 2d361eeabff27c30b71bb93b388ff527cccb9a80 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 19 Nov 2018 13:26:35 +0100 Subject: [PATCH 2/5] builds: introduces enum for relase type. --- .../build/release_publisher/externalrelease.go | 8 ++++++-- scripts/build/release_publisher/localrelease.go | 2 +- scripts/build/release_publisher/publisher.go | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/scripts/build/release_publisher/externalrelease.go b/scripts/build/release_publisher/externalrelease.go index 181dd4088ad..c18b94e5bbb 100644 --- a/scripts/build/release_publisher/externalrelease.go +++ b/scripts/build/release_publisher/externalrelease.go @@ -17,14 +17,18 @@ type releaseFromExternalContent struct { func (re releaseFromExternalContent) prepareRelease(baseArchiveUrl, whatsNewUrl string, releaseNotesUrl string, nightly bool) (*release, error) { version := re.rawVersion[1:] isBeta := strings.Contains(version, "beta") + var rt ReleaseType + if isBeta { + rt = BETA + } builds := []build{} for _, ba := range re.artifactConfigurations { - sha256, err := re.getter.getContents(fmt.Sprintf("%s.sha256", ba.getUrl(baseArchiveUrl, version, isBeta))) + sha256, err := re.getter.getContents(fmt.Sprintf("%s.sha256", ba.getUrl(baseArchiveUrl, version, rt))) if err != nil { return nil, err } - builds = append(builds, newBuild(baseArchiveUrl, ba, version, isBeta, sha256)) + builds = append(builds, newBuild(baseArchiveUrl, ba, version, rt, sha256)) } r := release{ diff --git a/scripts/build/release_publisher/localrelease.go b/scripts/build/release_publisher/localrelease.go index 0bbecff9327..e416a6dd490 100644 --- a/scripts/build/release_publisher/localrelease.go +++ b/scripts/build/release_publisher/localrelease.go @@ -70,7 +70,7 @@ func createBuildWalker(path string, data *buildData, archiveTypes []buildArtifac data.version = version data.builds = append(data.builds, build{ Os: archive.os, - Url: archive.getUrl(baseArchiveUrl, version, false), + Url: archive.getUrl(baseArchiveUrl, version, NIGHTLY), Sha256: string(shaBytes), Arch: archive.arch, }) diff --git a/scripts/build/release_publisher/publisher.go b/scripts/build/release_publisher/publisher.go index d2c10d1640f..24be6d7dc85 100644 --- a/scripts/build/release_publisher/publisher.go +++ b/scripts/build/release_publisher/publisher.go @@ -61,13 +61,21 @@ func (p *publisher) postRelease(r *release) error { return nil } +type ReleaseType int + +const ( + STABLE ReleaseType = iota + 1 + BETA + NIGHTLY +) + type buildArtifact struct { os string arch string urlPostfix string } -func (t buildArtifact) getUrl(baseArchiveUrl, version string, isBeta bool) string { +func (t buildArtifact) getUrl(baseArchiveUrl, version string, rt ReleaseType) string { prefix := "-" rhelReleaseExtra := "" @@ -75,7 +83,7 @@ func (t buildArtifact) getUrl(baseArchiveUrl, version string, isBeta bool) strin prefix = "_" } - if !isBeta && t.os == "rhel" { + if rt == BETA && t.os == "rhel" { rhelReleaseExtra = "-1" } @@ -141,10 +149,10 @@ var buildArtifactConfigurations = []buildArtifact{ }, } -func newBuild(baseArchiveUrl string, ba buildArtifact, version string, isBeta bool, sha256 string) build { +func newBuild(baseArchiveUrl string, ba buildArtifact, version string, rt ReleaseType, sha256 string) build { return build{ Os: ba.os, - Url: ba.getUrl(baseArchiveUrl, version, isBeta), + Url: ba.getUrl(baseArchiveUrl, version, rt), Sha256: sha256, Arch: ba.arch, } From 8f0d3ff7eac934eff66a86d087310dda606bd34a Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 19 Nov 2018 14:06:18 +0100 Subject: [PATCH 3/5] build: fixes a bug where nightly rpm builds would be handled as stable. --- .../release_publisher/externalrelease.go | 14 ++-- .../build/release_publisher/localrelease.go | 3 + scripts/build/release_publisher/publisher.go | 16 ++++- .../build/release_publisher/publisher_test.go | 67 ++++++++++++++++--- 4 files changed, 83 insertions(+), 17 deletions(-) diff --git a/scripts/build/release_publisher/externalrelease.go b/scripts/build/release_publisher/externalrelease.go index c18b94e5bbb..992cba38f90 100644 --- a/scripts/build/release_publisher/externalrelease.go +++ b/scripts/build/release_publisher/externalrelease.go @@ -16,10 +16,14 @@ type releaseFromExternalContent struct { func (re releaseFromExternalContent) prepareRelease(baseArchiveUrl, whatsNewUrl string, releaseNotesUrl string, nightly bool) (*release, error) { version := re.rawVersion[1:] - isBeta := strings.Contains(version, "beta") + beta := strings.Contains(version, "beta") var rt ReleaseType - if isBeta { + if beta { rt = BETA + } else if nightly { + rt = NIGHTLY + } else { + rt = STABLE } builds := []build{} @@ -34,9 +38,9 @@ func (re releaseFromExternalContent) prepareRelease(baseArchiveUrl, whatsNewUrl r := release{ Version: version, ReleaseDate: time.Now().UTC(), - Stable: !isBeta && !nightly, - Beta: isBeta, - Nightly: nightly, + Stable: rt.stable(), + Beta: rt.beta(), + Nightly: rt.nightly(), WhatsNewUrl: whatsNewUrl, ReleaseNotesUrl: releaseNotesUrl, Builds: builds, diff --git a/scripts/build/release_publisher/localrelease.go b/scripts/build/release_publisher/localrelease.go index e416a6dd490..4f4575c4ff4 100644 --- a/scripts/build/release_publisher/localrelease.go +++ b/scripts/build/release_publisher/localrelease.go @@ -18,6 +18,9 @@ type releaseLocalSources struct { } func (r releaseLocalSources) prepareRelease(baseArchiveUrl, whatsNewUrl string, releaseNotesUrl string, nightly bool) (*release, error) { + if !nightly { + return nil, errors.New("Local releases only supported for nightly builds.") + } buildData := r.findBuilds(baseArchiveUrl) rel := release{ diff --git a/scripts/build/release_publisher/publisher.go b/scripts/build/release_publisher/publisher.go index 24be6d7dc85..dd0415ad3ce 100644 --- a/scripts/build/release_publisher/publisher.go +++ b/scripts/build/release_publisher/publisher.go @@ -69,13 +69,25 @@ const ( NIGHTLY ) +func (rt ReleaseType) beta() bool { + return rt == BETA +} + +func (rt ReleaseType) stable() bool { + return rt == STABLE +} + +func (rt ReleaseType) nightly() bool { + return rt == NIGHTLY +} + type buildArtifact struct { os string arch string urlPostfix string } -func (t buildArtifact) getUrl(baseArchiveUrl, version string, rt ReleaseType) string { +func (t buildArtifact) getUrl(baseArchiveUrl, version string, releaseType ReleaseType) string { prefix := "-" rhelReleaseExtra := "" @@ -83,7 +95,7 @@ func (t buildArtifact) getUrl(baseArchiveUrl, version string, rt ReleaseType) st prefix = "_" } - if rt == BETA && t.os == "rhel" { + if releaseType == STABLE && t.os == "rhel" { rhelReleaseExtra = "-1" } diff --git a/scripts/build/release_publisher/publisher_test.go b/scripts/build/release_publisher/publisher_test.go index a61f6ef432d..39a5bd5969b 100644 --- a/scripts/build/release_publisher/publisher_test.go +++ b/scripts/build/release_publisher/publisher_test.go @@ -5,23 +5,61 @@ import "testing" func TestPreparingReleaseFromRemote(t *testing.T) { cases := []struct { - version string + version string expectedVersion string - whatsNewUrl string - relNotesUrl string - expectedArch string - expectedOs string - buildArtifacts []buildArtifact + whatsNewUrl string + relNotesUrl string + nightly bool + expectedBeta bool + expectedStable bool + expectedArch string + expectedOs string + expectedUrl string + baseArchiveUrl string + buildArtifacts []buildArtifact }{ { version: "v5.2.0-beta1", expectedVersion: "5.2.0-beta1", whatsNewUrl: "https://whatsnews.foo/", relNotesUrl: "https://relnotes.foo/", + nightly: false, + expectedBeta: true, + expectedStable: false, expectedArch: "amd64", expectedOs: "linux", + expectedUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.2.0-beta1.linux-amd64.tar.gz", + baseArchiveUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", buildArtifacts: []buildArtifact{{"linux", "amd64", ".linux-amd64.tar.gz"}}, }, + { + version: "v5.2.3", + expectedVersion: "5.2.3", + whatsNewUrl: "https://whatsnews.foo/", + relNotesUrl: "https://relnotes.foo/", + nightly: false, + expectedBeta: false, + expectedStable: true, + expectedArch: "amd64", + expectedOs: "rhel", + expectedUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.2.3-1.x86_64.rpm", + baseArchiveUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", + buildArtifacts: []buildArtifact{{"rhel", "amd64", ".x86_64.rpm"}}, + }, + { + version: "v5.4.0-pre1asdf", + expectedVersion: "5.4.0-pre1asdf", + whatsNewUrl: "https://whatsnews.foo/", + relNotesUrl: "https://relnotes.foo/", + nightly: true, + expectedBeta: false, + expectedStable: false, + expectedArch: "amd64", + expectedOs: "rhel", + expectedUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.4.0-pre1asdf.x86_64.rpm", + baseArchiveUrl: "https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", + buildArtifacts: []buildArtifact{{"rhel", "amd64", ".x86_64.rpm"}}, + }, } for _, test := range cases { @@ -32,10 +70,10 @@ func TestPreparingReleaseFromRemote(t *testing.T) { artifactConfigurations: test.buildArtifacts, } - rel, _ := builder.prepareRelease("https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana", test.whatsNewUrl, test.relNotesUrl, false) + rel, _ := builder.prepareRelease(test.baseArchiveUrl, test.whatsNewUrl, test.relNotesUrl, test.nightly) - if !rel.Beta || rel.Stable { - t.Errorf("%s should have been tagged as beta (not stable), but wasn't .", test.version) + if rel.Beta != test.expectedBeta || rel.Stable != test.expectedStable { + t.Errorf("%s should have been tagged as beta=%v, stable=%v.", test.version, test.expectedBeta, test.expectedStable) } if rel.Version != test.expectedVersion { @@ -53,7 +91,11 @@ func TestPreparingReleaseFromRemote(t *testing.T) { } if build.Os != test.expectedOs { - t.Errorf("Expected arch to be %v, but it was %v", test.expectedOs, build.Os) + t.Errorf("Expected os to be %v, but it was %v", test.expectedOs, build.Os) + } + + if build.Url != test.expectedUrl { + t.Errorf("Expected url to be %v, but it was %v", test.expectedUrl, build.Url) } } } @@ -129,4 +171,9 @@ func TestPreparingReleaseFromLocal(t *testing.T) { if build.Os != expectedOs { t.Fatalf("Expected os to be %s, but was %s", expectedOs, build.Os) } + + _, err := builder.prepareRelease("", "", "", false) + if err == nil { + t.Error("Error was nil, but expected an error as the local releaser only supports nightly builds.") + } } From ac55aeff953b76be440a8c0056a05211dc611c80 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 19 Nov 2018 14:12:04 +0100 Subject: [PATCH 4/5] build: minor refactor. --- scripts/build/release_publisher/publisher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build/release_publisher/publisher.go b/scripts/build/release_publisher/publisher.go index dd0415ad3ce..ad54a1ccb9b 100644 --- a/scripts/build/release_publisher/publisher.go +++ b/scripts/build/release_publisher/publisher.go @@ -95,7 +95,7 @@ func (t buildArtifact) getUrl(baseArchiveUrl, version string, releaseType Releas prefix = "_" } - if releaseType == STABLE && t.os == "rhel" { + if releaseType.stable() && t.os == "rhel" { rhelReleaseExtra = "-1" } From b041ad4134935d536e47c0d613ad81d12e44276c Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 19 Nov 2018 14:32:39 +0100 Subject: [PATCH 5/5] linter. --- scripts/build/release_publisher/publisher_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/build/release_publisher/publisher_test.go b/scripts/build/release_publisher/publisher_test.go index 39a5bd5969b..1d5fb683b2c 100644 --- a/scripts/build/release_publisher/publisher_test.go +++ b/scripts/build/release_publisher/publisher_test.go @@ -63,8 +63,7 @@ func TestPreparingReleaseFromRemote(t *testing.T) { } for _, test := range cases { - var builder releaseBuilder - builder = releaseFromExternalContent{ + builder := releaseFromExternalContent{ getter: mockHttpGetter{}, rawVersion: test.version, artifactConfigurations: test.buildArtifacts,