Improve Specs and some fixes

6 files changed, 198 insertions, 8 deletions

diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb
index aff830f54d7..63b7eea5add 100644
--- a/lib/system_check/base_check.rb
+++ b/lib/system_check/base_check.rb
@@ -1,9 +1,11 @@
+require 'tasks/gitlab/task_helpers'
+
 module SystemCheck
   # Base class for Checks. You must inherit from here
   # and implement the methods below when necessary
   class BaseCheck
     include ::Gitlab::TaskHelpers
-    include Helpers
+    include ::SystemCheck::Helpers
 
     # Define a custom term for when check passed
     #
diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb
index 7a5042047ab..ad2f549b3fb 100644
--- a/lib/system_check/simple_executor.rb
+++ b/lib/system_check/simple_executor.rb
@@ -24,17 +24,17 @@ module SystemCheck
 
       c = check.new
 
-      # When implements a multi check, we don't control the output
-      if c.is_multi_check?
-        c.multi_check
-        return
-      end
-
       # When implements skip method, we run it first, and if true, skip the check
       if c.can_skip? && c.skip?
         $stdout.puts check.skip_reason.color(:magenta)
         return
       end
+      
+      # When implements a multi check, we don't control the output
+      if c.is_multi_check?
+        c.multi_check
+        return
+      end
 
       if c.check?
         $stdout.puts check.check_pass.color(:green)
diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb
index e38e21b149f..e3c9d3b491c 100644
--- a/lib/tasks/gitlab/task_helpers.rb
+++ b/lib/tasks/gitlab/task_helpers.rb
@@ -113,7 +113,6 @@ module Gitlab
       end
     end
 
-    # TODO: MIGRATED
     # Tries to configure git itself
     #
     # Returns true if all subcommands were successfull (according to their exit code)
diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb
new file mode 100644
index 00000000000..f6ba437b71a
--- /dev/null
+++ b/spec/lib/system_check/simple_executor_spec.rb
@@ -0,0 +1,179 @@
+require 'spec_helper'
+require 'rake_helper'
+
+describe SystemCheck::SimpleExecutor, lib: true do
+  class SimpleCheck < SystemCheck::BaseCheck
+    set_name 'my simple check'
+
+    def check?
+      true
+    end
+  end
+
+  class OtherCheck < SystemCheck::BaseCheck
+    set_name 'other check'
+
+    def check?
+      false
+    end
+
+    def show_error
+      $stdout.puts 'this is an error text'
+    end
+  end
+
+  class SkipCheck < SystemCheck::BaseCheck
+    set_name 'skip check'
+    set_skip_reason 'this is a skip reason'
+
+    def skip?
+      true
+    end
+
+    def check?
+      raise 'should not execute this'
+    end
+  end
+
+  class MultiCheck < SystemCheck::BaseCheck
+    set_name 'multi check'
+
+    def multi_check
+      $stdout.puts 'this is a multi output check'
+    end
+
+    def check?
+      raise 'should not execute this'
+    end
+  end
+
+  class SkipMultiCheck < SystemCheck::BaseCheck
+    set_name 'skip multi check'
+
+    def skip?
+      true
+    end
+
+    def multi_check
+      raise 'should not execute this'
+    end
+  end
+
+  class RepairCheck < SystemCheck::BaseCheck
+    set_name 'repair check'
+
+    def check?
+      false
+    end
+
+    def repair!
+      true
+    end
+
+    def show_error
+      $stdout.puts 'this is an error message'
+    end
+  end
+
+  subject { described_class.new('Test') }
+
+  describe '#execute' do
+    before do
+      silence_output
+
+      subject << SimpleCheck
+      subject << OtherCheck
+    end
+
+    it 'runs included checks' do
+      expect(subject).to receive(:run_check).with(SimpleCheck)
+      expect(subject).to receive(:run_check).with(OtherCheck)
+
+      subject.execute
+    end
+  end
+
+  describe '#run_check' do
+    it 'prints check name' do
+      expect(SimpleCheck).to receive(:display_name).and_call_original
+      expect { subject.run_check(SimpleCheck) }.to output(/my simple check/).to_stdout
+    end
+
+    context 'when check pass' do
+      it 'prints yes' do
+        expect_any_instance_of(SimpleCheck).to receive(:check?).and_call_original
+        expect { subject.run_check(SimpleCheck) }.to output(/ \.\.\. yes/).to_stdout
+      end
+    end
+
+    context 'when check fails' do
+      it 'prints no' do
+        expect_any_instance_of(OtherCheck).to receive(:check?).and_call_original
+        expect { subject.run_check(OtherCheck) }.to output(/ \.\.\. no/).to_stdout
+      end
+
+      it 'displays error message from #show_error' do
+        expect_any_instance_of(OtherCheck).to receive(:show_error).and_call_original
+        expect { subject.run_check(OtherCheck) }.to output(/this is an error text/).to_stdout
+      end
+
+      context 'when check implements #repair!' do
+        it 'executes #repair!' do
+          expect_any_instance_of(RepairCheck).to receive(:repair!)
+          subject.run_check(RepairCheck)
+        end
+
+        context 'when repair succeeds' 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
+
+        context 'when repair fails' 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
+      end
+    end
+
+    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
+
+      it 'displays #skip_reason' do
+        expect { subject.run_check(SkipCheck) }.to output(/this is a skip reason/).to_stdout
+      end
+
+      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
+
+    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
+    end
+  end
+end
diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb
index 399a492ea2e..d2087ad2d83 100644
--- a/spec/lib/system_check_spec.rb
+++ b/spec/lib/system_check_spec.rb
@@ -1,8 +1,13 @@
 require 'spec_helper'
+require 'rake_helper'
 
 describe SystemCheck, lib: true do
   subject { SystemCheck }
 
+  before do
+    silence_output
+  end
+
   describe '.run' do
     it 'requires custom executor to be a BasicExecutor' do
       expect { subject.run('Component', [], SystemCheck::SimpleExecutor) }.not_to raise_error
diff --git a/spec/support/rake_helpers.rb b/spec/support/rake_helpers.rb
index 4a8158ed79b..5cb415111d2 100644
--- a/spec/support/rake_helpers.rb
+++ b/spec/support/rake_helpers.rb
@@ -7,4 +7,9 @@ module RakeHelpers
   def stub_warn_user_is_not_gitlab
     allow_any_instance_of(Object).to receive(:warn_user_is_not_gitlab)
   end
+
+  def silence_output
+    allow($stdout).to receive(:puts)
+    allow($stdout).to receive(:print)
+  end
 end