Commit 5720919c authored by Shinya Maeda's avatar Shinya Maeda

Brush up 2

parent d65c816e
...@@ -18,7 +18,6 @@ module Ci ...@@ -18,7 +18,6 @@ module Ci
after_create :schedule_next_run! after_create :schedule_next_run!
def 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) 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) if next_time.present? && !less_than_1_hour_from_now?(next_time)
...@@ -29,11 +28,9 @@ module Ci ...@@ -29,11 +28,9 @@ module Ci
def real_next_run(worker_cron: nil, worker_time_zone: nil) 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_cron = Settings.cron_jobs['trigger_schedule_worker']['cron'] unless worker_cron.present?
worker_time_zone = Time.zone.name unless worker_time_zone.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) 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 if next_run_at > worker_next_time
next_run_at next_run_at
else else
...@@ -64,7 +61,7 @@ module Ci ...@@ -64,7 +61,7 @@ module Ci
def check_ref def check_ref
if !ref.present? if !ref.present?
self.errors.add(:ref, " is empty") 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") self.errors.add(:ref, " does not exist")
end end
end end
......
...@@ -4,8 +4,6 @@ class TriggerScheduleWorker ...@@ -4,8 +4,6 @@ class TriggerScheduleWorker
def perform def perform
Ci::TriggerSchedule.where("next_run_at < ?", Time.now).find_each do |trigger_schedule| 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 begin
Ci::CreateTriggerRequestService.new.execute(trigger_schedule.project, Ci::CreateTriggerRequestService.new.execute(trigger_schedule.project,
trigger_schedule.trigger, trigger_schedule.trigger,
......
...@@ -11,7 +11,7 @@ module Ci ...@@ -11,7 +11,7 @@ module Ci
def next_time_from(time) def next_time_from(time)
cronLine = try_parse_cron(@cron, @cron_time_zone) cronLine = try_parse_cron(@cron, @cron_time_zone)
if cronLine.present? if cronLine.present?
cronLine.next_time(time) cronLine.next_time(time).in_time_zone(Time.zone)
else else
nil nil
end end
......
...@@ -8,10 +8,10 @@ module Ci ...@@ -8,10 +8,10 @@ module Ci
context 'when cron and cron_time_zone are valid' do context 'when cron and cron_time_zone are valid' do
context 'when specific time' do context 'when specific time' do
let(:cron) { '3 4 5 6 *' } 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 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.min).to eq(3)
expect(subject.hour).to eq(4) expect(subject.hour).to eq(4)
expect(subject.day).to eq(5) expect(subject.day).to eq(5)
...@@ -21,20 +21,20 @@ module Ci ...@@ -21,20 +21,20 @@ module Ci
context 'when specific day of week' do context 'when specific day of week' do
let(:cron) { '* * * * 0' } let(:cron) { '* * * * 0' }
let(:cron_time_zone) { 'Europe/London' } let(:cron_time_zone) { 'UTC' }
it 'returns exact day of week in the future' do 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) expect(subject.wday).to eq(0)
end end
end end
context 'when slash used' do context 'when slash used' do
let(:cron) { '*/10 */6 */10 */10 *' } let(:cron) { '*/10 */6 */10 */10 *' }
let(:cron_time_zone) { 'US/Pacific' } let(:cron_time_zone) { 'UTC' }
it 'returns exact minute' do 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.min).to be_in([0, 10, 20, 30, 40, 50])
expect(subject.hour).to be_in([0, 6, 12, 18]) expect(subject.hour).to be_in([0, 6, 12, 18])
expect(subject.day).to be_in([1, 11, 21, 31]) expect(subject.day).to be_in([1, 11, 21, 31])
...@@ -44,22 +44,25 @@ module Ci ...@@ -44,22 +44,25 @@ module Ci
context 'when range used' do context 'when range used' do
let(:cron) { '0,20,40 * 1-5 * *' } 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 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.min).to be_in([0, 20, 40])
expect(subject.day).to be_in((1..5).to_a) expect(subject.day).to be_in((1..5).to_a)
end end
end end
context 'when cron_time_zone is US/Pacific' do context 'when cron_time_zone is US/Pacific' do
let(:cron) { '* * * * *' } let(:cron) { '0 0 * * *' }
let(:cron_time_zone) { 'US/Pacific' } let(:cron_time_zone) { 'US/Pacific' }
it 'returns next time from now' do 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.utc_offset/60/60).to eq(-7) end
it 'converts time in server time zone' do
expect(subject.hour).to eq(7)
end end
end end
end end
......
...@@ -8,12 +8,37 @@ describe Ci::TriggerSchedule, models: true do ...@@ -8,12 +8,37 @@ describe Ci::TriggerSchedule, models: true do
# it { is_expected.to validate_presence_of :cron_time_zone } # it { is_expected.to validate_presence_of :cron_time_zone }
it { is_expected.to respond_to :ref } 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 = create(:ci_trigger_schedule, :cron_nightly_build)
trigger_schedule.cron = '* * * * *' trigger_schedule.trigger.ref = 'invalid-ref'
trigger_schedule.valid? trigger_schedule.valid?
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') expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour')
end 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 describe '#schedule_next_run!' do
context 'when more_than_1_hour_from_now' do context 'when more_than_1_hour_from_now' do
...@@ -31,65 +56,25 @@ describe Ci::TriggerSchedule, models: true do ...@@ -31,65 +56,25 @@ describe Ci::TriggerSchedule, models: true do
end end
describe '#real_next_run' do describe '#real_next_run' do
subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: worker_time_zone) } let(:trigger_schedule) { create(:ci_trigger_schedule, cron: user_cron, cron_time_zone: 'UTC') }
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) }
context 'when user is in Europe/London(+00:00)' do
let(:user_time_zone) { 'Europe/London' }
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 subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: 'UTC') }
let(:user_time_zone) { 'Asia/Hong_Kong' }
it 'returns next_run_at' do context 'when next_run_at > worker_next_time' do
is_expected.to eq(trigger_schedule.next_run_at) let(:worker_cron) { '* * * * *' } # every minutes
end let(:user_cron) { '0 0 1 1 *' } # every 00:00, January 1st
end
context 'when user is in Canada/Pacific(-08:00)' do
let(:user_time_zone) { 'Canada/Pacific' }
it 'returns next_run_at' do it 'returns next_run_at' do
is_expected.to eq(trigger_schedule.next_run_at) is_expected.to eq(trigger_schedule.next_run_at)
end end
end end
end
context 'when worker_next_time > next_run_at' do context 'when worker_next_time > next_run_at' do
let(:worker_cron) { '0 0 */2 * *' } # every 2 days let(:worker_cron) { '0 0 1 1 *' } # every 00:00, January 1st
let(:worker_time_zone) { 'UTC' } let(:user_cron) { '0 */6 * * *' } # each six hours
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 it 'returns worker_next_time' do
is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now) is_expected.to eq(Ci::CronParser.new(worker_cron, 'UTC').next_time_from(Time.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' }
it 'returns worker_next_time' do
is_expected.to eq(Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now)
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment