Merge pull request #1664 from envato/update_before_deploy_hook

Update before_deploy hook location

app/models/deploy_service.rb

@@ -22,11 +22,12 @@ def deploy!(stage, attributes)
   end
 
   def confirm_deploy!(deploy)
-    send_before_notifications(deploy)
-
     stage = deploy.stage
 
     job_execution = JobExecution.new(deploy.reference, deploy.job, construct_env(stage))
+    job_execution.on_start do
+      send_before_notifications(deploy)
+    end
     job_execution.on_complete do
       send_after_notifications(deploy)
     end

app/models/job_execution.rb

@@ -23,7 +23,8 @@ def initialize(reference, job, env = {}, &block)
     @executor = TerminalExecutor.new(@output, verbose: true, deploy: job.deploy)
     @viewers = JobViewers.new(@output)
 
-    @subscribers = []
+    @start_callbacks = []
+    @complete_callbacks = []
     @env = env
     @job = job
     @reference = reference
@@ -72,8 +73,12 @@ def stop!
     finish
   end
 
+  def on_start(&block)
+    @start_callbacks << block
+  end
+
   def on_complete(&block)
-    @subscribers << JobExecutionSubscriber.new(job, &block)
+    @complete_callbacks << JobExecutionSubscriber.new(job, &block)
   end
 
   def descriptor
@@ -95,7 +100,7 @@ def error!(exception)
 
   def run!
     @output.write('', :started)
-
+    @start_callbacks.each(&:call)
     @job.run!
 
     success = Dir.mktmpdir("samson-#{@job.project.permalink}-#{@job.id}") do |dir|
@@ -127,7 +132,7 @@ def run!
   def finish
     return if @finished
     @finished = true
-    @subscribers.each(&:call)
+    @complete_callbacks.each(&:call)
   end
 
   def execute!(dir)

test/lib/buddy_check_test.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require_relative '../test_helper'
 
-SingleCov.covered! uncovered: 4
+SingleCov.covered! uncovered: 5
 
 describe BuddyCheck do
   let(:project) { job.project }
@@ -69,7 +69,9 @@
       stage.update_attribute(:production, true)
       job_execution.stubs(:execute!)
       job_execution.stubs(:setup!).returns(true)
-      JobExecution.expects(:start_job).returns(job_execution)
+
+      JobExecution.stubs(:new).returns(job_execution)
+      JobQueue.any_instance.stubs(:delete_and_enqueue_next) # we do not properly add the job, so removal fails
 
       BuddyCheck.stubs(:enabled?).returns(true)
     end

test/models/deploy_service_test.rb

@@ -138,6 +138,12 @@ def create_previous_deploy(ref, stage, successful: true, bypassed: false)
     describe "when buddy check is needed" do
       before do
         stage.stubs(:deploy_requires_approval?).returns(true)
+
+        job_execution.stubs(:execute!)
+        job_execution.stubs(:setup!).returns(true)
+
+        JobExecution.stubs(:new).returns(job_execution)
+        JobQueue.any_instance.stubs(:delete_and_enqueue_next) # we do not properly add the job, so removal fails
       end
 
       it "starts a job execution" do
@@ -154,6 +160,7 @@ def create_previous_deploy(ref, stage, successful: true, bypassed: false)
         DeployMailer.expects(bypass_email: stub(deliver_now: true))
         deploy.buddy = user
         service.confirm_deploy!(deploy)
+        job_execution.send(:run!)
       end
     end
   end
@@ -167,21 +174,33 @@ def create_previous_deploy(ref, stage, successful: true, bypassed: false)
   end
 
   describe "before notifications" do
+    before do
+      stage.stubs(:create_deploy).returns(deploy)
+      deploy.stubs(:persisted?).returns(true)
+      job_execution.stubs(:execute!)
+      job_execution.stubs(:setup!).returns(true)
+
+      JobExecution.stubs(:new).returns(job_execution)
+      JobQueue.any_instance.stubs(:delete_and_enqueue_next) # we do not properly add the job, so removal fails
+    end
+
     it "sends before_deploy hook" do
       record_hooks(:before_deploy) do
         service.deploy!(stage, reference: reference)
-      end.must_equal [[Deploy.first, nil]]
+        job_execution.send(:run!)
+      end.must_equal [[deploy, nil]]
     end
 
     it "creates a github deployment" do
-      deployment = stub
+      deployment = stub(update_github_deployment_status: nil)
 
       stage.stubs(:use_github_deployment_api?).returns(true)
 
       GithubDeployment.stubs(new: deployment)
       deployment.expects(:create_github_deployment)
 
       service.deploy!(stage, reference: reference)
+      job_execution.send(:run!)
     end
   end
 

test/models/job_execution_test.rb

@@ -7,8 +7,12 @@
   include GitRepoTestHelper
 
   def execute_job(branch = 'master', **options)
+    on_complete = options.delete(:on_complete)
+    on_start = options.delete(:on_start)
+
     execution = JobExecution.new(branch, job, options)
-    execution.on_complete { yield } if block_given?
+    execution.on_complete(&on_complete) if on_complete.present?
+    execution.on_start(&on_start) if on_start.present?
     execution.send(:run!)
   end
 
@@ -185,12 +189,25 @@ def last_line_of_output
     JobExecution.find_by_id(job.id).must_be_nil
   end
 
-  it 'calls subscribers after finishing' do
+  it 'calls on complete subscribers after finishing' do
+    called_subscriber = false
+    execute_job('master', on_complete: -> { called_subscriber = true })
+    assert_equal true, called_subscriber
+  end
+
+  it 'calls on start subscribers before finishing' do
     called_subscriber = false
-    execute_job { called_subscriber = true }
+    execute_job('master', on_start: -> { called_subscriber = true })
     assert_equal true, called_subscriber
   end
 
+  it 'fails when on start callback fails' do
+    execute_job('master', on_start: -> { raise(Samson::Hooks::UserError, 'failure') })
+
+    assert job.output.include?('failed')
+    assert_equal 'errored', job.status
+  end
+
   it 'outputs start / stop events' do
     execution = JobExecution.new('master', job)
     output = execution.output
@@ -214,7 +231,7 @@ def last_line_of_output
 
   it 'saves job output before calling subscriber' do
     output = nil
-    execute_job { output = job.output }
+    execute_job('master', on_complete: -> { output = job.output })
     assert_equal 'monkey', output.split("\n").last.strip
   end