Slack pipeline events test and fix some issues along the way

app/models/project_services/slack_service.rb

@@ -66,16 +66,20 @@ class SlackService < Service
 
     message = get_message(object_kind, data)
 
-    opt = {}
-
-    event_channel = get_channel_field(object_kind) || channel
-
-    opt[:channel] = event_channel if event_channel
-    opt[:username] = username if username
-
     if message
+      opt = {}
+
+      event_channel = get_channel_field(object_kind) || channel
+
+      opt[:channel] = event_channel if event_channel
+      opt[:username] = username if username
+
       notifier = Slack::Notifier.new(webhook, opt)
       notifier.ping(message.pretext, attachments: message.attachments, fallback: message.fallback)
+
+      true
+    else
+      false
     end
   end
 
@@ -153,7 +157,7 @@ class SlackService < Service
   def should_pipeline_be_notified?(data)
     case data[:object_attributes][:status]
     when 'success'
-      !notify_only_broken_builds?
+      !notify_only_broken_pipelines?
     when 'failed'
       true
     else

app/models/project_services/slack_service/pipeline_message.rb

@@ -4,15 +4,17 @@ class SlackService
                 :user_name, :duration, :pipeline_id
 
     def initialize(data)
-      @sha = data[:sha]
-      @ref_type = data[:tag] ? 'tag' : 'branch'
-      @ref = data[:ref]
-      @status = data[:status]
+      pipeline_attributes = data[:object_attributes]
+      @sha = pipeline_attributes[:sha]
+      @ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch'
+      @ref = pipeline_attributes[:ref]
+      @status = pipeline_attributes[:status]
+      @duration = pipeline_attributes[:duration]
+      @pipeline_id = pipeline_attributes[:id]
+
       @project_name = data[:project][:path_with_namespace]
       @project_url = data[:project][:web_url]
       @user_name = data[:commit] && data[:commit][:author_name]
-      @duration = data[:object_attributes][:duration]
-      @pipeline_id = data[:object_attributes][:id]
     end
 
     def pretext

spec/models/project_services/slack_service_spec.rb

@@ -21,6 +21,9 @@
 require 'spec_helper'
 
 describe SlackService, models: true do
+  let(:slack) { SlackService.new }
+  let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
+
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -42,11 +45,9 @@ describe SlackService, models: true do
   end
 
   describe "Execute" do
-    let(:slack)   { SlackService.new }
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
     let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
-    let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
     let(:username) { 'slack_username' }
     let(:channel) { 'slack_channel' }
 
@@ -210,10 +211,8 @@ describe SlackService, models: true do
   end
 
   describe "Note events" do
-    let(:slack)   { SlackService.new }
     let(:user) { create(:user) }
     let(:project) { create(:project, creator_id: user.id) }
-    let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
 
     before do
       allow(slack).to receive_messages(
@@ -283,4 +282,63 @@ describe SlackService, models: true do
       end
     end
   end
+
+  describe 'Pipeline events' do
+    let(:user) { create(:user) }
+    let(:project) { create(:project) }
+    let(:pipeline) do
+      create(:ci_pipeline,
+             project: project, status: status,
+             sha: project.commit.sha, ref: project.default_branch)
+    end
+    let(:status) { raise NotImplementedError }
+
+    before do
+      allow(slack).to receive_messages(
+        project: project,
+        service_hook: true,
+        webhook: webhook_url
+      )
+    end
+
+    shared_examples 'call Slack API' do
+      before do
+        WebMock.stub_request(:post, webhook_url)
+      end
+
+      it 'calls Slack API for pipeline events' do
+        data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline)
+        slack.execute(data)
+
+        expect(WebMock).to have_requested(:post, webhook_url).once
+      end
+    end
+
+    context 'with failed pipeline' do
+      let(:status) { 'failed' }
+
+      it_behaves_like 'call Slack API'
+    end
+
+    context 'with succeeded pipeline' do
+      let(:status) { 'success' }
+
+      context 'with default to notify_only_broken_pipelines' do
+        it 'does not call Slack API for pipeline events' do
+          data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline)
+          result = slack.execute(data)
+
+          expect(result).to be_falsy
+        end
+      end
+
+      context 'with setting notify_only_broken_pipelines to false' do
+        before do
+          slack.notify_only_broken_pipelines = false
+        end
+
+        it_behaves_like 'call Slack API'
+      end
+    end
+  end
 end