Fix bug and add unit tests for stash payload

lib/parsers/webhook_json_parser.rb

@@ -13,7 +13,7 @@ def call(body)
           module_name: repo_name.sub(%r{^.*-}, ''),
           repo_name: repo_name,
           repo_user: repo_user
-        }
+        }.delete_if { |_k, v| v.nil? }
       end
 
       def detect_vcs
@@ -91,7 +91,7 @@ def deleted?
         when 'gitlab'
           @data['after'] == '0000000000000000000000000000000000000000'
         when 'stash'
-          @data['refChanges'][0]['type'] == 'DELETED'
+          @data['refChanges'][0]['type'] == 'DELETE'
         when 'bitbucket'
           @data['push']['changes'][0]['closed']
         else
@@ -109,6 +109,8 @@ def repo_name
       end
 
       def repo_user
+        # TODO: Clarify what repo_user actually is.
+        # github is currently using the repo's 'owner', gitlab is using the user who pushed.
         case @vcs
         when 'github'
           @data['repository']['owner']['login']

spec/fixtures/stash/delete.json

@@ -0,0 +1 @@
+{"repository":{"slug":"puppet-r10k","id":72,"name":"puppet-r10k","scmId":"git","state":"AVAILABLE","statusMessage":"Available","forkable":true,"project":{"key":"PUP","id":81,"name":"Puppet","public":false,"type":"NORMAL"},"public":false},"refChanges":[{"refId":"refs/heads/feature_branch","fromHash":"08ea6861a4ab73facb1ca42309f295d5ceb77127","toHash":"0000000000000000000000000000000000000000","type":"DELETE"}],"changesets":{"size":0,"limit":500,"isLastPage":true,"values":[],"start":0}}

spec/fixtures/stash/update.json

@@ -0,0 +1 @@
+{"repository":{"slug":"puppet-r10k","id":72,"name":"puppet-r10k","scmId":"git","state":"AVAILABLE","statusMessage":"Available","forkable":true,"project":{"key":"PUP","id":81,"name":"Puppet","public":false,"type":"NORMAL"},"public":false},"refChanges":[{"refId":"refs/heads/feature_branch","fromHash":"6b1c3cca52bc27b3073084656186048b6d2f6a67","toHash":"08ea6861a4ab73facb1ca42309f295d5ceb77127","type":"UPDATE"}],"changesets":{"size":1,"limit":100,"isLastPage":true,"values":[{"fromCommit":{"id":"6b1c3cca52bc27b3073084656186048b6d2f6a67","displayId":"6b1c3cca52b"},"toCommit":{"id":"08ea6861a4ab73facb1ca42309f295d5ceb77127","displayId":"08ea6861a4a","author":{"name":"Alexander Fisher","emailAddress":"alex@linfratech.co.uk"},"authorTimestamp":1509707506000,"message":"a test commit","parents":[{"id":"6b1c3cca52bc27b3073084656186048b6d2f6a67","displayId":"6b1c3cca52b"}]},"changes":{"size":1,"limit":100,"isLastPage":true,"values":[{"contentId":"84fe9e3cfe2673ad3e2f024d7b7a192ba4f1b7d7","fromContentId":"056e13b19e2abd6d3cec860ec32207041e431b25","path":{"components":["README.md"],"parent":"","name":"README.md","extension":"md","toString":"README.md"},"executable":false,"percentUnchanged":-1,"type":"MODIFY","nodeType":"FILE","srcExecutable":false,"links":{"self":[{"href":"https://bitbucket.example.com/projects/PUP/repos/puppet-r10k/commits/08ea6861a4ab73facb1ca42309f295d5ceb77127#README.md"}]}}],"start":0},"links":{"self":[{"href":"https://bitbucket.example.com/projects/PUP/repos/puppet-r10k/commits/08ea6861a4ab73facb1ca42309f295d5ceb77127#README.md"}]}}],"start":0}}

spec/spec_helper.rb

@@ -14,6 +14,27 @@
   add_filter '/.vendor'
 end
 
+ENV['RACK_ENV'] = 'test'
+
+require File.expand_path '../../lib/puppet_webhook.rb', __FILE__
+
+module Webhook
+  module Test
+    module Methods
+      def read_fixture(name)
+        File.read(File.join(File.expand_path('..', __FILE__), 'fixtures', name))
+      end
+    end
+  end
+end
+
+module RSpecMixin
+  include Rack::Test::Methods
+  include Webhook::Test::Methods
+  def app
+    described_class
+  end
+end
 RSpec.configure do |conf|
-  conf.include Rack::Test::Methods
+  conf.include RSpecMixin
 end

spec/unit/parsers/webhook_json_parser_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe Sinatra::Parsers::WebhookJsonParser do
+  let(:result) { subject.call(payload) }
+  context 'when payload is from stash' do
+    describe '#call' do
+      context 'payload is an update' do
+        let(:payload) { read_fixture('stash/update.json') }
+        it 'returns correctly populated params hash' do
+          expect(result).to include(
+            branch: 'feature_branch',
+            deleted: false,
+            module_name: 'r10k',
+            repo_name: 'puppet-r10k'
+          )
+        end
+        it 'doesn\'t populate repo_user' do
+          # The stash payload doesn't contain anything we could use for repo_user
+          expect(result).not_to include(:repo_user)
+        end
+      end
+      context 'payload is for a delete' do
+        let(:payload) { read_fixture('stash/delete.json') }
+        it 'returns params hash with :deleted => true' do
+          expect(result).to include(deleted: true)
+        end
+      end
+    end
+  end
+end

spec/unit/puppet_webhook_spec.rb

@@ -1,12 +1,6 @@
 require 'spec_helper'
 
-require 'puppet_webhook'
-
-describe 'PuppetWebhook' do
-  def app
-    PuppetWebhook
-  end
-
+describe PuppetWebhook do
   it '/heartbeat returns 200 ok and success message' do
     get '/heartbeat'
     expect(last_response).to be_ok