writes test suite

app/helpers/mirror_helper.rb

@@ -6,7 +6,7 @@ module MirrorHelper
   end
 
   def mirror_sync_time_options
-    Gitlab::Mirror.sync_time_options.select do |key, value|
+    Gitlab::Mirror::SYNC_TIME_OPTIONS.select do |key, value|
       value >= current_application_settings.minimum_mirror_sync_time
     end
   end

app/models/application_setting.rb

@@ -130,7 +130,7 @@ class ApplicationSetting < ActiveRecord::Base
 
   validates :minimum_mirror_sync_time,
             presence: true,
-            inclusion: { in: Gitlab::Mirror.sync_time_options.values }
+            inclusion: { in: Gitlab::Mirror::SYNC_TIME_OPTIONS.values }
 
   validates_each :restricted_visibility_levels do |record, attr, value|
     value&.each do |level|
@@ -231,7 +231,7 @@ class ApplicationSetting < ActiveRecord::Base
       elasticsearch_host: ENV['ELASTIC_HOST'] || 'localhost',
       elasticsearch_port: ENV['ELASTIC_PORT'] || '9200',
       usage_ping_enabled: true,
-      minimum_mirror_sync_time: 60
+      minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN
     }
   end
 

app/models/project.rb

@@ -218,7 +218,7 @@ class Project < ActiveRecord::Base
 
   validates :sync_time,
     presence: true,
-    inclusion: { in: Gitlab::Mirror.sync_time_options.values }
+    inclusion: { in: Gitlab::Mirror::SYNC_TIME_OPTIONS.values }
 
   with_options if: :mirror? do |project|
     project.validates :import_url, presence: true

app/models/remote_mirror.rb

@@ -14,7 +14,7 @@ class RemoteMirror < ActiveRecord::Base
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
   validates :sync_time,
     presence: true,
-    inclusion: { in: Gitlab::Mirror.sync_time_options.values }
+    inclusion: { in: Gitlab::Mirror::SYNC_TIME_OPTIONS.values }
 
   validate  :url_availability, if: -> (mirror) { mirror.url_changed? || mirror.enabled? }
 

app/views/admin/application_settings/_form.html.haml

@@ -74,7 +74,7 @@
     .form-group
       = f.label :minimum_mirror_sync_time, class: 'control-label col-sm-2'
       .col-sm-10
-        = f.select :minimum_mirror_sync_time, options_for_select(Gitlab::Mirror.sync_time_options, @application_setting.minimum_mirror_sync_time), {}, class: 'form-control'
+        = f.select :minimum_mirror_sync_time, options_for_select(Gitlab::Mirror::SYNC_TIME_OPTIONS, @application_setting.minimum_mirror_sync_time), {}, class: 'form-control'
     .form-group
       = f.label :session_expire_delay, 'Session duration (minutes)', class: 'control-label col-sm-2'
       .col-sm-10

db/migrate/20170210144005_add_global_minimum_mirror_sync_time_to_application_settings.rb

-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class AddGlobalMinimumMirrorSyncTimeToApplicationSettings < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
@@ -12,7 +9,7 @@ class AddGlobalMinimumMirrorSyncTimeToApplicationSettings < ActiveRecord::Migrat
     add_column_with_default :application_settings,
                             :minimum_mirror_sync_time,
                             :integer,
-                            default: 60,
+                            default: 15,
                             allow_null: false
   end
 

db/schema.rb

@@ -120,7 +120,7 @@ ActiveRecord::Schema.define(version: 20170211073944) do
     t.integer "shared_runners_minutes", default: 0, null: false
     t.integer "repository_size_limit", limit: 8, default: 0
     t.integer "terminal_max_session_time", default: 0, null: false
-    t.integer "minimum_mirror_sync_time", default: 60, null: false
+    t.integer "minimum_mirror_sync_time", default: 15, null: false
   end
 
   create_table "approvals", force: :cascade do |t|

lib/gitlab/mirror.rb

@@ -14,15 +14,13 @@ module Gitlab
       DAILY   => "0 0 * * *"
     }.freeze
 
-    class << self
-      def sync_time_options
-        {
+    SYNC_TIME_OPTIONS = {
       "Update every 15 minutes" => FIFTEEN,
       "Update hourly" => HOURLY,
       "Update every day" => DAILY,
-        }
-      end
+    }.freeze
 
+    class << self
       def sync_times
         sync_times = [FIFTEEN]
         sync_times << DAILY  if at_beginning_of_day?
@@ -32,7 +30,7 @@ module Gitlab
       end
 
       def configure_cron_jobs!
-        minimum_mirror_sync_time = current_application_settings.minimum_mirror_sync_time rescue DAILY
+        minimum_mirror_sync_time = current_application_settings.minimum_mirror_sync_time rescue FIFTEEN
         sync_time = SYNC_TIME_TO_CRON[minimum_mirror_sync_time]
         update_all_mirrors_worker_job = Sidekiq::Cron::Job.find("update_all_mirrors_worker")
         update_all_remote_mirrors_worker_job = Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker")

spec/controllers/projects/mirrors_controller_spec.rb

 require 'spec_helper'
 
 describe Projects::MirrorsController do
-  let(:sync_times) { Gitlab::Mirror.sync_time_options.values }
+  let(:sync_times) { Gitlab::Mirror::SYNC_TIME_OPTIONS.values }
 
   describe 'setting up a mirror' do
     context 'when the current project is a mirror' do

spec/lib/gitlab/mirror_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Mirror do
+  before { Sidekiq::Logging.logger = nil }
+
+  describe '#sync_times' do
+    describe 'at beginning of hour' do
+      before { Timecop.freeze(DateTime.now.at_beginning_of_hour) }
+
+      it 'returns only fifteen and hourly sync_times' do
+        expect(Gitlab::Mirror.sync_times).to contain_exactly(Gitlab::Mirror::FIFTEEN, Gitlab::Mirror::HOURLY)
+      end
+    end
+
+    describe 'at beginning of day' do
+      before { Timecop.freeze(DateTime.now.at_beginning_of_day) }
+
+      it 'returns daily hourly and fifteen sync_times' do
+        expect(Gitlab::Mirror.sync_times).to contain_exactly(Gitlab::Mirror::DAILY, Gitlab::Mirror::HOURLY, Gitlab::Mirror::FIFTEEN)
+      end
+    end
+
+    describe 'every fifteen minutes' do
+      before { Timecop.freeze(DateTime.now.at_beginning_of_hour + 15.minutes) }
+
+      it 'returns only fifteen minutes' do
+        expect(Gitlab::Mirror.sync_times).to contain_exactly(Gitlab::Mirror::FIFTEEN)
+      end
+    end
+
+    after { Timecop.return }
+  end
+
+  describe '#configure_cron_jobs!' do
+    let(:daily_cron) { Gitlab::Mirror::SYNC_TIME_TO_CRON[Gitlab::Mirror::DAILY] }
+    let(:hourly_cron) { Gitlab::Mirror::SYNC_TIME_TO_CRON[Gitlab::Mirror::HOURLY] }
+    let(:fifteen_cron) { Gitlab::Mirror::SYNC_TIME_TO_CRON[Gitlab::Mirror::FIFTEEN] }
+
+    describe 'with jobs already running' do
+      def setup_mirrors_cron_job(current, updated_time)
+        allow_any_instance_of(ApplicationSetting).to receive(:minimum_mirror_sync_time).and_return(current)
+        Gitlab::Mirror.configure_cron_jobs!
+        allow_any_instance_of(ApplicationSetting).to receive(:minimum_mirror_sync_time).and_return(updated_time)
+      end
+
+      describe 'with daily minimum_mirror_sync_time' do
+        before { setup_mirrors_cron_job(Gitlab::Mirror::HOURLY, Gitlab::Mirror::DAILY) }
+
+        it 'changes cron of update_all_mirrors_worker to daily' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron }.from(hourly_cron).to(daily_cron)
+        end
+
+        it 'changes cron of update_all_remote_mirrors_worker to daily' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron }.from(hourly_cron).to(daily_cron)
+        end
+      end
+
+      describe 'with hourly minimum_mirror_sync_time' do
+        before { setup_mirrors_cron_job(Gitlab::Mirror::DAILY, Gitlab::Mirror::HOURLY) }
+
+        it 'changes cron of update_all_mirrors_worker to daily' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron }.from(daily_cron).to(hourly_cron)
+        end
+
+        it 'changes cron of update_all_remote_mirrors_worker to daily' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron }.from(daily_cron).to(hourly_cron)
+        end
+      end
+
+      describe 'with fifteen minimum_mirror_sync_time' do
+        before { setup_mirrors_cron_job(Gitlab::Mirror::DAILY, Gitlab::Mirror::FIFTEEN) }
+
+        it 'changes cron of update_all_mirrors_worker to fifteen' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron }.from(daily_cron).to(fifteen_cron)
+        end
+
+        it 'changes cron of update_all_remote_mirrors_worker to fifteen' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron }.from(daily_cron).to(fifteen_cron)
+        end
+      end
+    end
+
+    describe 'without jobs already running' do
+      before do
+        Sidekiq::Cron::Job.find("update_all_mirrors_worker").destroy
+        Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").destroy
+      end
+
+      describe 'with daily minimum_mirror_sync_time' do
+        before { allow_any_instance_of(ApplicationSetting).to receive(:minimum_mirror_sync_time).and_return(Gitlab::Mirror::DAILY) }
+
+        it 'creates update_all_mirrors_worker with cron of daily sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron).to eq(daily_cron)
+        end
+
+        it 'creates update_all_remote_mirrors_worker with cron of daily sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron).to eq(daily_cron)
+        end
+      end
+
+      describe 'with hourly minimum_mirror_sync_time' do
+        before { allow_any_instance_of(ApplicationSetting).to receive(:minimum_mirror_sync_time).and_return(Gitlab::Mirror::HOURLY) }
+
+        it 'creates update_all_mirrors_worker with cron of hourly sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron).to eq(hourly_cron)
+        end
+
+        it 'creates update_all_remote_mirrors_worker with cron of hourly sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron).to eq(hourly_cron)
+        end
+      end
+
+      describe 'with fifteen minimum_mirror_sync_time' do
+        it 'creates update_all_mirrors_worker with cron of fifteen sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron).to eq(fifteen_cron)
+        end
+
+        it 'creates update_all_remote_mirrors_worker with cron of fifteen sync_time' do
+          expect { Gitlab::Mirror.configure_cron_jobs! }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker") }.from(nil).to(Sidekiq::Cron::Job)
+          expect(Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron).to eq(fifteen_cron)
+        end
+      end
+    end
+  end
+
+  describe '#at_beginning_of_day?' do
+    it 'returns true if at beginning_of_day' do
+      Timecop.freeze(DateTime.now.at_beginning_of_day)
+
+      expect(Gitlab::Mirror.at_beginning_of_day?).to be true
+    end
+
+    it 'returns false if at beginning of hour' do
+      Timecop.freeze(DateTime.now.at_beginning_of_hour)
+
+      expect(Gitlab::Mirror.at_beginning_of_day?).to be false
+    end
+
+    it 'returns false in every 15 minute mark' do
+      Timecop.freeze(DateTime.now.at_beginning_of_hour + 15.minutes)
+
+      expect(Gitlab::Mirror.at_beginning_of_day?).to be false
+    end
+
+    after { Timecop.return }
+  end
+
+  describe '#at_beginning_of_hour?' do
+    it 'returns true if at beginning of day' do
+      Timecop.freeze(DateTime.now.at_beginning_of_day)
+
+      expect(Gitlab::Mirror.at_beginning_of_day?).to be true
+    end
+
+    it 'returns true if at beginning of hour' do
+      Timecop.freeze(DateTime.now.at_beginning_of_hour)
+
+      expect(Gitlab::Mirror.at_beginning_of_hour?).to be true
+    end
+
+    it 'returns false in every 15 minute mark' do
+      Timecop.freeze(DateTime.now.at_beginning_of_hour + 15.minutes)
+
+      expect(Gitlab::Mirror.at_beginning_of_hour?).to be false
+    end
+
+    after { Timecop.return }
+  end
+end

spec/models/application_setting_spec.rb

@@ -47,21 +47,71 @@ describe ApplicationSetting, models: true do
       subject { setting }
     end
 
-    context "update minimum_mirror_cron_jobs" do
-      let(:daily_cron)  { Gitlab::Mirror::SYNC_TIME_TO_CRON[Gitlab::Mirror::DAILY] }
-      let(:hourly_cron) { Gitlab::Mirror::SYNC_TIME_TO_CRON[Gitlab::Mirror::HOURLY] }
-
+    context "update minimum_mirror_sync_time" do
       before do
-        Gitlab::Mirror.configure_cron_jobs!
-        allow_any_instance_of(Gitlab::CurrentSettings).to receive(:current_application_settings).and_return(setting)
+        Sidekiq::Logging.logger = nil
+        Gitlab::Mirror::SYNC_TIME_TO_CRON.keys.each do |sync_time|
+          create(:project, :mirror, sync_time: sync_time)
+          create(:project, :remote_mirror, sync_time: sync_time)
+        end
       end
 
-      it "changes update_all_mirrors_worker cron" do
-        expect { setting.update_attributes(minimum_mirror_sync_time: Gitlab::Mirror::DAILY) }.to change { Sidekiq::Cron::Job.find("update_all_mirrors_worker").cron }.from(hourly_cron).to(daily_cron)
+      context 'with daily sync_time' do
+        let(:sync_time) { Gitlab::Mirror::DAILY }
+
+        it 'updates minimum_mirror_sync_time to daily and updates cron jobs' do
+          expect_any_instance_of(ApplicationSetting).to receive(:update_mirror_cron_jobs).and_call_original
+          expect(Gitlab::Mirror).to receive(:configure_cron_jobs!)
+
+          setting.update_attributes(minimum_mirror_sync_time: sync_time)
+        end
+
+        it 'updates every mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { Project.mirror.where('sync_time < ?', sync_time).count }.from(2).to(0)
+        end
+
+        it 'updates every remote mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { RemoteMirror.where('sync_time < ?', sync_time).count }.from(2).to(0)
+        end
+      end
+
+      context 'with hourly sync time' do
+        let(:sync_time) { Gitlab::Mirror::HOURLY }
+
+        it 'updates minimum_mirror_sync_time to daily and updates cron jobs' do
+          expect_any_instance_of(ApplicationSetting).to receive(:update_mirror_cron_jobs).and_call_original
+          expect(Gitlab::Mirror).to receive(:configure_cron_jobs!)
+
+          setting.update_attributes(minimum_mirror_sync_time: sync_time)
         end
 
-      it "changes update_all_remote_mirrors_worker cron" do
-        expect { setting.update_attributes(minimum_mirror_sync_time: Gitlab::Mirror::DAILY) }.to change { Sidekiq::Cron::Job.find("update_all_remote_mirrors_worker").cron }.from(hourly_cron).to(daily_cron)
+        it 'updates every mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { Project.mirror.where('sync_time < ?', sync_time).count }.from(1).to(0)
+        end
+
+        it 'updates every remote mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.to change { RemoteMirror.where('sync_time < ?', sync_time).count }.from(1).to(0)
+        end
+      end
+
+      context 'with default fifteen sync time' do
+        let(:sync_time) { Gitlab::Mirror::FIFTEEN }
+
+        it 'does not update minimum_mirror_sync_time' do
+          expect_any_instance_of(ApplicationSetting).not_to receive(:update_mirror_cron_jobs)
+          expect(Gitlab::Mirror).not_to receive(:configure_cron_jobs!)
+          expect(setting.minimum_mirror_sync_time).to eq(Gitlab::Mirror::FIFTEEN)
+
+          setting.update_attributes(minimum_mirror_sync_time: sync_time)
+        end
+
+        it 'updates every mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.not_to change { Project.mirror.where('sync_time < ?', sync_time).count }
+        end
+
+        it 'updates every remote mirror to the current minimum_mirror_sync_time' do
+          expect { setting.update_attributes(minimum_mirror_sync_time: sync_time) }.not_to change { RemoteMirror.where('sync_time < ?', sync_time).count }
+        end
       end
     end