Merge pull request #36 from travis-ci/fix-campfire-notifications

Fix problem with Campfire hook notifying on start.

lib/travis/notifications/handler/campfire.rb

@@ -7,6 +7,7 @@ module Handler
       #
       # Campfire credentials are encrypted using the repository's ssl key.
       class Campfire < Webhook
+        EVENTS = /build:finished/
 
         class << self
           def campfire_url(config)

lib/travis/notifications/subscription.rb

@@ -1,3 +1,5 @@
+require 'metriks'
+
 module Travis
   module Notifications
 
@@ -20,12 +22,20 @@ def initialize(name)
         end
 
         def notify(event, *args)
-          subscriber.new.notify(event, *args) if matches?(event)
+          if matches?(event)
+            subscriber.new.notify(event, *args)
+            increment_counter(event)
+          end
         end
 
         def matches?(event)
           patterns.any? { |patterns| patterns.is_a?(Regexp) ? patterns.match(event) : patterns == event }
         end
+
+        def increment_counter(event)
+          metric = "travis.notifications.#{name}.#{event.gsub(/:/, '.')}"
+          Metriks.counter(metric).increment
+        end
       }
     end
   end

spec/travis/notifications/handler/campfire_spec.rb

@@ -19,6 +19,11 @@
     end
   end
 
+  it "only is set up to accept build:finished notifications" do
+    Travis::Notifications::Handler::Campfire::EVENTS.should === 'build:finished'
+    Travis::Notifications::Handler::Campfire::EVENTS.should_not === 'build:started'
+  end
+
   it "sends campfire notifications to the room given as a string" do
     target = 'evome:apitoken@42'
     build.config[:notifications][:campfire] = target

spec/travis/notifications/handler/webhook_spec.rb

@@ -20,6 +20,11 @@
     end
   end
 
+  it "only is set up to accept build:finished and build:started notifications" do
+    Travis::Notifications::Handler::Webhook::EVENTS.should === 'build:finished'
+    Travis::Notifications::Handler::Webhook::EVENTS.should === 'build:started'
+  end
+
   it 'sends webhook notifications to a url given as a string' do
     target = 'http://evome.fr/notifications'
     build.config[:notifications][:webhooks] = target

spec/travis/notifications/subscription_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+require 'active_support/core_ext/class'
+
+describe Travis::Notifications::Subscription do
+  class Travis::Notifications::Handler::SubscriptionTestHandler
+    class_attribute :events
+    self.events = []
+
+    EVENTS = /build:finished/
+
+    def notify(*args)
+      self.class.notify(*args)
+    end
+
+    def self.notify(*args)
+      events << args
+    end
+  end
+
+  let(:subscription) {Travis::Notifications::Subscription.new(:subscription_test_handler)}
+
+  describe "triggering a notification" do
+    before do
+      subscription.subscriber.events.clear
+    end
+
+    it "should notify when the event matches" do
+      subscription.notify('build:finished')
+      subscription.subscriber.events.should have(1).item
+    end
+
+    it "should increment a counter when the event is triggered" do
+      expect {
+        subscription.notify('build:finished')
+      }.to change {Metriks.counter('travis.notifications.subscription_test_handler.build.finished').count}
+    end
+
+    it "shouldn't notify when the event doesn't match" do
+      subscription.notify('build:started') 
+      subscription.subscriber.events.should have(0).items
+    end
+  end
+end

spec/travis/notifications_spec.rb

@@ -0,0 +1,47 @@
+require 'spec_helper'
+require 'support/active_record'
+
+# A more high-level test for notifications as a whole
+describe Travis::Notifications do
+  include Travis::Notifications
+  include Support::ActiveRecord
+
+  let(:build)    { Factory(:build, :config => { 'notifications' => { 'campfire' => 'evome:apitoken@42' } }) }
+  describe "notifying of an event" do
+    describe "campfire" do
+      before do
+        Travis.config.notifications = [:campfire]
+      end
+
+      it "should not publish start events to campfire" do
+        Travis::Notifications::Handler::Campfire.any_instance.expects(:notify).never
+        notify("build:started", build)
+      end
+
+      it "should publish finish events to campfire" do
+        Travis::Notifications::Handler::Campfire.any_instance.expects(:notify)
+        notify("build:finished", build)
+      end
+    end
+
+    describe "webhooks" do
+      before do
+        Travis.config.notifications = [:webhook]
+      end
+
+      it "should publish start events to webhooks" do
+        targets = ['http://evome.fr/notifications', 'http://example.com/']
+        build.config[:notifications][:webhooks] = {:urls => targets}
+        Travis::Notifications::Handler::Webhook.any_instance.expects(:notify)
+        notify("build:started")
+      end
+
+      it "should publish finish events to webhooks" do
+        targets = ['http://evome.fr/notifications', 'http://example.com/']
+        build.config[:notifications][:webhooks] = {:urls => targets}
+        Travis::Notifications::Handler::Webhook.any_instance.expects(:notify)
+        notify("build:finish")
+      end
+    end
+  end
+end