From 6c6c7c3c43689846df0da17b8b6d1b090c7ee63e Mon Sep 17 00:00:00 2001 From: Chris Rohr Date: Mon, 28 Sep 2015 22:01:52 -0400 Subject: [PATCH 1/2] Addresses regression with jenkins setup that does not use the multiproject setup --- .../project_services/jenkins_service.rb | 17 +++++- .../project_services/jenkins_service_spec.rb | 59 +++++++++++++++---- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/app/models/project_services/jenkins_service.rb b/app/models/project_services/jenkins_service.rb index e1ae9ff80d..4984a1d2c2 100644 --- a/app/models/project_services/jenkins_service.rb +++ b/app/models/project_services/jenkins_service.rb @@ -14,6 +14,7 @@ class JenkinsService < CiService prop_accessor :project_url + prop_accessor :multiproject_enabled validates :project_url, presence: true, if: :activated? @@ -46,13 +47,23 @@ class JenkinsService < CiService def fields [ - { type: 'text', name: 'project_url', placeholder: 'Jenkins project URL like http://jenkins.example.com/job/my-project/' } + { type: 'text', name: 'project_url', placeholder: 'Jenkins project URL like http://jenkins.example.com/job/my-project/' }, + { type: 'checkbox', name: 'multiproject_enabled', title: "Multi-project setup enabled?", + help: "Multi-project mode is configured in Jenkins Gitlab Hook plugin." } ] end + def multiproject_enabled? + self.multiproject_enabled == '1' + end + def build_page(sha, ref = nil) - base_url = ref.nil? || ref == 'master' ? project_url : "#{project_url}_#{ref}" - base_url + "/scm/bySHA1/#{sha}" + if multiproject_enabled? + base_url = ref.nil? || ref == 'master' ? project_url : "#{project_url}_#{ref}" + base_url + "/scm/bySHA1/#{sha}" + else + project_url + "/scm/bySHA1/#{sha}" + end end def commit_status(sha, ref = nil) diff --git a/spec/models/project_services/jenkins_service_spec.rb b/spec/models/project_services/jenkins_service_spec.rb index 8bb081ca72..bf2bcdb94c 100644 --- a/spec/models/project_services/jenkins_service_spec.rb +++ b/spec/models/project_services/jenkins_service_spec.rb @@ -35,16 +35,17 @@ describe JenkinsService do eos end - before do - @service = JenkinsService.new - allow(@service).to receive_messages( - service_hook: true, - project_url: 'http://jenkins.gitlab.org/projects/2', - token: 'verySecret' - ) - end - describe :commit_status do + before do + @service = JenkinsService.new + allow(@service).to receive_messages( + service_hook: true, + project_url: 'http://jenkins.gitlab.org/projects/2', + multiproject_enabled: '1', + token: 'verySecret' + ) + end + statuses = { 'blue.png' => 'success', 'yellow.png' => 'failed', 'red.png' => 'failed', 'aborted.png' => 'failed', 'blue-anime.gif' => 'running', 'grey.png' => 'pending' } statuses.each do |icon, state| it "should have a status of #{state} when the icon #{icon} exists." do @@ -54,12 +55,44 @@ eos end end - describe :build_page do - it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c") } + describe 'multiproject enabled' do + before do + @service = JenkinsService.new + allow(@service).to receive_messages( + service_hook: true, + project_url: 'http://jenkins.gitlab.org/projects/2', + multiproject_enabled: '1', + token: 'verySecret' + ) + end + + describe :build_page do + it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c") } + end + + describe :build_page_with_branch do + it { expect(@service.build_page("2ab7834c", 'test_branch')).to eq("http://jenkins.gitlab.org/projects/2_test_branch/scm/bySHA1/2ab7834c") } + end end - describe :build_page_with_branch do - it { expect(@service.build_page("2ab7834c", 'test_branch')).to eq("http://jenkins.gitlab.org/projects/2_test_branch/scm/bySHA1/2ab7834c") } + describe 'multiproject disabled' do + before do + @service = JenkinsService.new + allow(@service).to receive_messages( + service_hook: true, + project_url: 'http://jenkins.gitlab.org/projects/2', + multiproject_enabled: '0', + token: 'verySecret' + ) + end + + describe :build_page do + it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c") } + end + + describe :build_page_with_branch do + it { expect(@service.build_page("2ab7834c", 'test_branch')).to eq("http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c") } + end end end end From f80ca51b3b7bb890d8b56d02f37c8d29d04ba459 Mon Sep 17 00:00:00 2001 From: Chris Rohr Date: Thu, 1 Oct 2015 07:01:01 -0400 Subject: [PATCH 2/2] Cleaned up conditional syntax --- .../project_services/jenkins_service.rb | 18 ++++++++++----- .../project_services/jenkins_service_spec.rb | 23 +++++++++++++++++-- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/app/models/project_services/jenkins_service.rb b/app/models/project_services/jenkins_service.rb index 4984a1d2c2..2678cb783e 100644 --- a/app/models/project_services/jenkins_service.rb +++ b/app/models/project_services/jenkins_service.rb @@ -15,6 +15,7 @@ class JenkinsService < CiService prop_accessor :project_url prop_accessor :multiproject_enabled + prop_accessor :pass_unstable validates :project_url, presence: true, if: :activated? @@ -49,7 +50,9 @@ class JenkinsService < CiService [ { type: 'text', name: 'project_url', placeholder: 'Jenkins project URL like http://jenkins.example.com/job/my-project/' }, { type: 'checkbox', name: 'multiproject_enabled', title: "Multi-project setup enabled?", - help: "Multi-project mode is configured in Jenkins Gitlab Hook plugin." } + help: "Multi-project mode is configured in Jenkins Gitlab Hook plugin." }, + { type: 'checkbox', name: 'pass_unstable', title: 'Should unstable builds be treated as passing?', + help: 'Unstable builds will be treated as passing.'} ] end @@ -57,12 +60,15 @@ class JenkinsService < CiService self.multiproject_enabled == '1' end + def pass_unstable? + self.pass_unstable == '1' + end + def build_page(sha, ref = nil) - if multiproject_enabled? - base_url = ref.nil? || ref == 'master' ? project_url : "#{project_url}_#{ref}" - base_url + "/scm/bySHA1/#{sha}" + if multiproject_enabled? && ref.present? + "#{project_url}_#{ref}/scm/bySHA1/#{sha}" else - project_url + "/scm/bySHA1/#{sha}" + "#{project_url}/scm/bySHA1/#{sha}" end end @@ -83,7 +89,7 @@ class JenkinsService < CiService if response.code == 200 # img.build-caption-status-icon for old jenkins version src = Nokogiri.parse(response).css('img.build-caption-status-icon,.build-caption>img').first.attributes['src'].value - if src =~ /blue\.png$/ + if src =~ /blue\.png$/ || (src =~ /yellow\.png/ && pass_unstable?) 'success' elsif src =~ /(red|aborted|yellow)\.png$/ 'failed' diff --git a/spec/models/project_services/jenkins_service_spec.rb b/spec/models/project_services/jenkins_service_spec.rb index bf2bcdb94c..a67945a609 100644 --- a/spec/models/project_services/jenkins_service_spec.rb +++ b/spec/models/project_services/jenkins_service_spec.rb @@ -41,7 +41,8 @@ eos allow(@service).to receive_messages( service_hook: true, project_url: 'http://jenkins.gitlab.org/projects/2', - multiproject_enabled: '1', + multiproject_enabled: '0', + pass_unstable: '0', token: 'verySecret' ) end @@ -55,6 +56,24 @@ eos end end + describe 'commit status with passing unstable' do + before do + @service = JenkinsService.new + allow(@service).to receive_messages( + service_hook: true, + project_url: 'http://jenkins.gitlab.org/projects/2', + multiproject_enabled: '0', + pass_unstable: '1', + token: 'verySecret' + ) + end + + it "should have a status of success when the icon yellow exists." do + stub_request(:get, "http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c").to_return(status: 200, body: status_body_for_icon('yellow.png'), headers: {}) + expect(@service.commit_status("2ab7834c", 'master')).to eq('success') + end + end + describe 'multiproject enabled' do before do @service = JenkinsService.new @@ -67,7 +86,7 @@ eos end describe :build_page do - it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://jenkins.gitlab.org/projects/2/scm/bySHA1/2ab7834c") } + it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://jenkins.gitlab.org/projects/2_master/scm/bySHA1/2ab7834c") } end describe :build_page_with_branch do