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