Commit d65c816e authored by Shinya Maeda's avatar Shinya Maeda

Brush up

parent 9573bb44
......@@ -18,20 +18,22 @@ 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_now
if next_time.present?
# 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)
update!(next_run_at: next_time)
end
end
def real_next_run(worker_cron: nil, worker_time_zone: nil)
puts "worker_cron: #{worker_cron.inspect} | worker_time_zone: #{worker_time_zone.inspect}"
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_next_time = Ci::CronParser.new(worker_cron, worker_time_zone).next_time_from_now
# 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 "next_run_at: #{next_run_at.inspect} | worker_next_time: #{worker_next_time.inspect}"
# 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
......@@ -41,15 +43,20 @@ module Ci
private
def less_than_1_hour_from_now?(time)
((time - Time.now).abs < 1.hour) ? true : false
end
def check_cron
cron_parser = Ci::CronParser.new(cron, cron_time_zone)
is_valid_cron, is_valid_cron_time_zone = cron_parser.validation
next_time = cron_parser.next_time_from(Time.now)
if !is_valid_cron
self.errors.add(:cron, " is invalid syntax")
elsif !is_valid_cron_time_zone
self.errors.add(:cron_time_zone, " is invalid timezone")
elsif (cron_parser.next_time_from_now - Time.now).abs < 1.hour
elsif less_than_1_hour_from_now?(next_time)
self.errors.add(:cron, " can not be less than 1 hour")
end
end
......
......@@ -4,11 +4,14 @@ 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,
trigger_schedule.ref)
rescue => e
puts "#{trigger_schedule.id}: Failed to trigger_schedule job: #{e.message}" # TODO: Remove before merge
Rails.logger.error "#{trigger_schedule.id}: Failed to trigger_schedule job: #{e.message}"
ensure
trigger_schedule.schedule_next_run!
......
......@@ -8,10 +8,13 @@ module Ci
@cron_time_zone = cron_time_zone
end
def next_time_from_now
def next_time_from(time)
cronLine = try_parse_cron(@cron, @cron_time_zone)
return nil unless cronLine.present?
cronLine.next_time
if cronLine.present?
cronLine.next_time(time)
else
nil
end
end
def validation
......
FactoryGirl.define do
factory :ci_trigger_schedule, class: Ci::TriggerSchedule do
project factory: :project
trigger factory: :ci_trigger_with_ref
trigger factory: :ci_trigger_for_trigger_schedule
after(:build) do |trigger_schedule, evaluator|
trigger_schedule.update!(project: trigger_schedule.trigger.project)
end
trait :force_triggable do
after(:create) do |trigger_schedule, evaluator|
trigger_schedule.next_run_at -= 1.month
trigger_schedule.update!(next_run_at: trigger_schedule.next_run_at - 1.year)
end
end
......
......@@ -3,7 +3,9 @@ FactoryGirl.define do
factory :ci_trigger do
token { SecureRandom.hex(15) }
factory :ci_trigger_with_ref do
factory :ci_trigger_for_trigger_schedule do
owner factory: :user
project factory: :project
ref 'master'
end
end
......
......@@ -2,8 +2,8 @@ require 'spec_helper'
module Ci
describe CronParser, lib: true do
describe '#next_time_from_now' do
subject { described_class.new(cron, cron_time_zone).next_time_from_now }
describe '#next_time_from' do
subject { described_class.new(cron, cron_time_zone).next_time_from(Time.now) }
context 'when cron and cron_time_zone are valid' do
context 'when specific time' do
......@@ -65,8 +65,8 @@ module Ci
end
context 'when cron and cron_time_zone are invalid' do
let(:cron) { 'hack' }
let(:cron_time_zone) { 'hack' }
let(:cron) { 'invalid_cron' }
let(:cron_time_zone) { 'invalid_cron_time_zone' }
it 'returns nil' do
is_expected.to be_nil
......@@ -74,25 +74,23 @@ module Ci
end
end
describe '#valid_syntax?' do
subject { described_class.new(cron, cron_time_zone).valid_syntax? }
context 'when cron and cron_time_zone are valid' do
let(:cron) { '* * * * *' }
let(:cron_time_zone) { 'Europe/Istanbul' }
it 'returns true' do
is_expected.to eq(true)
end
describe '#validation' do
it 'returns results' do
is_valid_cron, is_valid_cron_time_zone = described_class.new('* * * * *', 'Europe/Istanbul').validation
expect(is_valid_cron).to eq(true)
expect(is_valid_cron_time_zone).to eq(true)
end
context 'when cron and cron_time_zone are invalid' do
let(:cron) { 'hack' }
let(:cron_time_zone) { 'hack' }
it 'returns results' do
is_valid_cron, is_valid_cron_time_zone = described_class.new('*********', 'Europe/Istanbul').validation
expect(is_valid_cron).to eq(false)
expect(is_valid_cron_time_zone).to eq(true)
end
it 'returns false' do
is_expected.to eq(false)
end
it 'returns results' do
is_valid_cron, is_valid_cron_time_zone = described_class.new('* * * * *', 'Invalid-zone').validation
expect(is_valid_cron).to eq(true)
expect(is_valid_cron_time_zone).to eq(false)
end
end
end
......
require 'spec_helper'
describe Ci::TriggerSchedule, models: true do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:trigger) { create(:ci_trigger, owner: user, project: project, ref: 'master') }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:trigger) }
......@@ -11,17 +8,27 @@ describe Ci::TriggerSchedule, models: true do
# it { is_expected.to validate_presence_of :cron_time_zone }
it { is_expected.to respond_to :ref }
# describe '#schedule_next_run!' do
# let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, next_run_at: nil, trigger: trigger) }
it 'should validate less_than_1_hour_from_now' do
trigger_schedule = create(:ci_trigger_schedule, :cron_nightly_build)
trigger_schedule.cron = '* * * * *'
trigger_schedule.valid?
expect(trigger_schedule.errors[:cron].first).to include('can not be less than 1 hour')
end
describe '#schedule_next_run!' do
context 'when more_than_1_hour_from_now' do
let(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) }
# before do
# trigger_schedule.schedule_next_run!
# end
before do
trigger_schedule.schedule_next_run!
end
# it 'updates next_run_at' do
# expect(Ci::TriggerSchedule.last.next_run_at).not_to be_nil
# end
# end
it 'updates next_run_at' do
next_time = Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_time_zone).next_time_from(Time.now)
expect(Ci::TriggerSchedule.last.next_run_at).to eq(next_time)
end
end
end
describe '#real_next_run' do
subject { trigger_schedule.real_next_run(worker_cron: worker_cron, worker_time_zone: worker_time_zone) }
......
......@@ -8,38 +8,36 @@ describe TriggerScheduleWorker do
end
context 'when there is a scheduled trigger within next_run_at' do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:trigger) { create(:ci_trigger, owner: user, project: project, ref: 'master') }
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, :force_triggable, trigger: trigger, project: project) }
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, :force_triggable) }
before do
worker.perform
end
it 'creates a new trigger request' do
expect(Ci::TriggerRequest.first.trigger_id).to eq(trigger.id)
expect(trigger_schedule.trigger.id).to eq(Ci::TriggerRequest.first.trigger_id)
end
it 'creates a new pipeline' do
expect(Ci::Pipeline.last.status).to eq('pending')
end
it 'schedules next_run_at' do
next_time = Ci::CronParser.new('0 1 * * *', 'Europe/Istanbul').next_time_from_now
it 'updates next_run_at' do
next_time = Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_time_zone).next_time_from(Time.now)
expect(Ci::TriggerSchedule.last.next_run_at).to eq(next_time)
end
end
context 'when there are no scheduled triggers within next_run_at' do
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) }
context 'when there is a scheduled trigger within next_run_at and a runnign pipeline' do
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, :force_triggable) }
before do
create(:ci_pipeline, project: trigger_schedule.project, ref: trigger_schedule.ref, status: 'running')
worker.perform
end
it 'do not create a new pipeline' do
expect(Ci::Pipeline.all).to be_empty
expect(Ci::Pipeline.count).to eq(1)
end
it 'do not reschedule next_run_at' do
......@@ -47,15 +45,15 @@ describe TriggerScheduleWorker do
end
end
context 'when next_run_at is nil' do
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build, next_run_at: nil) }
context 'when there are no scheduled triggers within next_run_at' do
let!(:trigger_schedule) { create(:ci_trigger_schedule, :cron_nightly_build) }
before do
worker.perform
end
it 'do not create a new pipeline' do
expect(Ci::Pipeline.all).to be_empty
expect(Ci::Pipeline.count).to eq(0)
end
it 'do not reschedule next_run_at' do
......
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