From 4dd0c96a78a1848f32dff68778a625c7e34ba915 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 17 Feb 2017 11:17:17 -0800 Subject: [PATCH] (POOLER-73) Add spec tests for check_disk_queue Add spec tests for check_disk_queue Previously the check_disk_queue method would execute the loop indefinitely as it did not have a terminating condition. This made it impossible to test. This commit modifies the check_disk_queue method so that it can take a maxloop and delay parameter so that it can be tested. --- lib/vmpooler/pool_manager.rb | 11 ++++-- spec/unit/pool_manager_spec.rb | 66 ++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 2892adc..aa89fc1 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -412,15 +412,20 @@ module Vmpooler end end - def check_disk_queue + def check_disk_queue(maxloop = 0, loop_delay = 5) $logger.log('d', "[*] [disk_manager] starting worker thread") $vsphere['disk_manager'] ||= Vmpooler::VsphereHelper.new $config, $metrics - $threads['disk_manager'] = Thread.new do + loop_count = 1 loop do _check_disk_queue $vsphere['disk_manager'] - sleep(5) + sleep(loop_delay) + + unless maxloop.zero? + break if loop_count >= maxloop + loop_count = loop_count + 1 + end end end end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 375a355..36cd9b3 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -955,6 +955,72 @@ EOT end end + describe '#check_disk_queue' do + let(:threads) {[]} + + before(:each) do + expect(Thread).to receive(:new).and_yield + allow(subject).to receive(:_check_disk_queue) + end + + it 'should log the disk manager is starting' do + expect(logger).to receive(:log).with('d', "[*] [disk_manager] starting worker thread") + + expect($threads.count).to be(0) + subject.check_disk_queue(1,0) + expect($threads.count).to be(1) + end + + it 'should add the manager to the global thread list' do + # Note - Ruby core types are not necessarily thread safe + expect($threads.count).to be(0) + subject.check_disk_queue(1,0) + expect($threads.count).to be(1) + end + + it 'should call _check_disk_queue' do + expect(subject).to receive(:_check_disk_queue).with(Vmpooler::VsphereHelper) + + subject.check_disk_queue(1,0) + end + + context 'delays between loops' do + let(:maxloop) { 2 } + let(:loop_delay) { 1 } + # Note a maxloop of zero can not be tested as it never terminates + + it 'defaults to 5 second loop delay' do + expect(subject).to receive(:sleep).with(5).exactly(maxloop).times + subject.check_disk_queue(maxloop) + end + + it 'when a non-default loop delay is specified' do + start_time = Time.now + subject.check_disk_queue(maxloop,loop_delay) + finish_time = Time.now + + # Use a generous delta to take into account various CPU load etc. + expect(finish_time - start_time).to be_within(0.75).of(maxloop * loop_delay) + end + end + + context 'loops specified number of times (5)' do + let(:maxloop) { 5 } + # Note a maxloop of zero can not be tested as it never terminates + + after(:each) do + # Reset the global variable - Note this is a code smell + $threads = nil + end + + it 'should call _check_disk_queue 5 times' do + expect(subject).to receive(:_check_disk_queue).with(Vmpooler::VsphereHelper).exactly(maxloop).times + + subject.check_disk_queue(maxloop,0) + end + end + end + describe '#migration_limit' do # This is a little confusing. Is this supposed to return a boolean # or integer type?