Merge remote-tracking branch 'dev/master' into 'master'

19 files changed, 443 insertions, 209 deletions

diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index 31b2c90122d..e771f35811e 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -27,86 +27,51 @@ describe BambooService, models: true do
   end
 
   describe 'Validations' do
-    describe '#bamboo_url' do
-      it 'does not validate the presence of bamboo_url if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
-
-        expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
-      end
-
-      it 'validates the presence of bamboo_url if service is active' do
-        bamboo_service = service
-        bamboo_service.active = true
-
-        expect(bamboo_service).to validate_presence_of(:bamboo_url)
-      end
-    end
+    subject { service }
 
-    describe '#build_key' do
-      it 'does not validate the presence of build_key if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
+    context 'when service is active' do
+      before { subject.active = true }
 
-        expect(bamboo_service).not_to validate_presence_of(:build_key)
-      end
+      it { is_expected.to validate_presence_of(:build_key) }
+      it { is_expected.to validate_presence_of(:bamboo_url) }
+      it_behaves_like 'issue tracker service URL attribute', :bamboo_url
 
-      it 'validates the presence of build_key if service is active' do
-        bamboo_service = service
-        bamboo_service.active = true
+      describe '#username' do
+        it 'does not validate the presence of username if password is nil' do
+          subject.password = nil
 
-        expect(bamboo_service).to validate_presence_of(:build_key)
-      end
-    end
+          expect(subject).not_to validate_presence_of(:username)
+        end
 
-    describe '#username' do
-      it 'does not validate the presence of username if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
+        it 'validates the presence of username if password is present' do
+          subject.password = 'secret'
 
-        expect(bamboo_service).not_to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:username)
+        end
       end
 
-      it 'does not validate the presence of username if username is nil' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.password = nil
+      describe '#password' do
+        it 'does not validate the presence of password if username is nil' do
+          subject.username = nil
 
-        expect(bamboo_service).not_to validate_presence_of(:username)
-      end
+          expect(subject).not_to validate_presence_of(:password)
+        end
 
-      it 'validates the presence of username if service is active and username is present' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.password = 'secret'
+        it 'validates the presence of password if username is present' do
+          subject.username = 'john'
 
-        expect(bamboo_service).to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:password)
+        end
       end
     end
 
-    describe '#password' do
-      it 'does not validate the presence of password if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
-
-        expect(bamboo_service).not_to validate_presence_of(:password)
-      end
-
-      it 'does not validate the presence of password if username is nil' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.username = nil
-
-        expect(bamboo_service).not_to validate_presence_of(:password)
-      end
-
-      it 'validates the presence of password if service is active and username is present' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.username = 'john'
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-        expect(bamboo_service).to validate_presence_of(:password)
-      end
+      it { is_expected.not_to validate_presence_of(:build_key) }
+      it { is_expected.not_to validate_presence_of(:bamboo_url) }
+      it { is_expected.not_to validate_presence_of(:username) }
+      it { is_expected.not_to validate_presence_of(:password) }
     end
   end
 
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 88cd624877a..60364df2015 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -26,6 +26,23 @@ describe BuildkiteService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:token) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe 'commits methods' do
     before do
       @project = Project.new
diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb
index 7c23c2efccd..236df8f047d 100644
--- a/spec/models/project_services/builds_email_service_spec.rb
+++ b/spec/models/project_services/builds_email_service_spec.rb
@@ -1,76 +1,71 @@
 require 'spec_helper'
 
 describe BuildsEmailService do
-  let(:build) { create(:ci_build) }
-  let(:data) { Gitlab::BuildDataBuilder.build(build) }
-  let!(:project) { create(:project, :public, ci_id: 1) }
-  let(:service) { described_class.new(project: project, active: true) }
+  let(:data) { Gitlab::BuildDataBuilder.build(create(:ci_build)) }
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
+
+      context 'when pusher is added' do
+        before { subject.add_pusher = true }
+
+        it { is_expected.not_to validate_presence_of(:recipients) }
+      end
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
+    end
+  end
 
   describe '#execute' do
     it 'sends email' do
-      service.recipients = 'test@gitlab.com'
+      subject.recipients = 'test@gitlab.com'
       data[:build_status] = 'failed'
+
       expect(BuildEmailWorker).to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with succeeded build and notify_only_broken_builds on' do
-      expect(service).to receive(:notify_only_broken_builds).and_return(true)
+      expect(subject).to receive(:notify_only_broken_builds).and_return(true)
       data[:build_status] = 'success'
+
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with failed build and build_allow_failure is true' do
       data[:build_status] = 'failed'
       data[:build_allow_failure] = true
+
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with unknown build status' do
       data[:build_status] = 'foo'
-      expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
-    end
 
-    it 'does not send email when recipients list is empty' do
-      service.recipients = ' ,, '
-      data[:build_status] = 'failed'
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
-    end
-  end
-
-  describe 'validations' do
-
-    context 'when pusher is not added' do
-      before { service.add_pusher = false }
-
-      it 'does not allow empty recipient input' do
-        service.recipients = ''
-        expect(service.valid?).to be false
-      end
-
-      it 'does allow non-empty recipient input' do
-        service.recipients = 'test@example.com'
-        expect(service.valid?).to be true
-      end
 
+      subject.execute(data)
     end
 
-    context 'when pusher is added' do
-      before { service.add_pusher = true }
+    it 'does not send email when recipients list is empty' do
+      subject.recipients = ' ,, '
+      data[:build_status] = 'failed'
 
-      it 'does allow empty recipient input' do
-        service.recipients = ''
-        expect(service.valid?).to be true
-      end
+      expect(BuildEmailWorker).not_to receive(:perform_async)
 
-      it 'does allow non-empty recipient input' do
-        service.recipients = 'test@example.com'
-        expect(service.valid?).to be true
-      end
+      subject.execute(data)
     end
   end
 end
diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb
new file mode 100644
index 00000000000..3e6da42803b
--- /dev/null
+++ b/spec/models/project_services/campfire_service_spec.rb
@@ -0,0 +1,42 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe CampfireService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+end
diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb
new file mode 100644
index 00000000000..ff976f8ec59
--- /dev/null
+++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb
@@ -0,0 +1,49 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe CustomIssueTrackerService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+end
diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb
index a2cf68a9e38..3a8e67438fc 100644
--- a/spec/models/project_services/drone_ci_service_spec.rb
+++ b/spec/models/project_services/drone_ci_service_spec.rb
@@ -28,25 +28,18 @@ describe DroneCiService, models: true do
 
   describe 'validations' do
     context 'active' do
-      before { allow(subject).to receive(:activated?).and_return(true) }
+      before { subject.active = true }
 
       it { is_expected.to validate_presence_of(:token) }
       it { is_expected.to validate_presence_of(:drone_url) }
-      it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) }
-      it { is_expected.to allow_value('http://ci.example.com').for(:drone_url) }
-      it { is_expected.not_to allow_value('this is not url').for(:drone_url) }
-      it { is_expected.not_to allow_value('http//noturl').for(:drone_url) }
-      it { is_expected.not_to allow_value('ftp://ci.example.com').for(:drone_url) }
+      it_behaves_like 'issue tracker service URL attribute', :drone_url
     end
 
     context 'inactive' do
-      before { allow(subject).to receive(:activated?).and_return(false) }
+      before { subject.active = false }
 
       it { is_expected.not_to validate_presence_of(:token) }
       it { is_expected.not_to validate_presence_of(:drone_url) }
-      it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) }
-      it { is_expected.to allow_value('http://drone.example.com').for(:drone_url) }
-      it { is_expected.to allow_value('ftp://drone.example.com').for(:drone_url) }
     end
   end
 
diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb
new file mode 100644
index 00000000000..e6f78898c82
--- /dev/null
+++ b/spec/models/project_services/emails_on_push_service_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe EmailsOnPushService do
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
+    end
+  end
+end
diff --git a/spec/models/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb
index d37978720bf..5fe5ea7d2df 100644
--- a/spec/models/external_wiki_service_spec.rb
+++ b/spec/models/project_services/external_wiki_service_spec.rb
@@ -28,13 +28,18 @@ describe ExternalWikiService, models: true do
     it { should have_one :service_hook }
   end
 
-  describe "Validations" do
-    context "active" do
-      before do
-        subject.active = true
-      end
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:external_wiki_url) }
+      it_behaves_like 'issue tracker service URL attribute', :external_wiki_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-      it { should validate_presence_of :external_wiki_url }
+      it { is_expected.not_to validate_presence_of(:external_wiki_url) }
     end
   end
 
diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb
index ff7fbcaa004..b7e627e6518 100644
--- a/spec/models/project_services/flowdock_service_spec.rb
+++ b/spec/models/project_services/flowdock_service_spec.rb
@@ -26,6 +26,20 @@ describe FlowdockService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb
index ecb3ccb1673..a08f1ac229f 100644
--- a/spec/models/project_services/gemnasium_service_spec.rb
+++ b/spec/models/project_services/gemnasium_service_spec.rb
@@ -26,6 +26,22 @@ describe GemnasiumService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+      it { is_expected.to validate_presence_of(:api_key) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+      it { is_expected.not_to validate_presence_of(:api_key) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
index 3518dbd1728..7a1f106d6e3 100644
--- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
@@ -26,6 +26,20 @@ describe GitlabIssueTrackerService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      subject { described_class.new(project: create(:project), active: true) }
+
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+    end
+
+    context 'when service is inactive' do
+      subject { described_class.new(project: create(:project), active: false) }
+
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+    end
+  end
 
   describe 'project and issue urls' do
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index d878162a220..6fb5cad5011 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -26,6 +26,20 @@ describe HipchatService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe "Execute" do
     let(:hipchat) { HipchatService.new }
     let(:user)    { create(:user, username: 'username') }
diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index b783b1a576e..4ee022a5171 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -29,14 +29,16 @@ describe IrkerService, models: true do
   end
 
   describe 'Validations' do
-    before do
-      subject.active = true
-      subject.properties['recipients'] = _recipients
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
     end
 
-    context 'active' do
-      let(:_recipients) { nil }
-      it { should validate_presence_of :recipients }
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
     end
   end
 
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 2f8193170ae..5309cfb99ff 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -26,6 +26,30 @@ describe JiraService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:api_url) }
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :api_url
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:api_url) }
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
@@ -72,7 +96,7 @@ describe JiraService, models: true do
 
     context "when a password was previously set" do
       before do
-        @jira_service = JiraService.create(
+        @jira_service = JiraService.create!(
           project: create(:project),
           properties: {
             api_url: 'http://jira.example.com/rest/api/2',
diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb
new file mode 100644
index 00000000000..f37edd4d970
--- /dev/null
+++ b/spec/models/project_services/pivotaltracker_service_spec.rb
@@ -0,0 +1,42 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe PivotaltrackerService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+end
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index 96039f9491b..555d9757b47 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -27,14 +27,20 @@ describe PushoverService, models: true do
   end
 
   describe 'Validations' do
-    context 'active' do
-      before do
-        subject.active = true
-      end
+    context 'when service is active' do
+      before { subject.active = true }
 
-      it { is_expected.to validate_presence_of :api_key }
-      it { is_expected.to validate_presence_of :user_key }
-      it { is_expected.to validate_presence_of :priority }
+      it { is_expected.to validate_presence_of(:api_key) }
+      it { is_expected.to validate_presence_of(:user_key) }
+      it { is_expected.to validate_presence_of(:priority) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:api_key) }
+      it { is_expected.not_to validate_presence_of(:user_key) }
+      it { is_expected.not_to validate_presence_of(:priority) }
     end
   end
 
diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb
new file mode 100644
index 00000000000..7d14f6e8280
--- /dev/null
+++ b/spec/models/project_services/redmine_service_spec.rb
@@ -0,0 +1,49 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe RedmineService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+end
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 478d59be08b..a97b7560137 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -26,13 +26,18 @@ describe SlackService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
-  describe "Validations" do
-    context "active" do
-      before do
-        subject.active = true
-      end
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
 
-      it { is_expected.to validate_presence_of :webhook }
+      it { is_expected.to validate_presence_of(:webhook) }
+      it_behaves_like 'issue tracker service URL attribute', :webhook
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:webhook) }
     end
   end
 
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index bc7423cee69..ad24b895170 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -27,86 +27,51 @@ describe TeamcityService, models: true do
   end
 
   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 '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
+    subject { service }
 
-    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
+    context 'when service is active' do
+      before { subject.active = true }
 
-        expect(teamcity_service).not_to validate_presence_of(:build_type)
-      end
+      it { is_expected.to validate_presence_of(:build_type) }
+      it { is_expected.to validate_presence_of(:teamcity_url) }
+      it_behaves_like 'issue tracker service URL attribute', :teamcity_url
 
-      it 'validates the presence of build_type if service is active' do
-        teamcity_service = service
-        teamcity_service.active = true
+      describe '#username' do
+        it 'does not validate the presence of username if password is nil' do
+          subject.password = nil
 
-        expect(teamcity_service).to validate_presence_of(:build_type)
-      end
-    end
+          expect(subject).not_to validate_presence_of(:username)
+        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
+        it 'validates the presence of username if password is present' do
+          subject.password = 'secret'
 
-        expect(teamcity_service).not_to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:username)
+        end
       end
 
-      it 'does not validate the presence of username if username is nil' do
-        teamcity_service = service
-        teamcity_service.active = true
-        teamcity_service.password = nil
+      describe '#password' do
+        it 'does not validate the presence of password if username is nil' do
+          subject.username = nil
 
-        expect(teamcity_service).not_to validate_presence_of(:username)
-      end
+          expect(subject).not_to validate_presence_of(:password)
+        end
 
-      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'
+        it 'validates the presence of password if username is present' do
+          subject.username = 'john'
 
-        expect(teamcity_service).to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:password)
+        end
       end
     end
 
-    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 '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'
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-        expect(teamcity_service).to validate_presence_of(:password)
-      end
+      it { is_expected.not_to validate_presence_of(:build_type) }
+      it { is_expected.not_to validate_presence_of(:teamcity_url) }
+      it { is_expected.not_to validate_presence_of(:username) }
+      it { is_expected.not_to validate_presence_of(:password) }
     end
   end