From 423b2a102fb4473cf4f8f8e484997afc79861ffd Mon Sep 17 00:00:00 2001 From: Shane Hender Date: Tue, 1 Dec 2015 14:33:31 +0000 Subject: [PATCH 1/2] Use the Github API to check if last release already contains a new commit. --- app/models/project.rb | 3 ++- .../integrations/base_controller_test.rb | 1 + test/models/project_test.rb | 14 +++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 0f4c59daf0..6ba66a368d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -66,7 +66,8 @@ def docker_repo def last_release_contains_commit?(commit) last_release = releases.order(:id).last - last_release && repository.downstream_commit?(last_release.commit, commit) + # status values documented here: http://stackoverflow.com/questions/23943855/github-api-to-compare-commits-response-status-is-diverged + last_release && %w(behind identical).include?(GITHUB.compare(github_repo, last_release.commit, commit).status) end def auto_release_stages diff --git a/test/controllers/integrations/base_controller_test.rb b/test/controllers/integrations/base_controller_test.rb index f2007ec2f7..90eecfbea8 100644 --- a/test/controllers/integrations/base_controller_test.rb +++ b/test/controllers/integrations/base_controller_test.rb @@ -36,6 +36,7 @@ end it 're-uses last release if commit already present' do + stub_github_api("repos/bar/foo/compare/#{sha}...#{sha}", { status: 'identical' }) base_controller.expects(:head).with(:ok).twice base_controller.create diff --git a/test/models/project_test.rb b/test/models/project_test.rb index d8691c6583..7900e71777 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -21,14 +21,26 @@ def clone_repository(project) before do project.stubs(:repository).returns(repository) - repository.stubs(:downstream_commit?).with('LAST', 'NEW').returns(true) end it "returns true if the last release contains that commit" do + stub_github_api('repos/bar/foo/compare/LAST...NEW', { status: 'behind' }) project.releases.create!(commit: "LAST", author: author) assert project.last_release_contains_commit?("NEW") end + it "returns false if last release does not contain commit" do + stub_github_api('repos/bar/foo/compare/LAST...NEW', { status: 'ahead' }) + project.releases.create!(commit: "LAST", author: author) + refute project.last_release_contains_commit?("NEW") + end + + it "returns true if last release has the same commit" do + stub_github_api('repos/bar/foo/compare/LAST...LAST', { status: 'identical' }) + project.releases.create!(commit: "LAST", author: author) + assert project.last_release_contains_commit?("LAST") + end + it "returns false if there have been no releases" do project.releases.destroy_all refute project.last_release_contains_commit?("NEW") From cfeaac6f935b52d66ec59e839a9a8c71570d5b20 Mon Sep 17 00:00:00 2001 From: Shane Hender Date: Wed, 2 Dec 2015 10:39:36 +0000 Subject: [PATCH 2/2] Add error handling --- app/models/project.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 6ba66a368d..42a523b8ca 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -68,6 +68,9 @@ def last_release_contains_commit?(commit) last_release = releases.order(:id).last # status values documented here: http://stackoverflow.com/questions/23943855/github-api-to-compare-commits-response-status-is-diverged last_release && %w(behind identical).include?(GITHUB.compare(github_repo, last_release.commit, commit).status) + rescue Octokit::Error => e + Airbrake.notify(e, parameters: { github_repo: github_repo, last_commit: last_release.commit, commit: commit }) + false # Err on side of caution and cause a new release to be created. end def auto_release_stages