From 0d4d9a6a63b2ac60ffeeeaef389295919c3119f4 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 31 May 2017 20:45:00 +0200 Subject: [PATCH] Refactor and move things around to improve in YAGNI perspective --- lib/system_check/base_executor.rb | 24 --------- lib/system_check/simple_executor.rb | 24 ++++++++- spec/lib/system_check/base_executor_spec.rb | 53 ------------------- spec/lib/system_check/simple_executor_spec.rb | 44 +++++++++++++++ spec/lib/system_check_spec.rb | 42 +++++++-------- spec/support/matchers/execute_check.rb | 6 ++- 6 files changed, 91 insertions(+), 102 deletions(-) delete mode 100644 lib/system_check/base_executor.rb delete mode 100644 spec/lib/system_check/base_executor_spec.rb diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb deleted file mode 100644 index f76f44f3a28..00000000000 --- a/lib/system_check/base_executor.rb +++ /dev/null @@ -1,24 +0,0 @@ -module SystemCheck - # @attr_reader [Array] checks classes of corresponding checks to be executed in the same order - # @attr_reader [String] component name of the component relative to the checks being executed - class BaseExecutor - attr_reader :checks - attr_reader :component - - # @param [String] component name of the component relative to the checks being executed - def initialize(component) - raise ArgumentError unless component.is_a? String - - @component = component - @checks = Set.new - end - - # Add a check to be executed - # - # @param [BaseCheck] check class - def <<(check) - raise ArgumentError unless check < BaseCheck - @checks << check - end - end -end diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index c5403693f7a..dc2d4643a01 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -4,7 +4,29 @@ module SystemCheck # # There is no concurrency level and the output is progressively # printed into the STDOUT - class SimpleExecutor < BaseExecutor + # + # @attr_reader [Array] checks classes of corresponding checks to be executed in the same order + # @attr_reader [String] component name of the component relative to the checks being executed + class SimpleExecutor + attr_reader :checks + attr_reader :component + + # @param [String] component name of the component relative to the checks being executed + def initialize(component) + raise ArgumentError unless component.is_a? String + + @component = component + @checks = Set.new + end + + # Add a check to be executed + # + # @param [BaseCheck] check class + def <<(check) + raise ArgumentError unless check < BaseCheck + @checks << check + end + # Executes defined checks in the specified order and outputs confirmation or error information def execute start_checking(component) diff --git a/spec/lib/system_check/base_executor_spec.rb b/spec/lib/system_check/base_executor_spec.rb deleted file mode 100644 index 4b379392da0..00000000000 --- a/spec/lib/system_check/base_executor_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'spec_helper' - -describe SystemCheck::BaseExecutor, lib: true do - class SimpleCheck < SystemCheck::BaseCheck - def check? - true - end - end - - class OtherCheck < SystemCheck::BaseCheck - def check? - false - end - end - - subject { described_class.new('Test') } - - describe '#component' do - it 'returns stored component name' do - expect(subject.component).to eq('Test') - end - end - - describe '#checks' do - before do - subject << SimpleCheck - end - - it 'returns a set of classes' do - expect(subject.checks).to include(SimpleCheck) - end - end - - describe '#<<' do - before do - subject << SimpleCheck - end - - it 'appends a new check to the Set' do - subject << OtherCheck - stored_checks = subject.checks.to_a - - expect(stored_checks.first).to eq(SimpleCheck) - expect(stored_checks.last).to eq(OtherCheck) - end - - it 'inserts unique itens only' do - subject << SimpleCheck - - expect(subject.checks.size).to eq(1) - end - end -end diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb index f6ba437b71a..a5c6170cd7d 100644 --- a/spec/lib/system_check/simple_executor_spec.rb +++ b/spec/lib/system_check/simple_executor_spec.rb @@ -75,6 +75,42 @@ describe SystemCheck::SimpleExecutor, lib: true do end end + describe '#component' do + it 'returns stored component name' do + expect(subject.component).to eq('Test') + end + end + + describe '#checks' do + before do + subject << SimpleCheck + end + + it 'returns a set of classes' do + expect(subject.checks).to include(SimpleCheck) + end + end + + describe '#<<' do + before do + subject << SimpleCheck + end + + it 'appends a new check to the Set' do + subject << OtherCheck + stored_checks = subject.checks.to_a + + expect(stored_checks.first).to eq(SimpleCheck) + expect(stored_checks.last).to eq(OtherCheck) + end + + it 'inserts unique itens only' do + subject << SimpleCheck + + expect(subject.checks.size).to eq(1) + end + end + subject { described_class.new('Test') } describe '#execute' do @@ -120,6 +156,7 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when check implements #repair!' do it 'executes #repair!' do expect_any_instance_of(RepairCheck).to receive(:repair!) + subject.run_check(RepairCheck) end @@ -127,6 +164,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #show_error' do expect_any_instance_of(RepairCheck).to receive(:repair!).and_call_original expect_any_instance_of(RepairCheck).not_to receive(:show_error) + subject.run_check(RepairCheck) end end @@ -135,6 +173,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #show_error' do expect_any_instance_of(RepairCheck).to receive(:repair!) { false } expect_any_instance_of(RepairCheck).to receive(:show_error) + subject.run_check(RepairCheck) end end @@ -144,6 +183,7 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when check implements skip?' do it 'executes #skip? method' do expect_any_instance_of(SkipCheck).to receive(:skip?).and_call_original + subject.run_check(SkipCheck) end @@ -153,6 +193,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #check when #skip? is true' do expect_any_instance_of(SkipCheck).not_to receive(:check?) + subject.run_check(SkipCheck) end end @@ -160,17 +201,20 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when implements a #multi_check' do it 'executes #multi_check method' do expect_any_instance_of(MultiCheck).to receive(:multi_check) + subject.run_check(MultiCheck) end it 'does not execute #check method' do expect_any_instance_of(MultiCheck).not_to receive(:check) + subject.run_check(MultiCheck) end context 'when check implements #skip?' do it 'executes #skip? method' do expect_any_instance_of(SkipMultiCheck).to receive(:skip?).and_call_original + subject.run_check(SkipMultiCheck) end end diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb index 01679282f87..23d9beddb08 100644 --- a/spec/lib/system_check_spec.rb +++ b/spec/lib/system_check_spec.rb @@ -2,39 +2,35 @@ require 'spec_helper' require 'rake_helper' describe SystemCheck, lib: true do - subject { SystemCheck } + class SimpleCheck < SystemCheck::BaseCheck + def check? + true + end + end + + class OtherCheck < SystemCheck::BaseCheck + def check? + false + end + end before do silence_output end describe '.run' do - context 'custom matcher' do - class SimpleCheck < SystemCheck::BaseCheck - def check? - true - end - end + subject { SystemCheck } - class OtherCheck < SystemCheck::BaseCheck - def check? - false - end - end + it 'detects execution of SimpleCheck' do + is_expected.to execute_check(SimpleCheck) - subject { SystemCheck } + subject.run('Test', [SimpleCheck]) + end - it 'detects execution of SimpleCheck' do - is_expected.to execute_check(SimpleCheck) + it 'detects exclusion of OtherCheck in execution' do + is_expected.not_to execute_check(OtherCheck) - SystemCheck.run('Test', [SimpleCheck]) - end - - it 'detects exclusion of OtherCheck in execution' do - is_expected.not_to execute_check(OtherCheck) - - SystemCheck.run('Test', [SimpleCheck]) - end + subject.run('Test', [SimpleCheck]) end end end diff --git a/spec/support/matchers/execute_check.rb b/spec/support/matchers/execute_check.rb index 9664eb3879d..7232fad52fb 100644 --- a/spec/support/matchers/execute_check.rb +++ b/spec/support/matchers/execute_check.rb @@ -14,6 +14,10 @@ RSpec::Matchers.define :execute_check do |expected| end failure_message do |actual| - return 'This matcher must be used with SystemCheck' unless actual == SystemCheck + 'This matcher must be used with SystemCheck' unless actual == SystemCheck + end + + failure_message_when_negated do |actual| + 'This matcher must be used with SystemCheck' unless actual == SystemCheck end end