Merge pull request #308 from travis-ci/fix_crons

Fix crons

lib/travis/api/v3/models/cron.rb

@@ -13,7 +13,7 @@ module Travis::API::V3
       elsif last_cron_build_date >= planned_time(LastBuild)
         planned_time(ThisBuild)
       else
-        Time.now
+        Time.now - 5.minutes
       end
     end
 

lib/travis/api/v3/queries/crons.rb

@@ -7,10 +7,15 @@ module Travis::API::V3
 
     def start_all()
       Models::Cron.all.select do |cron|
-        start(cron) if cron.next_enqueuing <= Time.now
+        begin
+          @cron = cron
+          start(cron) if cron.next_enqueuing <= Time.now
+        rescue => e
+          Raven.capture_exception(e, tags: { 'cron_id' => @cron.try(:id) })
+          sleep(10) # This ensures the dyno does not spin down before the http request to send the error to sentry completes
+          next
+        end
       end
-      rescue => e
-        Raven.capture_exception(e)
     end
 
     def start(cron)

lib/travis/api/v3/service.rb

@@ -68,7 +68,7 @@ module Travis::API::V3
 
     def check_login_and_find(*args)
       raise LoginRequired unless access_control.full_access_or_logged_in?
-      find(*args) or raise NotFound 
+      find(*args) or raise NotFound
     end
 
     def not_found(actually_not_found = false, type = nil)

spec/v3/models/cron_spec.rb

@@ -43,11 +43,13 @@ describe Travis::API::V3::Models::Cron do
   describe "push build is ignored if disable by build is false" do
 
     before do
+      Timecop.return
       Timecop.travel(DateTime.new(2015, 12, 31, 16))
     end
 
     after do
       Timecop.return
+      Timecop.freeze(Time.now.utc)
     end
 
     it "for daily builds" do
@@ -85,11 +87,13 @@ describe Travis::API::V3::Models::Cron do
   describe "disable by build works with build" do
 
     before do
+      Timecop.return
       Timecop.travel(DateTime.new(2015, 12, 31, 16))
     end
 
     after do
       Timecop.return
+      Timecop.freeze(Time.now.utc)
     end
 
     it "for daily builds" do
@@ -121,11 +125,13 @@ describe Travis::API::V3::Models::Cron do
   describe "disable by build works without build" do
 
     before do
+      Timecop.return
       Timecop.travel(DateTime.new(2015, 12, 31, 16))
     end
 
     after do
       Timecop.return
+      Timecop.freeze(Time.now.utc)
     end
 
     it "for daily builds" do
@@ -157,12 +163,12 @@ describe Travis::API::V3::Models::Cron do
   describe "build starts now if next build time is in the past" do
 
     before do
-      # nothing, this time
-      # time freeze is performed in examples
+      Timecop.return
     end
 
     after do
       Timecop.return
+      Timecop.freeze(Time.now.utc)
     end
 
     it "for daily builds with disable_by_build true" do
@@ -170,7 +176,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'daily', disable_by_build: true)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 1, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end
@@ -180,7 +186,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'daily', disable_by_build: false)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 1, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end
@@ -190,7 +196,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'weekly', disable_by_build: true)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 7, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end
@@ -200,7 +206,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'weekly', disable_by_build: false)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 7, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end
@@ -210,7 +216,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'monthly', disable_by_build: true)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 31, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end
@@ -220,7 +226,7 @@ describe Travis::API::V3::Models::Cron do
       cron = Travis::API::V3::Models::Cron.create(branch_id: branch.id, interval: 'monthly', disable_by_build: false)
       build = Travis::API::V3::Models::Build.create(:repository_id => repo.id, :branch_name => branch.name, :event_type => 'cron')
       Timecop.freeze(DateTime.new(2016, 1, 31, 19))
-      expect(cron.next_enqueuing).to be == DateTime.now
+      expect(cron.next_enqueuing).to be == DateTime.now - 5.minutes
       build.destroy
       cron.destroy
     end

spec/v3/queries/cron_spec.rb

@@ -4,6 +4,7 @@ describe Travis::API::V3::Queries::Crons do
   let(:user) { Travis::API::V3::Models::User.find_by_login('svenfuchs') }
   let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first }
   let(:existing_branch) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test-existing', exists_on_github: true) }
+  let(:existing_branch2) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test-existing2', exists_on_github: true) }
   let(:non_existing_branch) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test-non-existing', exists_on_github: false) }
   let(:query) { Travis::API::V3::Queries::Crons.new({}, 'Overview')
 }
@@ -25,8 +26,22 @@ describe Travis::API::V3::Queries::Crons do
     it 'enques error into a thread' do
       cron = Travis::API::V3::Models::Cron.create(branch_id: existing_branch.id, interval: 'daily', disable_by_build: false)
       error = StandardError.new('Konstantin broke all the thingz!')
+      Travis::API::V3::Queries::Crons.any_instance.expects(:sleep).with(10)
       Travis::API::V3::Models::Cron.any_instance.stubs(:branch).raises(error)
-      Raven.expects(:capture_exception).with(error)
+      Raven.expects(:capture_exception).with(error, tags: {'cron_id' => cron.id })
+      query.start_all
+    end
+
+    it 'continues running crons if one breaks' do
+      cron = Travis::API::V3::Models::Cron.create(branch_id: existing_branch.id, interval: 'daily', disable_by_build: false)
+      cron2 = Travis::API::V3::Models::Cron.create(branch_id: existing_branch2.id, interval: 'daily', disable_by_build: false)
+      
+      error = StandardError.new('Konstantin broke all the thingz!')
+      Travis::API::V3::Models::Cron.any_instance.stubs(:branch).raises(error)
+
+      Travis::API::V3::Queries::Crons.any_instance.expects(:sleep).twice.with(10)
+      Raven.expects(:capture_exception).with(error, tags: {'cron_id' => cron.id })
+      Raven.expects(:capture_exception).with(error, tags: {'cron_id' => cron2.id })
       query.start_all
     end
   end