From a200915862d5cecf1bb1fc99c7aa5ca38b148a92 Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Thu, 12 Oct 2017 14:21:50 +0200 Subject: [PATCH 1/9] Add failing specs for commit_sha file and metadata --- fakes/fake_git_hub.go | 42 ++++++++++++++++++++++++++++++++++++++++++ github.go | 10 ++++++++++ github_test.go | 36 ++++++++++++++++++++++++++++++++++++ in_command_test.go | 22 ++++++++++++++++++++++ 4 files changed, 110 insertions(+) diff --git a/fakes/fake_git_hub.go b/fakes/fake_git_hub.go index a4c21e8..3f9b929 100644 --- a/fakes/fake_git_hub.go +++ b/fakes/fake_git_hub.go @@ -109,6 +109,15 @@ type FakeGitHub struct { result1 *url.URL result2 error } + GetRefStub func(tag string) (*github.Reference, error) + getRefMutex sync.RWMutex + getRefArgsForCall []struct { + tag string + } + getRefReturns struct { + result1 *github.Reference + result2 error + } } func (fake *FakeGitHub) ListReleases() ([]*github.RepositoryRelease, error) { @@ -466,4 +475,37 @@ func (fake *FakeGitHub) GetZipballLinkReturns(result1 *url.URL, result2 error) { }{result1, result2} } +func (fake *FakeGitHub) GetRef(tag string) (*github.Reference, error) { + fake.getRefMutex.Lock() + fake.getRefArgsForCall = append(fake.getRefArgsForCall, struct { + tag string + }{tag}) + fake.getRefMutex.Unlock() + if fake.GetRefStub != nil { + return fake.GetRefStub(tag) + } else { + return fake.getRefReturns.result1, fake.getRefReturns.result2 + } +} + +func (fake *FakeGitHub) GetRefCallCount() int { + fake.getRefMutex.RLock() + defer fake.getRefMutex.RUnlock() + return len(fake.getRefArgsForCall) +} + +func (fake *FakeGitHub) GetRefArgsForCall(i int) string { + fake.getRefMutex.RLock() + defer fake.getRefMutex.RUnlock() + return fake.getRefArgsForCall[i].tag +} + +func (fake *FakeGitHub) GetRefReturns(result1 *github.Reference, result2 error) { + fake.GetRefStub = nil + fake.getRefReturns = struct { + result1 *github.Reference + result2 error + }{result1, result2} +} + var _ resource.GitHub = new(FakeGitHub) diff --git a/github.go b/github.go index a8e7195..71151c3 100644 --- a/github.go +++ b/github.go @@ -31,6 +31,7 @@ type GitHub interface { GetTarballLink(tag string) (*url.URL, error) GetZipballLink(tag string) (*url.URL, error) + GetRef(tag string) (*github.Reference, error) } type GitHubClient struct { @@ -247,6 +248,15 @@ func (g *GitHubClient) GetZipballLink(tag string) (*url.URL, error) { return u, nil } +func (g *GitHubClient) GetRef(tag string) (*github.Reference, error) { + ref, res, err := g.client.Git.GetRef(context.TODO(), g.owner, g.repository, "tags/"+tag) + if err != nil { + return nil, err + } + res.Body.Close() + return ref, nil +} + func oauthClient(ctx context.Context, source Source) (*http.Client, error) { ts := oauth2.StaticTokenSource(&oauth2.Token{ AccessToken: source.AccessToken, diff --git a/github_test.go b/github_test.go index 1bad6bd..18b10d0 100644 --- a/github_test.go +++ b/github_test.go @@ -189,4 +189,40 @@ var _ = Describe("GitHub Client", func() { }) }) }) + + Describe("GetRef", func() { + BeforeEach(func() { + source = Source{ + Owner: "concourse", + Repository: "concourse", + } + }) + Context("When GitHub's rate limit has been exceeded", func() { + BeforeEach(func() { + rateLimitResponse := `{ + "message": "API rate limit exceeded for 127.0.0.1. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", + "documentation_url": "https://developer.github.com/v3/#rate-limiting" + }` + + rateLimitHeaders := http.Header(map[string][]string{ + "X-RateLimit-Limit": {"60"}, + "X-RateLimit-Remaining": {"0"}, + "X-RateLimit-Reset": {"1377013266"}, + }) + + server.AppendHandlers( + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/git/refs/tags/some-tag"), + ghttp.RespondWith(403, rateLimitResponse, rateLimitHeaders), + ), + ) + }) + + It("Returns an appropriate error", func() { + _, err := client.GetRef("some-tag") + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("API rate limit exceeded for 127.0.0.1. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)")) + }) + }) + }) }) diff --git a/in_command_test.go b/in_command_test.go index a0818a9..e1b0760 100644 --- a/in_command_test.go +++ b/in_command_test.go @@ -86,10 +86,23 @@ var _ = Describe("In Command", func() { } } + buildTagRef := func(tagRef, commitSHA string) *github.Reference { + return &github.Reference{ + Ref: github.String(tagRef), + URL: github.String("https://example.com"), + Object: &github.GitObject{ + Type: github.String("commit"), + SHA: github.String(commitSHA), + URL: github.String("https://example.com"), + }, + } + } + Context("when there is a tagged release", func() { Context("when a present version is specified", func() { BeforeEach(func() { githubClient.GetReleaseByTagReturns(buildRelease(1, "v0.35.0", false), nil) + githubClient.GetRefReturns(buildTagRef("v0.35.0", "f28085a4a8f744da83411f5e09fd7b1709149eee"), nil) githubClient.ListReleaseAssetsReturns([]*github.ReleaseAsset{ buildAsset(0, "example.txt"), @@ -129,6 +142,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -157,6 +171,10 @@ var _ = Describe("In Command", func() { Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("0.35.0")) + contents, err = ioutil.ReadFile(path.Join(destDir, "commit_sha")) + Ω(err).ShouldNot(HaveOccurred()) + Ω(string(contents)).Should(Equal("f28085a4a8f744da83411f5e09fd7b1709149eee")) + contents, err = ioutil.ReadFile(path.Join(destDir, "body")) Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("*markdown*")) @@ -343,6 +361,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -416,6 +435,7 @@ var _ = Describe("In Command", func() { Context("which has a tag", func() { BeforeEach(func() { githubClient.GetReleaseReturns(buildRelease(1, "v0.35.0", true), nil) + githubClient.GetRefReturns(buildTagRef("v0.35.0", "f28085a4a8f744da83411f5e09fd7b1709149eee"), nil) inRequest.Version = &resource.Version{ID: "1"} inResponse, inErr = command.Run(destDir, inRequest) @@ -435,6 +455,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, resource.MetadataPair{Name: "draft", Value: "true"}, )) }) @@ -479,6 +500,7 @@ var _ = Describe("In Command", func() { It("does not create the tag and version files", func() { Ω(path.Join(destDir, "tag")).ShouldNot(BeAnExistingFile()) Ω(path.Join(destDir, "version")).ShouldNot(BeAnExistingFile()) + Ω(path.Join(destDir, "commit_sha")).ShouldNot(BeAnExistingFile()) }) }) From 9968f6d17f165dd6f243adf49e487a476280e34a Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Thu, 12 Oct 2017 17:30:18 +0200 Subject: [PATCH 2/9] Add implementation, rename to target_commit --- in_command.go | 26 +++++++++++++++++++++++++- in_command_test.go | 10 +++++----- metadata.go | 9 ++++++++- out_command.go | 2 +- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/in_command.go b/in_command.go index 2e19dfe..ec5bb59 100644 --- a/in_command.go +++ b/in_command.go @@ -32,6 +32,7 @@ func (c *InCommand) Run(destDir string, request InRequest) (InResponse, error) { } var foundRelease *github.RepositoryRelease + var commitSHA string if request.Version.Tag != "" { foundRelease, err = c.github.GetReleaseByTag(request.Version.Tag) @@ -61,6 +62,13 @@ func (c *InCommand) Run(destDir string, request InRequest) (InResponse, error) { return InResponse{}, err } + commitPath := filepath.Join(destDir, "target_commit") + commitSHA = c.resolveTagToCommitSHA(*foundRelease.TagName) + err = ioutil.WriteFile(commitPath, []byte(commitSHA), 0644) + if err != nil { + return InResponse{}, err + } + if foundRelease.Body != nil && *foundRelease.Body != "" { body := *foundRelease.Body bodyPath := filepath.Join(destDir, "body") @@ -133,7 +141,7 @@ func (c *InCommand) Run(destDir string, request InRequest) (InResponse, error) { return InResponse{ Version: versionFromRelease(foundRelease), - Metadata: metadataFromRelease(foundRelease), + Metadata: metadataFromRelease(foundRelease, commitSHA), }, nil } @@ -182,3 +190,19 @@ func (c *InCommand) downloadFile(url, destPath string) error { return nil } + +func (c *InCommand) resolveTagToCommitSHA(tag string) string { + reference, err := c.github.GetRef(tag) + + if err != nil { + fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: %v", tag, err) + return "" + } + + if *reference.Object.Type != "commit" { + fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: returned type is not 'commit' - only lightweight tags are supported", tag) + return "" + } + + return *reference.Object.SHA +} diff --git a/in_command_test.go b/in_command_test.go index e1b0760..6e638e3 100644 --- a/in_command_test.go +++ b/in_command_test.go @@ -142,7 +142,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -171,7 +171,7 @@ var _ = Describe("In Command", func() { Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("0.35.0")) - contents, err = ioutil.ReadFile(path.Join(destDir, "commit_sha")) + contents, err = ioutil.ReadFile(path.Join(destDir, "target_commit")) Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("f28085a4a8f744da83411f5e09fd7b1709149eee")) @@ -361,7 +361,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -455,7 +455,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, resource.MetadataPair{Name: "draft", Value: "true"}, )) }) @@ -500,7 +500,7 @@ var _ = Describe("In Command", func() { It("does not create the tag and version files", func() { Ω(path.Join(destDir, "tag")).ShouldNot(BeAnExistingFile()) Ω(path.Join(destDir, "version")).ShouldNot(BeAnExistingFile()) - Ω(path.Join(destDir, "commit_sha")).ShouldNot(BeAnExistingFile()) + Ω(path.Join(destDir, "target_commit")).ShouldNot(BeAnExistingFile()) }) }) diff --git a/metadata.go b/metadata.go index 1f8fb2f..de0ee5e 100644 --- a/metadata.go +++ b/metadata.go @@ -2,7 +2,7 @@ package resource import "github.com/google/go-github/github" -func metadataFromRelease(release *github.RepositoryRelease) []MetadataPair { +func metadataFromRelease(release *github.RepositoryRelease, commitSHA string) []MetadataPair { metadata := []MetadataPair{} if release.Name != nil { @@ -40,6 +40,13 @@ func metadataFromRelease(release *github.RepositoryRelease) []MetadataPair { }) } + if commitSHA != "" { + metadata = append(metadata, MetadataPair{ + Name: "target_commit", + Value: commitSHA, + }) + } + if *release.Draft { metadata = append(metadata, MetadataPair{ Name: "draft", diff --git a/out_command.go b/out_command.go index 5d877d6..c5fb5eb 100644 --- a/out_command.go +++ b/out_command.go @@ -145,7 +145,7 @@ func (c *OutCommand) Run(sourceDir string, request OutRequest) (OutResponse, err return OutResponse{ Version: versionFromRelease(release), - Metadata: metadataFromRelease(release), + Metadata: metadataFromRelease(release, ""), }, nil } From 4da19b53b471be90dbd53e55fa3d294edbbecfdf Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Thu, 12 Oct 2017 17:35:55 +0200 Subject: [PATCH 3/9] Update README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 0ef6418..e6ab410 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,7 @@ Also creates the following files: * `tag` containing the git tag name of the release being fetched. * `version` containing the version determined by the git tag of the release being fetched. * `body` containing the body text of the release. +* `target_commit` containing the commit SHA the tag is pointing to. #### Parameters From 7bff836cc2e7296fe0adefd2045a130da590a16f Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Sun, 14 Jan 2018 10:19:47 +0100 Subject: [PATCH 4/9] Rename generated file back to commit_sha --- README.md | 2 +- in_command.go | 2 +- in_command_test.go | 10 +++++----- metadata.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index e6ab410..beb1d09 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ Also creates the following files: * `tag` containing the git tag name of the release being fetched. * `version` containing the version determined by the git tag of the release being fetched. * `body` containing the body text of the release. -* `target_commit` containing the commit SHA the tag is pointing to. +* `commit_sha` containing the commit SHA the tag is pointing to. #### Parameters diff --git a/in_command.go b/in_command.go index ec5bb59..f5f9a3d 100644 --- a/in_command.go +++ b/in_command.go @@ -62,7 +62,7 @@ func (c *InCommand) Run(destDir string, request InRequest) (InResponse, error) { return InResponse{}, err } - commitPath := filepath.Join(destDir, "target_commit") + commitPath := filepath.Join(destDir, "commit_sha") commitSHA = c.resolveTagToCommitSHA(*foundRelease.TagName) err = ioutil.WriteFile(commitPath, []byte(commitSHA), 0644) if err != nil { diff --git a/in_command_test.go b/in_command_test.go index 6e638e3..e1b0760 100644 --- a/in_command_test.go +++ b/in_command_test.go @@ -142,7 +142,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -171,7 +171,7 @@ var _ = Describe("In Command", func() { Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("0.35.0")) - contents, err = ioutil.ReadFile(path.Join(destDir, "target_commit")) + contents, err = ioutil.ReadFile(path.Join(destDir, "commit_sha")) Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("f28085a4a8f744da83411f5e09fd7b1709149eee")) @@ -361,7 +361,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, )) }) @@ -455,7 +455,7 @@ var _ = Describe("In Command", func() { resource.MetadataPair{Name: "name", Value: "release-name", URL: "http://google.com"}, resource.MetadataPair{Name: "body", Value: "*markdown*", Markdown: true}, resource.MetadataPair{Name: "tag", Value: "v0.35.0"}, - resource.MetadataPair{Name: "target_commit", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, + resource.MetadataPair{Name: "commit_sha", Value: "f28085a4a8f744da83411f5e09fd7b1709149eee"}, resource.MetadataPair{Name: "draft", Value: "true"}, )) }) @@ -500,7 +500,7 @@ var _ = Describe("In Command", func() { It("does not create the tag and version files", func() { Ω(path.Join(destDir, "tag")).ShouldNot(BeAnExistingFile()) Ω(path.Join(destDir, "version")).ShouldNot(BeAnExistingFile()) - Ω(path.Join(destDir, "target_commit")).ShouldNot(BeAnExistingFile()) + Ω(path.Join(destDir, "commit_sha")).ShouldNot(BeAnExistingFile()) }) }) diff --git a/metadata.go b/metadata.go index de0ee5e..54fb51c 100644 --- a/metadata.go +++ b/metadata.go @@ -42,7 +42,7 @@ func metadataFromRelease(release *github.RepositoryRelease, commitSHA string) [] if commitSHA != "" { metadata = append(metadata, MetadataPair{ - Name: "target_commit", + Name: "commit_sha", Value: commitSHA, }) } From dae25957fc39ff4c3514ede1d9d3454a2e882df0 Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Sun, 21 Jan 2018 10:46:41 +0100 Subject: [PATCH 5/9] Add github client happy path specs --- github_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/github_test.go b/github_test.go index 18b10d0..08902c7 100644 --- a/github_test.go +++ b/github_test.go @@ -161,6 +161,7 @@ var _ = Describe("GitHub Client", func() { Repository: "concourse", } }) + Context("When GitHub's rate limit has been exceeded", func() { BeforeEach(func() { rateLimitResponse := `{ @@ -188,6 +189,22 @@ var _ = Describe("GitHub Client", func() { Expect(err.Error()).To(ContainSubstring("API rate limit exceeded for 127.0.0.1. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)")) }) }) + + Context("When GitHub responds successfully", func() { + BeforeEach(func() { + server.AppendHandlers( + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases/tags/some-tag"), + ghttp.RespondWith(200, "{}"), + ), + ) + }) + + It("Returns without error", func() { + _, err := client.GetReleaseByTag("some-tag") + Ω(err).ShouldNot(HaveOccurred()) + }) + }) }) Describe("GetRef", func() { @@ -197,6 +214,7 @@ var _ = Describe("GitHub Client", func() { Repository: "concourse", } }) + Context("When GitHub's rate limit has been exceeded", func() { BeforeEach(func() { rateLimitResponse := `{ @@ -224,5 +242,21 @@ var _ = Describe("GitHub Client", func() { Expect(err.Error()).To(ContainSubstring("API rate limit exceeded for 127.0.0.1. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)")) }) }) + + Context("When GitHub responds successfully", func() { + BeforeEach(func() { + server.AppendHandlers( + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/repos/concourse/concourse/git/refs/tags/some-tag"), + ghttp.RespondWith(200, "{}"), + ), + ) + }) + + It("Returns without error", func() { + _, err := client.GetRef("some-tag") + Ω(err).ShouldNot(HaveOccurred()) + }) + }) }) }) From 5819c61aa633a58527d59ed714f963c47be0e1ed Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Sun, 21 Jan 2018 11:03:06 +0100 Subject: [PATCH 6/9] Pass on github client error, do not create empty commit_sha file --- in_command.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/in_command.go b/in_command.go index f5f9a3d..634ec82 100644 --- a/in_command.go +++ b/in_command.go @@ -63,12 +63,18 @@ func (c *InCommand) Run(destDir string, request InRequest) (InResponse, error) { } commitPath := filepath.Join(destDir, "commit_sha") - commitSHA = c.resolveTagToCommitSHA(*foundRelease.TagName) - err = ioutil.WriteFile(commitPath, []byte(commitSHA), 0644) + commitSHA, err = c.resolveTagToCommitSHA(*foundRelease.TagName) if err != nil { return InResponse{}, err } + if commitSHA != "" { + err = ioutil.WriteFile(commitPath, []byte(commitSHA), 0644) + if err != nil { + return InResponse{}, err + } + } + if foundRelease.Body != nil && *foundRelease.Body != "" { body := *foundRelease.Body bodyPath := filepath.Join(destDir, "body") @@ -191,18 +197,13 @@ func (c *InCommand) downloadFile(url, destPath string) error { return nil } -func (c *InCommand) resolveTagToCommitSHA(tag string) string { +func (c *InCommand) resolveTagToCommitSHA(tag string) (string, error) { reference, err := c.github.GetRef(tag) - if err != nil { - fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: %v", tag, err) - return "" + if err != nil && *reference.Object.Type != "commit" { + fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: returned type is not 'commit' - only lightweight tags are supported") + return "", nil } - if *reference.Object.Type != "commit" { - fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: returned type is not 'commit' - only lightweight tags are supported", tag) - return "" - } - - return *reference.Object.SHA + return *reference.Object.SHA, err } From d9e71688fcd1f49479b054ae476ac1cd79a1d09b Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Wed, 24 Jan 2018 09:03:54 +0100 Subject: [PATCH 7/9] Return on error to avoid dereference errors --- in_command.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/in_command.go b/in_command.go index 634ec82..f302fe7 100644 --- a/in_command.go +++ b/in_command.go @@ -199,10 +199,13 @@ func (c *InCommand) downloadFile(url, destPath string) error { func (c *InCommand) resolveTagToCommitSHA(tag string) (string, error) { reference, err := c.github.GetRef(tag) + if err != nil { + return "", err + } - if err != nil && *reference.Object.Type != "commit" { + if *reference.Object.Type != "commit" { fmt.Fprintln(c.writer, "could not resolve tag '%s' to commit: returned type is not 'commit' - only lightweight tags are supported") - return "", nil + return "", err } return *reference.Object.SHA, err From a4894b099226e20f4cd10ba85f7bb4d4fbcdc9bd Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Wed, 24 Jan 2018 09:04:03 +0100 Subject: [PATCH 8/9] Add missing file specs --- in_command_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/in_command_test.go b/in_command_test.go index e1b0760..f1f1108 100644 --- a/in_command_test.go +++ b/in_command_test.go @@ -468,6 +468,10 @@ var _ = Describe("In Command", func() { contents, err = ioutil.ReadFile(path.Join(destDir, "version")) Ω(err).ShouldNot(HaveOccurred()) Ω(string(contents)).Should(Equal("0.35.0")) + + contents, err = ioutil.ReadFile(path.Join(destDir, "commit_sha")) + Ω(err).ShouldNot(HaveOccurred()) + Ω(string(contents)).Should(Equal("f28085a4a8f744da83411f5e09fd7b1709149eee")) }) }) @@ -532,6 +536,7 @@ var _ = Describe("In Command", func() { It("does not create the tag and version files", func() { Ω(path.Join(destDir, "tag")).ShouldNot(BeAnExistingFile()) Ω(path.Join(destDir, "version")).ShouldNot(BeAnExistingFile()) + Ω(path.Join(destDir, "commit_sha")).ShouldNot(BeAnExistingFile()) }) }) }) From 4bef95a488f7017815a13b8645f99ea867e838e5 Mon Sep 17 00:00:00 2001 From: Christoph Sassenberg Date: Wed, 24 Jan 2018 09:45:57 +0100 Subject: [PATCH 9/9] Add assertion for return values of GetRef and GetReleaseByTag --- github_test.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/github_test.go b/github_test.go index 08902c7..4254e21 100644 --- a/github_test.go +++ b/github_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/ghttp" + "github.com/google/go-github/github" ) var _ = Describe("GitHub Client", func() { @@ -195,14 +196,20 @@ var _ = Describe("GitHub Client", func() { server.AppendHandlers( ghttp.CombineHandlers( ghttp.VerifyRequest("GET", "/repos/concourse/concourse/releases/tags/some-tag"), - ghttp.RespondWith(200, "{}"), + ghttp.RespondWith(200, `{ "id": 1 }`), ), ) }) - It("Returns without error", func() { - _, err := client.GetReleaseByTag("some-tag") + It("Returns a populated github.RepositoryRelease", func() { + expectedRelease := &github.RepositoryRelease{ + ID: github.Int(1), + } + + release, err := client.GetReleaseByTag("some-tag") + Ω(err).ShouldNot(HaveOccurred()) + Expect(release).To(Equal(expectedRelease)) }) }) }) @@ -248,14 +255,20 @@ var _ = Describe("GitHub Client", func() { server.AppendHandlers( ghttp.CombineHandlers( ghttp.VerifyRequest("GET", "/repos/concourse/concourse/git/refs/tags/some-tag"), - ghttp.RespondWith(200, "{}"), + ghttp.RespondWith(200, `{ "ref": "refs/tags/some-tag" }`), ), ) }) - It("Returns without error", func() { - _, err := client.GetRef("some-tag") + It("Returns a populated github.Reference", func() { + expectedReference := &github.Reference{ + Ref: github.String("refs/tags/some-tag"), + } + + reference, err := client.GetRef("some-tag") + Ω(err).ShouldNot(HaveOccurred()) + Expect(reference).To(Equal(expectedReference)) }) }) })