Brush up 2

5 files changed, 55 insertions, 72 deletions

diff --git a/app/models/ci/trigger_schedule.rb b/app/models/ci/trigger_schedule.rb
index fc144d3958d..b861f41fed1 100644
--- a/app/models/ci/trigger_schedule.rb
+++ b/app/models/ci/trigger_schedule.rb
@@ -18,7 +18,6 @@ module Ci
     after_create :schedule_next_run!
 
     def schedule_next_run!
-      # puts "cron: #{cron.inspect} | cron_time_zone: #{cron_time_zone.inspect}"
       next_time = Ci::CronParser.new(cron, cron_time_zone).next_time_from(Time.now)
 
       if next_time.present? && !less_than_1_hour_from_now?(next_time)
@@ -29,11 +28,9 @@ module Ci
     def real_next_run(worker_cron: nil, worker_time_zone: nil)
       worker_cron = Settings.cron_jobs['trigger_schedule_worker']['cron'] unless worker_cron.present?
       worker_time_zone = Time.zone.name unless worker_time_zone.present?
-      # puts "real_next_run: worker_cron: #{worker_cron.inspect} | worker_time_zone: #{worker_time_zone.inspect}"
 
       worker_next_time = Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from(Time.now)
 
-      # puts "real_next_run: next_run_at: #{next_run_at.inspect} | worker_next_time: #{worker_next_time.inspect}"
       if next_run_at > worker_next_time
         next_run_at
       else
@@ -64,7 +61,7 @@ module Ci
     def check_ref
       if !ref.present?
         self.errors.add(:ref, " is empty")
-      elsif project.repository.ref_exists?(ref)
+      elsif !project.repository.branch_exists?(ref)
         self.errors.add(:ref, " does not exist")
       end
     end
diff --git a/app/workers/trigger_schedule_worker.rb b/app/workers/trigger_schedule_worker.rb
index 9216103b8da..36e640592a7 100644
--- a/app/workers/trigger_schedule_worker.rb
+++ b/app/workers/trigger_schedule_worker.rb
@@ -4,8 +4,6 @@ class TriggerScheduleWorker
 
   def perform
     Ci::TriggerSchedule.where("next_run_at < ?", Time.now).find_each do |trigger_schedule|
-      next if Ci::Pipeline.where(project: trigger_schedule.project, ref: trigger_schedule.ref).running_or_pending.count > 0
-
       begin
         Ci::CreateTriggerRequestService.new.execute(trigger_schedule.project,
                                                     trigger_schedule.trigger,
diff --git a/lib/ci/cron_parser.rb b/lib/ci/cron_parser.rb
index 2919543bbef..eb348306436 100644
--- a/lib/ci/cron_parser.rb
+++ b/lib/ci/cron_parser.rb
@@ -11,7 +11,7 @@ module Ci
     def next_time_from(time)
       cronLine = try_parse_cron(@cron, @cron_time_zone)
       if cronLine.present?
-        cronLine.next_time(time)
+        cronLine.next_time(time).in_time_zone(Time.zone)
       else
         nil
       end
diff --git a/spec/lib/ci/cron_parser_spec.rb b/spec/lib/ci/cron_parser_spec.rb
index dd1449b5b02..11d1e1c8a78 100644
--- a/spec/lib/ci/cron_parser_spec.rb
+++ b/spec/lib/ci/cron_parser_spec.rb
@@ -8,10 +8,10 @@ module Ci
       context 'when cron and cron_time_zone are valid' do
         context 'when specific time' do
           let(:cron) { '3 4 5 6 *' }
-          let(:cron_time_zone) { 'Europe/London' }
+          let(:cron_time_zone) { 'UTC' }
 
           it 'returns exact time in the future' do
-            expect(subject).to be > Time.now.in_time_zone(cron_time_zone)
+            expect(subject).to be > Time.now
             expect(subject.min).to eq(3)
             expect(subject.hour).to eq(4)
             expect(subject.day).to eq(5)
@@ -21,20 +21,20 @@ module Ci
 
         context 'when specific day of week' do
           let(:cron) { '* * * * 0' }
-          let(:cron_time_zone) { 'Europe/London' }
+          let(:cron_time_zone) { 'UTC' }
 
           it 'returns exact day of week in the future' do
-            expect(subject).to be > Time.now.in_time_zone(cron_time_zone)
+            expect(subject).to be > Time.now
             expect(subject.wday).to eq(0)
           end
         end
 
         context 'when slash used' do
           let(:cron) { '*/10 */6 */10 */10 *' }
-          let(:cron_time_zone) { 'US/Pacific' }
+          let(:cron_time_zone) { 'UTC' }
 
           it 'returns exact minute' do
-            expect(subject).to be > Time.now.in_time_zone(cron_time_zone)
+            expect(subject).to be > Time.now
             expect(subject.min).to be_in([0, 10, 20, 30, 40, 50])
             expect(subject.hour).to be_in([0, 6, 12, 18])
             expect(subject.day).to be_in([1, 11, 21, 31])
@@ -44,22 +44,25 @@ module Ci
 
         context 'when range used' do
           let(:cron) { '0,20,40 * 1-5 * *' }
-          let(:cron_time_zone) { 'US/Pacific' }
+          let(:cron_time_zone) { 'UTC' }
 
           it 'returns next time from now' do
-            expect(subject).to be > Time.now.in_time_zone(cron_time_zone)
+            expect(subject).to be > Time.now
             expect(subject.min).to be_in([0, 20, 40])
             expect(subject.day).to be_in((1..5).to_a)
           end
         end
 
         context 'when cron_time_zone is US/Pacific' do
-          let(:cron) { '* * * * *' }
+          let(:cron) { '0 0 * * *' }
           let(:cron_time_zone) { 'US/Pacific' }
 
           it 'returns next time from now' do
-            expect(subject).to be > Time.now.in_time_zone(cron_time_zone)
-            expect(subject.utc_offset/60/60).to eq(-7)
+            expect(subject).to be > Time.now
+          end
+
+          it 'converts time in server time zone' do
+            expect(subject.hour).to eq(7)
           end
         end
       end
diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb
index d47ab529bc0..da23611f1a0 100644
--- a/spec/models/ci/trigger_schedule_spec.rb
+++ b/spec/models/ci/trigger_schedule_spec.rb
@@ -8,11 +8,36 @@ describe Ci::TriggerSchedule, models: true do
   # it { is_expected.to validate_presence_of :cron_time_zone }
   it { is_expected.to respond_to :ref }
 
-  it 'should validate less_than_1_hour_from_now' do
+  it 'should validate ref existence' do
     trigger_schedule = create(:ci_trigger_schedule, :cron_nightly_build)
-    trigger_schedule.cron = '* * * * *'
+    trigger_schedule.trigger.ref = 'invalid-ref'
     trigger_schedule.valid?
-    expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour')
+    expect(trigger_schedule.errors[:ref].first).to include('does not exist')
+  end
+
+  describe 'cron limitation' do
+    let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) }
+
+    before do
+      trigger_schedule.cron = cron
+      trigger_schedule.valid?
+    end
+
+    context 'when every hour' do
+      let(:cron) { '0 * * * *' } # 00:00, 01:00, 02:00, ..., 23:00
+
+      it 'fails' do
+        expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour')
+      end
+    end
+
+    context 'when each six hours' do
+      let(:cron) { '0 */6 * * *' } # 00:00, 06:00, 12:00, 18:00
+
+      it 'succeeds' do
+        expect(trigger_schedule.errors[:cron]).to be_empty
+      end
+    end
   end
 
   describe '#schedule_next_run!' do
@@ -31,65 +56,25 @@ describe Ci::TriggerSchedule, models: true do
   end
 
   describe '#real_next_run' do
-    subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: worker_time_zone) }
-
-    context 'when next_run_at > worker_next_time' do
-      let(:worker_cron) { '0 */12 * * *' } # each 00:00, 12:00
-      let(:worker_time_zone) { 'UTC' }
-      let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_weekly_build, cron_time_zone: user_time_zone, trigger: trigger) }
+    let(:trigger_schedule) { create(:ci_trigger_schedule, cron: user_cron, cron_time_zone: 'UTC') }
 
-      context 'when user is in Europe/London(+00:00)' do
-        let(:user_time_zone) { 'Europe/London' }
+    subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: 'UTC') }
 
-        it 'returns next_run_at' do
-          is_expected.to eq(trigger_schedule.next_run_at)
-        end
-      end
-
-      context 'when user is in Asia/Hong_Kong(+08:00)' do
-        let(:user_time_zone) { 'Asia/Hong_Kong' }
-
-        it 'returns next_run_at' do
-          is_expected.to eq(trigger_schedule.next_run_at)
-        end
-      end
-
-      context 'when user is in Canada/Pacific(-08:00)' do
-        let(:user_time_zone) { 'Canada/Pacific' }
+    context 'when next_run_at > worker_next_time' do
+      let(:worker_cron) { '* * * * *' } # every minutes
+      let(:user_cron) { '0 0 1 1 *' } # every 00:00, January 1st
 
-        it 'returns next_run_at' do
-          is_expected.to eq(trigger_schedule.next_run_at)
-        end
+      it 'returns next_run_at' do
+        is_expected.to eq(trigger_schedule.next_run_at)
       end
     end
 
     context 'when worker_next_time > next_run_at' do
-      let(:worker_cron) { '0 0 */2 * *' } # every 2 days
-      let(:worker_time_zone) { 'UTC' }
-      let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, cron_time_zone: user_time_zone, trigger: trigger) }
-
-      context 'when user is in Europe/London(+00:00)' do
-        let(:user_time_zone) { 'Europe/London' }
-
-        it 'returns worker_next_time' do
-          is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now)
-        end
-      end
-
-      context 'when user is in Asia/Hong_Kong(+08:00)' do
-        let(:user_time_zone) { 'Asia/Hong_Kong' }
-
-        it 'returns worker_next_time' do
-          is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now)
-        end
-      end
-
-      context 'when user is in Canada/Pacific(-08:00)' do
-        let(:user_time_zone) { 'Canada/Pacific' }
+      let(:worker_cron) { '0 0 1 1 *' } # every 00:00, January 1st
+      let(:user_cron) { '0 */6 * * *' } # each six hours
 
-        it 'returns worker_next_time' do
-          is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now)
-        end
+      it 'returns worker_next_time' do
+        is_expected.to eq(Ci::CronParser.new(worker_cron, 'UTC').next_time_from(Time.now))
       end
     end
   end