diff options
| -rw-r--r-- | CHANGELOG | 4 | ||||
| -rw-r--r-- | app/models/project_services/teamcity_service.rb | 6 | ||||
| -rw-r--r-- | spec/models/project_services/teamcity_service_spec.rb | 251 |
3 files changed, 203 insertions, 58 deletions
diff --git a/CHANGELOG b/CHANGELOG index c6ca0ea3de1..df40a1e1b81 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,6 +13,7 @@ v 8.7.0 (unreleased) - Expose label description in API (Mariusz Jachimowicz) - Allow back dating on issues when created through the API - Fix Error 500 after renaming a project path (Stan Hu) + - Fix a bug whith trailing slash in teamcity_url (Charles May) - Fix avatar stretching by providing a cropping feature - API: Expose `subscribed` for issues and merge requests (Robert Schilling) - Allow SAML to handle external users based on user's information !3530 @@ -123,9 +124,6 @@ v 8.6.0 - Add information about `image` and `services` field at `job` level in the `.gitlab-ci.yml` documentation (Pat Turner) - HTTP error pages work independently from location and config (Artem Sidorenko) - Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set - - Fix avatar stretching by providing a cropping feature (Johann Pardanaud) - - Fix a bug whith trailing slash in teamcity_url (Charles May) - - Don't load all of GitLab in mail_room - Memoize @group in Admin::GroupsController (Yatish Mehta) - Indicate how much an MR diverged from the target branch (Pierre de La Morinerie) - Added omniauth-auth0 Gem (Daniel Carraro) diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index 246c5eb4a82..8dceee5e2c5 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -91,7 +91,7 @@ class TeamcityService < CiService ).to_s auth = { username: username, - password: password, + password: password } @response = HTTParty.get(url, verify: false, basic_auth: auth) end @@ -108,7 +108,7 @@ class TeamcityService < CiService built_id = @response['build']['id'] URI.join( teamcity_url, - "#{teamcity_url}/viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}" + "/viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}" ).to_s end end @@ -145,7 +145,7 @@ class TeamcityService < CiService branch = Gitlab::Git.ref_name(data[:ref]) self.class.post( - URI.join(teamcity_url, "/httpAuth/app/rest/buildQueue").to_s, + URI.join(teamcity_url, '/httpAuth/app/rest/buildQueue').to_s, body: "<build branchName=\"#{branch}\">"\ "<buildType id=\"#{build_type}\"/>"\ '</build>', diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index f26b47a856c..bc7423cee69 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -21,73 +21,220 @@ require 'spec_helper' describe TeamcityService, models: true do - describe "Associations" do + describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } end - describe "Execute" do - let(:user) { create(:user) } - let(:project) { create(:project) } - - context "when a password was previously set" do - before do - @teamcity_service = TeamcityService.create( - project: create(:project), - properties: { - teamcity_url: 'http://gitlab.com', - username: 'mic', - password: "password" - } - ) + describe 'Validations' do + describe '#teamcity_url' do + it 'does not validate the presence of teamcity_url if service is not active' do + teamcity_service = service + teamcity_service.active = false + + expect(teamcity_service).not_to validate_presence_of(:teamcity_url) end - - it "reset password if url changed" do - @teamcity_service.teamcity_url = 'http://gitlab1.com' - @teamcity_service.save - expect(@teamcity_service.password).to be_nil + + it 'validates the presence of teamcity_url if service is active' do + teamcity_service = service + teamcity_service.active = true + + expect(teamcity_service).to validate_presence_of(:teamcity_url) + end + end + + describe '#build_type' do + it 'does not validate the presence of build_type if service is not active' do + teamcity_service = service + teamcity_service.active = false + + expect(teamcity_service).not_to validate_presence_of(:build_type) + end + + it 'validates the presence of build_type if service is active' do + teamcity_service = service + teamcity_service.active = true + + expect(teamcity_service).to validate_presence_of(:build_type) end - - it "does not reset password if username changed" do - @teamcity_service.username = "some_name" - @teamcity_service.save - expect(@teamcity_service.password).to eq("password") + end + + describe '#username' do + it 'does not validate the presence of username if service is not active' do + teamcity_service = service + teamcity_service.active = false + + expect(teamcity_service).not_to validate_presence_of(:username) end - it "does not reset password if new url is set together with password, even if it's the same password" do - @teamcity_service.teamcity_url = 'http://gitlab_edited.com' - @teamcity_service.password = 'password' - @teamcity_service.save - expect(@teamcity_service.password).to eq("password") - expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") + it 'does not validate the presence of username if username is nil' do + teamcity_service = service + teamcity_service.active = true + teamcity_service.password = nil + + expect(teamcity_service).not_to validate_presence_of(:username) end - it "should reset password if url changed, even if setter called multiple times" do - @teamcity_service.teamcity_url = 'http://gitlab1.com' - @teamcity_service.teamcity_url = 'http://gitlab1.com' - @teamcity_service.save - expect(@teamcity_service.password).to be_nil + it 'validates the presence of username if service is active and username is present' do + teamcity_service = service + teamcity_service.active = true + teamcity_service.password = 'secret' + + expect(teamcity_service).to validate_presence_of(:username) end end - - context "when no password was previously set" do - before do - @teamcity_service = TeamcityService.create( - project: create(:project), - properties: { - teamcity_url: 'http://gitlab.com', - username: 'mic' - } - ) + + describe '#password' do + it 'does not validate the presence of password if service is not active' do + teamcity_service = service + teamcity_service.active = false + + expect(teamcity_service).not_to validate_presence_of(:password) + end + + it 'does not validate the presence of password if username is nil' do + teamcity_service = service + teamcity_service.active = true + teamcity_service.username = nil + + expect(teamcity_service).not_to validate_presence_of(:password) end - it "saves password if new url is set together with password" do - @teamcity_service.teamcity_url = 'http://gitlab_edited.com' - @teamcity_service.password = 'password' - @teamcity_service.save - expect(@teamcity_service.password).to eq("password") - expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") + it 'validates the presence of password if service is active and username is present' do + teamcity_service = service + teamcity_service.active = true + teamcity_service.username = 'john' + + expect(teamcity_service).to validate_presence_of(:password) end end end + + describe 'Callbacks' do + describe 'before_update :reset_password' do + context 'when a password was previously set' do + it 'resets password if url changed' do + teamcity_service = service + + teamcity_service.teamcity_url = 'http://gitlab1.com' + teamcity_service.save + + expect(teamcity_service.password).to be_nil + end + + it 'does not reset password if username changed' do + teamcity_service = service + + teamcity_service.username = 'some_name' + teamcity_service.save + + expect(teamcity_service.password).to eq('password') + end + + it "does not reset password if new url is set together with password, even if it's the same password" do + teamcity_service = service + + teamcity_service.teamcity_url = 'http://gitlab_edited.com' + teamcity_service.password = 'password' + teamcity_service.save + + expect(teamcity_service.password).to eq('password') + expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com') + end + end + + it 'saves password if new url is set together with password when no password was previously set' do + teamcity_service = service + teamcity_service.password = nil + + teamcity_service.teamcity_url = 'http://gitlab_edited.com' + teamcity_service.password = 'password' + teamcity_service.save + + expect(teamcity_service.password).to eq('password') + expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com') + end + end + end + + describe '#build_page' do + it 'returns a specific URL when status is 500' do + stub_request(status: 500) + + expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo') + end + + it 'returns a build URL when teamcity_url has no trailing slash' do + stub_request(body: %Q({"build":{"id":"666"}})) + + expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo') + end + + it 'returns a build URL when teamcity_url has a trailing slash' do + stub_request(body: %Q({"build":{"id":"666"}})) + + expect(service(teamcity_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo') + end + end + + describe '#commit_status' do + it 'sets commit status to :error when status is 500' do + stub_request(status: 500) + + expect(service.commit_status('123', 'unused')).to eq(:error) + end + + it 'sets commit status to "pending" when status is 404' do + stub_request(status: 404) + + expect(service.commit_status('123', 'unused')).to eq('pending') + end + + it 'sets commit status to "success" when build status contains SUCCESS' do + stub_request(build_status: 'YAY SUCCESS!') + + expect(service.commit_status('123', 'unused')).to eq('success') + end + + it 'sets commit status to "failed" when build status contains FAILURE' do + stub_request(build_status: 'NO FAILURE!') + + expect(service.commit_status('123', 'unused')).to eq('failed') + end + + it 'sets commit status to "pending" when build status contains Pending' do + stub_request(build_status: 'NO Pending!') + + expect(service.commit_status('123', 'unused')).to eq('pending') + end + + it 'sets commit status to :error when build status is unknown' do + stub_request(build_status: 'FOO BAR!') + + expect(service.commit_status('123', 'unused')).to eq(:error) + end + end + + def service(teamcity_url: 'http://gitlab.com') + described_class.create( + project: build_stubbed(:empty_project), + properties: { + teamcity_url: teamcity_url, + username: 'mic', + password: 'password', + build_type: 'foo' + } + ) + end + + def stub_request(status: 200, body: nil, build_status: 'success') + teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123' + body ||= %Q({"build":{"status":"#{build_status}","id":"666"}}) + + WebMock.stub_request(:get, teamcity_full_url).to_return( + status: status, + headers: { 'Content-Type' => 'application/json' }, + body: body + ) + end end |