Start spec test for API v1 config

Kevin Paulisse · Kevin Paulisse · commit e42f4305ea08 · 2017-01-07T19:45:03.000-06:00
diff --git a/lib/octocatalog-diff/api/v1/config.rb b/lib/octocatalog-diff/api/v1/config.rb
@@ -7,6 +7,10 @@ module API
     module V1
       # This class interacts with the configuration file typically named `.octocatalog-diff.cfg.rb`.
       class Config
+        # Error class for handled configuration file errors
+        class ConfigurationFileNotFoundError < RuntimeError; end
+        class ConfigurationFileContentError < RuntimeError; end
+
         # Default directory paths: These are the documented default locations that will be checked
         # for the configuration file.
         DEFAULT_PATHS = [
@@ -24,7 +28,7 @@ class Config
         #
         # @param :filename [String] Specified file name (default = search the default paths)
         # @param :logger [Logger] Logger object
-        # @param :test [Boolean] Configuration file test mode only (test then exit)
+        # @param :test [Boolean] Configuration file test mode (log some extra debugging, raises errors)
         # @return [Hash] Parsed configuration file
         def self.config(options_in = {})
           # Initialize the logger - if not passed, set to a throwaway object.
@@ -33,23 +37,20 @@ def self.config(options_in = {})
           # Locate the configuration file
           paths = [options.fetch(:filename, DEFAULT_PATHS)].compact
           config_file = first_file(paths)
+
+          # Can't find the configuration file?
           if config_file.nil?
             message = "Unable to find configuration file in #{paths.join(':')}"
-            raise Errno::ENOENT, message if options[:test]
+            raise ConfigurationFileNotFoundError, message if options[:test]
             logger.debug message
             return {}
           end
 
-          # Load the configuration file
+          # Load/parse the configuration file - this returns a hash
           settings = load_config_file(config_file, logger)
-          raise 'Configuration file failed to return a hash' unless settings.is_a?(Hash)
 
           # Debug the configuration file if requested.
-          if options[:test]
-            debug_config_file(settings, logger)
-            logger.info 'Exiting now because --config-test was specified'
-            exit 0
-          end
+          debug_config_file(settings, logger) if options[:test]
 
           # Return the settings hash
           logger.debug "Loaded #{settings.keys.size} settings from #{config_file}"
@@ -61,9 +62,11 @@ def self.config(options_in = {})
         # @param settings [Hash] Parsed settings from load_config_file
         # @param logger [Logger] Logger object
         def self.debug_config_file(settings, logger)
-          settings.each do |key, val|
-            logger.debug ":#{key} => (#{val.class}) #{val.inspect}"
+          unless settings.is_a?(Hash)
+            raise ArgumentError, "Settings must be hash not #{settings.class}"
           end
+
+          settings.each { |key, val| logger.debug ":#{key} => (#{val.class}) #{val.inspect}" }
         end
 
         # Private: Load the configuration file from a given path. Returns the settings hash.
@@ -72,25 +75,38 @@ def self.debug_config_file(settings, logger)
         # @param logger [Logger] Logger object
         # @return [Hash] Settings
         def self.load_config_file(filename, logger)
+          # This should never happen unless somebody calls this method directly outside of
+          # the published `.config` method. Check for problems anyway.
+          raise Errno::ENOENT, "File #{filename} doesn't exist" unless File.file?(filename)
+
+          # Attempt to require in the file. Problems here will fall through to the rescued
+          # exception below.
           logger.debug "Loading octocatalog-diff configuration from #{filename}"
-          require filename
+          load filename
 
+          # The required file should contain `OctocatalogDiff::Config` with `.config` method.
+          # If this is undefined, raise an exception.
           begin
-            options = OctocatalogDiff::Config.config
-          rescue => exc
-            logger.fatal "#{exc.class} error with #{filename}: #{exc.message}\n#{exc.backtrace}"
-            exit 1
+            loaded_class = Kernel.const_get(:OctocatalogDiff).const_get(:Config)
+          rescue NameError
+            raise ConfigurationFileContentError, 'Configuration must define OctocatalogDiff::Config!'
+          end
+
+          unless loaded_class.respond_to?(:config)
+            raise ConfigurationFileContentError, 'Configuration must define OctocatalogDiff::Config.config!'
           end
 
+          # The configuration file looks like it defines the correct method, so read it.
+          # Make sure it's a hash.
+          options = loaded_class.config
           unless options.is_a?(Hash)
-            logger.fatal "Configuration must be Hash not #{options.class}!"
-            exit 1
+            raise ConfigurationFileContentError, "Configuration must be Hash not #{options.class}!"
           end
 
           options
-        rescue => exc
+        rescue Exception => exc # rubocop:disable Lint/RescueException
           logger.fatal "#{exc.class} error with #{filename}: #{exc.message}\n#{exc.backtrace}"
-          raise 'Unable to load octocatalog-diff configuration file'
+          raise exc
         end
 
         # Private: Find the first element of the given array that is a file and return it.
diff --git a/spec/octocatalog-diff/fixtures/cli-configs/not-hash.rb b/spec/octocatalog-diff/fixtures/cli-configs/not-hash.rb
@@ -0,0 +1,9 @@
+# This is a configuration file for octocatalog-diff
+
+module OctocatalogDiff
+  class Config
+    def self.config
+      %w(apple banana)
+    end
+  end
+end
diff --git a/spec/octocatalog-diff/fixtures/cli-configs/not-proper.rb b/spec/octocatalog-diff/fixtures/cli-configs/not-proper.rb
@@ -0,0 +1,13 @@
+# This is a configuration file for octocatalog-diff
+
+module OctocatalogDiff
+  class Config
+    def self.foobuzz
+      {
+        header: :default,
+        hiera_config: 'config/hiera.yaml',
+        hiera_path: 'hieradata'
+      }
+    end
+  end
+end
diff --git a/spec/octocatalog-diff/fixtures/cli-configs/not-ruby.rb b/spec/octocatalog-diff/fixtures/cli-configs/not-ruby.rb
@@ -0,0 +1,3 @@
+Ruby is not here.
+It should fail to compile.
+And raise an error.
diff --git a/spec/octocatalog-diff/tests/api/v1/config_spec.rb b/spec/octocatalog-diff/tests/api/v1/config_spec.rb
@@ -0,0 +1,151 @@
+# frozen_string_literal: true
+
+require 'ostruct'
+
+require_relative '../../spec_helper'
+
+require OctocatalogDiff::Spec.require_path('/api/v1/config')
+
+describe OctocatalogDiff::API::V1::Config do
+  before(:each) do
+    @logger, @logger_str = OctocatalogDiff::Spec.setup_logger
+  end
+
+  after(:each) do
+    begin
+      OctocatalogDiff.send(:remove_const, :Config)
+    rescue NameError # rubocop:disable Lint/HandleExceptions
+      # Don't care if it's not defined already
+    end
+  end
+
+  describe '#config' do
+  end
+
+  describe '#debug_config_file' do
+    it 'should raise error if non-hash is passed in' do
+      expect do
+        described_class.debug_config_file(nil, @logger)
+      end.to raise_error(ArgumentError, /Settings must be hash not NilClass/)
+    end
+
+    it 'should log keys and values in the hash' do
+      described_class.debug_config_file({ foo: 'bar', baz: ['buzz'] }, @logger)
+      expect(@logger_str.string).to match(/:foo => \(String\) "bar"/)
+      expect(@logger_str.string).to match(/:baz => \(Array\) \["buzz"\]/)
+    end
+  end
+
+  describe '#load_config_file' do
+    context 'with a non-existent file' do
+      let(:filename) { OctocatalogDiff::Spec.fixture_path('this-does-not-exist.rb') }
+
+      it 'should raise Errno::ENOENT' do
+        expect do
+          described_class.load_config_file(filename, @logger)
+        end.to raise_error(Errno::ENOENT, /File.+doesn't exist/)
+      end
+
+      it 'should log fatal message' do
+        exception = nil
+        begin
+          described_class.load_config_file(filename, @logger)
+        rescue Errno::ENOENT => exc
+          exception = exc
+        end
+        expect(exception.message).to match(/this-does-not-exist.rb doesn't exist/)
+        expect(@logger_str.string).to match(/FATAL .+ Errno::ENOENT error with .+this-does-not-exist.rb/)
+      end
+    end
+
+    context 'with an invalid file' do
+      let(:filename) { OctocatalogDiff::Spec.fixture_path('cli-configs/not-ruby.rb') }
+
+      it 'should raise SyntaxError' do
+        expect do
+          described_class.load_config_file(filename, @logger)
+        end.to raise_error(SyntaxError)
+      end
+
+      it 'should log fatal message' do
+        exception = nil
+        begin
+          described_class.load_config_file(filename, @logger)
+        rescue SyntaxError => exc
+          exception = exc
+        end
+        expect(exception).to be_a_kind_of(SyntaxError)
+        expect(exception.message).to match(/unexpected tIDENTIFIER/)
+        expect(@logger_str.string).to match(/DEBUG -- : Loading octocatalog-diff configuration from/)
+        expect(@logger_str.string).to match(/FATAL .+ SyntaxError error with .+not-ruby.rb/)
+      end
+    end
+
+    context 'with a file that throws an exception' do
+      let(:filename) { OctocatalogDiff::Spec.fixture_path('cli-configs/invalid.rb') }
+
+      it 'should raise RuntimeError' do
+        expect do
+          described_class.load_config_file(filename, @logger)
+        end.to raise_error(RuntimeError, /Fizz Buzz/)
+      end
+
+      it 'should log fatal message' do
+        exception = nil
+        begin
+          described_class.load_config_file(filename, @logger)
+        rescue RuntimeError => exc
+          exception = exc
+        end
+        expect(exception).to be_a_kind_of(RuntimeError)
+        expect(exception.message).to eq('Fizz Buzz')
+        expect(@logger_str.string).to match(/DEBUG -- : Loading octocatalog-diff configuration from/)
+        expect(@logger_str.string).to match(/FATAL .+ RuntimeError error with .+invalid.rb/)
+      end
+    end
+
+    context 'with a file that does not define a .config method' do
+      let(:filename) { OctocatalogDiff::Spec.fixture_path('cli-configs/not-proper.rb') }
+
+      it 'should raise ConfigurationFileContentError' do
+        pattern = Regexp.new('must define OctocatalogDiff::Config.config!')
+        expect do
+          described_class.load_config_file(filename, @logger)
+        end.to raise_error(OctocatalogDiff::API::V1::Config::ConfigurationFileContentError, pattern)
+      end
+
+      it 'should log fatal message' do
+        exception = nil
+        begin
+          described_class.load_config_file(filename, @logger)
+        rescue OctocatalogDiff::API::V1::Config::ConfigurationFileContentError => exc
+          exception = exc
+        end
+        expect(exception).to be_a_kind_of(OctocatalogDiff::API::V1::Config::ConfigurationFileContentError)
+        expect(exception.message).to eq('Configuration must define OctocatalogDiff::Config.config!')
+        expect(@logger_str.string).to match(/Configuration must define OctocatalogDiff::Config.config!/)
+      end
+    end
+
+    context 'with a file that does not define a hash' do
+    end
+
+    context 'with a valid file' do
+      let(:filename) { OctocatalogDiff::Spec.fixture_path('cli-configs/valid.rb') }
+
+      it 'should return the expected settings' do
+        result = described_class.load_config_file(filename, @logger)
+        answer = { header: :default, hiera_config: 'config/hiera.yaml', hiera_path: 'hieradata' }
+        expect(result).to eq(answer)
+      end
+
+      it 'should log debug message' do
+        described_class.load_config_file(filename, @logger)
+        expect(@logger_str.string).to match(/DEBUG -- : Loading octocatalog-diff configuration from/)
+      end
+    end
+  end
+
+  describe '#first_file' do
+  end
+end

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Ruby is not here.`
	`2`	`+It should fail to compile.`
	`3`	`+And raise an error.`