5 files changed, 139 insertions, 25 deletions
diff --git a/app/models/project_services/hip_chat_message.rb b/app/models/project_services/hip_chat_message.rb
index 85695cc..8350bb6 100644
--- a/app/models/project_services/hip_chat_message.rb
+++ b/app/models/project_services/hip_chat_message.rb
@@ -7,42 +7,56 @@ class HipChatMessage
 
   def to_s
     lines = Array.new
-    lines.push("<a href=\"#{RoutesHelper.project_url(build.project)}\">#{build.project.name}</a> - ")
-    if build.commit.matrix?
-      lines.push("<a href=\"#{RoutesHelper.project_ref_commit_url(build.project, build.commit.ref, build.commit.sha)}\">Commit ##{commit.id}</a></br>")
+    lines.push("<a href=\"#{RoutesHelper.project_url(project)}\">#{project.name}</a> - ")
+    if commit.matrix?
+      lines.push("<a href=\"#{RoutesHelper.project_ref_commit_url(project, commit.ref, commit.sha)}\">Commit ##{commit.id}</a></br>")
     else
-      lines.push("<a href=\"#{RoutesHelper.project_build_url(build.project, build)}\">Build '#{build.job_name}' ##{build.id}</a></br>")
+      first_build = commit.builds_without_retry.first
+      lines.push("<a href=\"#{RoutesHelper.project_build_url(project, first_build)}\">Build '#{first_build.job_name}' ##{first_build.id}</a></br>")
     end
-    lines.push("#{build.commit.short_sha} #{build.commit.git_author_name} - #{build.commit.git_commit_message}</br>")
-    lines.push("#{humanized_status} in #{build.commit.duration} second(s).")
+    lines.push("#{commit.short_sha} #{commit.git_author_name} - #{commit.git_commit_message}</br>")
+    lines.push("#{humanized_status(commit_status)} in #{commit.duration} second(s).")
     lines.join('')
   end
 
-  def color
-    case status
+  def status_color(build_or_commit=nil)
+    build_or_commit ||= commit_status
+    case build_or_commit
     when :success
       'green'
     when :failed, :canceled
       'red'
-    when :pending, :running
+    else # :pending, :running or unknown
       'yellow'
-    else
-      'random'
     end
   end
 
   def notify?
-    [:failed, :canceled].include?(status)
+    [:failed, :canceled].include?(commit_status)
   end
 
+
   private
 
-  def status
+  def commit
+    build.commit
+  end
+
+  def project
+    commit.project
+  end
+
+  def build_status
     build.status.to_sym
   end
 
-  def humanized_status
-    case status
+  def commit_status
+    commit.status.to_sym
+  end
+
+  def humanized_status(build_or_commit=nil)
+    build_or_commit ||= commit_status
+    case build_or_commit
     when :pending
       "Pending"
     when :running
diff --git a/app/models/project_services/hip_chat_service.rb b/app/models/project_services/hip_chat_service.rb
index 0faaca5..8e5f024 100644
--- a/app/models/project_services/hip_chat_service.rb
+++ b/app/models/project_services/hip_chat_service.rb
@@ -47,21 +47,34 @@ class HipChatService < Service
     commit = build.commit
     return unless commit
     return unless commit.builds_without_retry.include? build
+    return if commit.success? and notify_only_broken_builds?
+    return if commit.running?
 
     msg = HipChatMessage.new(build)
-    HipChatNotifierWorker.perform_async(hipchat_room, hipchat_token, msg.to_s, {
-      message_format: 'html',
-      color: msg.color,
-      notify: notify_only_broken_builds? && msg.notify?
-    })
+    opts = default_options.merge(
+      token: hipchat_token,
+      room: hipchat_room,
+      server: server_url,
+      color: msg.status_color,
+      notify: msg.notify?
+    )
+    HipChatNotifierWorker.perform_async(msg.to_s, opts)
   end
 
   private
 
   def default_options
     {
-      hipchat_server: 'https://api.hipchat.com'
+      service_name: 'GitLab CI',
+      message_format: 'html'
     }
   end
 
+  def server_url
+    if hipchat_server.blank?
+      'https://api.hipchat.com'
+    else
+      hipchat_server
+    end
+  end
 end
diff --git a/app/workers/hip_chat_notifier_worker.rb b/app/workers/hip_chat_notifier_worker.rb
index 5bc7dda..0403578 100644
--- a/app/workers/hip_chat_notifier_worker.rb
+++ b/app/workers/hip_chat_notifier_worker.rb
@@ -2,8 +2,17 @@
 class HipChatNotifierWorker
   include Sidekiq::Worker
 
-  def perform(room, token, message, options={})
-    client = HipChat::Client.new(token, api_version: 'v2') # v1.5.0 requires explicit version (
-    client[room].send("GitLab CI", message, options)
+  def perform(message, options={})
+    room   = options.delete('room')
+    token  = options.delete('token')
+    server = options.delete('server')
+    name   = options.delete('service_name')
+    client_opts = {
+      api_version: 'v2',
+      server_url: server
+    }
+
+    client = HipChat::Client.new(token, client_opts)
+    client[room].send(name, message, options.symbolize_keys)
   end
 end
diff --git a/spec/models/project_services/hip_chat_message_spec.rb b/spec/models/project_services/hip_chat_message_spec.rb
new file mode 100644
index 0000000..6096b5c
--- /dev/null
+++ b/spec/models/project_services/hip_chat_message_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe HipChatMessage do
+  subject { HipChatMessage.new(build) }
+
+  let(:project) { FactoryGirl.create(:project) }
+  let(:commit)  { FactoryGirl.create(:commit, project: project) }
+  let(:job)     { FactoryGirl.create(:job, project: project) }
+  let(:build)   { FactoryGirl.create(:build, commit: commit, job: job, status: 'success') }
+
+  context 'when build succeeds' do
+
+    before { build.save }
+
+    it 'returns a successful message' do
+      expect( subject.status_color ).to eq 'green'
+      expect( subject.notify? ).to be_false
+      expect( subject.to_s ).to match(/Build '[^']+' #\d+/)
+      expect( subject.to_s ).to match(/Successful in \d+ second\(s\)\./)
+    end
+  end
+
+  context 'when build fails' do
+
+    before do
+      build.status = 'failed'
+      build.save
+    end
+
+    it 'returns a failure message' do
+      expect( subject.status_color ).to eq 'red'
+      expect( subject.notify? ).to be_true
+      expect( subject.to_s ).to match(/Build '[^']+' #\d+/)
+      expect( subject.to_s ).to match(/Failed in \d+ second\(s\)\./)
+    end
+  end
+
+  context 'when all matrix builds succeed' do
+    let(:job2)    { FactoryGirl.create(:job, project: project, name: 'Another Job') }
+    let(:build2)  { FactoryGirl.create(:build, id: 10, commit: commit, job: job2, status: 'success') }
+
+    before { build.save; build2.save }
+
+    it 'returns a successful message' do
+      expect( subject.status_color ).to eq 'green'
+      expect( subject.notify? ).to be_false
+      expect( subject.to_s ).to match(/Commit #\d+/)
+      expect( subject.to_s ).to match(/Successful in \d+ second\(s\)\./)
+    end
+  end
+
+  context 'when at least one matrix build fails' do
+    let(:job2)    { FactoryGirl.create(:job, project: project, name: 'Another Job') }
+    let(:build2)  { FactoryGirl.create(:build, id: 10, commit: commit, job: job2, status: 'failed') }
+
+    before { build.save; build2.save }
+
+    it 'returns a failure message' do
+      expect( subject.status_color ).to eq 'red'
+      expect( subject.notify? ).to be_true
+      expect( subject.to_s ).to match(/Commit #\d+/)
+      expect( subject.to_s ).to match(/Failed in \d+ second\(s\)\./)
+    end
+  end
+end
diff --git a/spec/models/project_services/hip_chat_service_spec.rb b/spec/models/project_services/hip_chat_service_spec.rb
index e578461..5642221 100644
--- a/spec/models/project_services/hip_chat_service_spec.rb
+++ b/spec/models/project_services/hip_chat_service_spec.rb
@@ -41,7 +41,20 @@ describe HipChatService do
       service.execute(build)
       HipChatNotifierWorker.drain
 
-      WebMock.should have_requested(:post, api_url).once
+      expect( WebMock ).to have_requested(:post, api_url).once
+    end
+
+    it "calls the worker with expected arguments" do
+      expect( HipChatNotifierWorker ).to receive(:perform_async) \
+        .with(an_instance_of(String), hash_including(
+          token: 'a1b2c3d4e5f6',
+          room: 123,
+          server: 'https://api.hipchat.com',
+          color: 'red',
+          notify: true
+        ))
+
+      service.execute(build)
     end
   end
 end