diff --git a/lib/octocatalog-diff/cli.rb b/lib/octocatalog-diff/cli.rb index 70603a78..04f3084a 100644 --- a/lib/octocatalog-diff/cli.rb +++ b/lib/octocatalog-diff/cli.rb @@ -116,8 +116,8 @@ def self.cli(argv = ARGV, logger = Logger.new(STDERR), opts = {}) printer_obj = OctocatalogDiff::Cli::Printer.new(options, logger) printer_obj.printer(diffs, catalog_diff.from.compilation_dir, catalog_diff.to.compilation_dir) - # Return the diff object if requested (generally for testing) or otherwise return exit code - return diffs if opts[:RETURN_DIFFS] + # Return the resulting diff object if requested (generally for testing) or otherwise return exit code + return catalog_diff if opts[:INTEGRATION] diffs.any? ? EXITCODE_SUCCESS_WITH_DIFFS : EXITCODE_SUCCESS_NO_DIFFS end @@ -169,11 +169,8 @@ def self.catalog_only(logger, options) puts to_catalog.to_json end - return EXITCODE_SUCCESS_NO_DIFFS unless options[:RETURN_DIFFS] # For integration testing - # :nocov: - dummy_catalog = OctocatalogDiff::API::V1::Catalog.new(OctocatalogDiff::Catalog.new(backend: :noop)) - [dummy_catalog, to_catalog] - # :nocov: + return { exitcode: EXITCODE_SUCCESS_NO_DIFFS, to: to_catalog } if options[:INTEGRATION] # For integration testing + EXITCODE_SUCCESS_NO_DIFFS end # --bootstrap-then-exit command diff --git a/spec/octocatalog-diff/integration/catalog_only_spec.rb b/spec/octocatalog-diff/integration/catalog_only_spec.rb index 0f416008..1c516bd9 100644 --- a/spec/octocatalog-diff/integration/catalog_only_spec.rb +++ b/spec/octocatalog-diff/integration/catalog_only_spec.rb @@ -26,25 +26,18 @@ it 'should compile the catalog' do expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result) - expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}" - end - - it 'should set the from-catalog to a no-op catalog type' do - pending 'catalog compilation failed' unless @result[:exitcode] == 2 - from_catalog = @result[:diffs][0] - expect(from_catalog).to be_a_kind_of(OctocatalogDiff::API::V1::Catalog) - expect(from_catalog.builder).to eq('OctocatalogDiff::Catalog::Noop') + expect(@result[:exitcode]).to eq(0), "Runtime error: #{@result[:logs]}" end it 'should set the to-catalog to a computed catalog type' do - pending 'catalog compilation failed' unless @result[:exitcode] == 2 - to_catalog = @result[:diffs][1] + pending 'catalog compilation failed' unless (@result[:exitcode]).zero? + to_catalog = @result.to expect(to_catalog).to be_a_kind_of(OctocatalogDiff::API::V1::Catalog) expect(to_catalog.builder).to eq('OctocatalogDiff::Catalog::Computed') end it 'should have log messages indicating catalog compilations' do - pending 'catalog compilation failed' unless @result[:exitcode] == 2 + pending 'catalog compilation failed' unless (@result[:exitcode]).zero? logs = @result[:logs] expect(logs).to match(/Compiling catalog for rspec-node.github.net/) expect(logs).to match(/Initialized OctocatalogDiff::Catalog::Noop for from-catalog/) @@ -52,8 +45,8 @@ end it 'should produce a valid catalog' do - pending 'catalog compilation failed' unless @result[:exitcode] == 2 - to_catalog = @result[:diffs][1] + pending 'catalog compilation failed' unless (@result[:exitcode]).zero? + to_catalog = @result.to expect(to_catalog.valid?).to eq(true) expect(to_catalog.to_h).to be_a_kind_of(Hash) expect(to_catalog.to_json).to be_a_kind_of(String) @@ -61,8 +54,8 @@ end it 'should produce the expected catalog' do - pending 'catalog compilation failed' unless @result[:exitcode] == 2 - to_catalog = @result[:diffs][1] + pending 'catalog compilation failed' unless @result.exitcode.zero? + to_catalog = @result.to param1 = { 'owner' => 'root', 'group' => 'root', 'mode' => '0644', 'content' => 'Testy McTesterson' } expect(to_catalog.resource(type: 'File', title: '/tmp/foo')['parameters']).to eq(param1) diff --git a/spec/octocatalog-diff/integration/integration_helper.rb b/spec/octocatalog-diff/integration/integration_helper.rb index 284404b2..05880431 100644 --- a/spec/octocatalog-diff/integration/integration_helper.rb +++ b/spec/octocatalog-diff/integration/integration_helper.rb @@ -113,10 +113,9 @@ def self.integration(options = {}) options[:from_puppet_binary] ||= OctocatalogDiff::Spec::PUPPET_BINARY options[:to_puppet_binary] ||= OctocatalogDiff::Spec::PUPPET_BINARY options[:parallel] = false if ENV['COVERAGE'] + options[:INTEGRATION] = true # Run octocatalog-diff CLI method. Capture stdout and stderr using 'strio'. - # Set options[:RETURN_DIFFS] so that the .cli method returns the JSON array - # of differences instead of the exit code. logger, logger_string = OctocatalogDiff::Spec.setup_logger begin # Capture stdout to a variable @@ -124,28 +123,30 @@ def self.integration(options = {}) stdout_strio = StringIO.new $stdout = stdout_strio - # Tell OctocatalogDiff::Cli.cli to return the JSON differences and not a numeric exit code - # for a full catalog-diff. - options[:RETURN_DIFFS] = true - # Run the OctocatalogDiff::Cli.cli and validate output format. result = OctocatalogDiff::Cli.cli(argv, logger, options) if result.is_a?(Fixnum) - return { - logs: logger_string.string, - output: stdout_strio.string, - exitcode: result - } + result = OpenStruct.new(exitcode: result, exception: nil, diffs: [], to: nil, from: nil) + elsif result.is_a?(Hash) + result = OpenStruct.new({ exitcode: nil, exception: nil, diffs: [], to: nil, from: nil }.merge(result)) + elsif !result.is_a?(OpenStruct) + raise "Expected OpenStruct, got #{result.inspect} from OctocatalogDiff::Cli.cli!" end - raise "OctocatalogDiff::Cli.cli should return array, got #{result.inspect}" unless result.is_a?(Array) + exitcode = if result.exitcode.nil? + result.diffs.any? ? 2 : 0 + else + result.exitcode + end - # Return hash OpenStruct.new( logs: logger_string.string, + log_messages: logger_string.string.split(/\n/).map { |x| OctocatalogDiff::Spec.strip_log_message(x) }, output: stdout_strio.string, - diffs: result, - exitcode: result.any? ? 2 : 0, + diffs: result.diffs, + to: result.to, + from: result.from, + exitcode: exitcode, options: options ) rescue => exc # Yes, rescue *everything* diff --git a/spec/octocatalog-diff/integration/reference_validation_spec.rb b/spec/octocatalog-diff/integration/reference_validation_spec.rb index 43705a0b..568ba269 100644 --- a/spec/octocatalog-diff/integration/reference_validation_spec.rb +++ b/spec/octocatalog-diff/integration/reference_validation_spec.rb @@ -45,7 +45,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should return the valid catalog' do - expect(@result.exitcode).to eq(2), OctocatalogDiff::Integration.format_exception(@result) + expect(@result.exitcode).to eq(0), OctocatalogDiff::Integration.format_exception(@result) end end @@ -55,7 +55,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should return the valid catalog' do - expect(@result.exitcode).to eq(2) + expect(@result.exitcode).to eq(0) end it 'should not raise any exceptions' do @@ -63,7 +63,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should contain representative resources' do - pending 'Catalog failed' unless @result.exitcode == 2 + pending 'Catalog failed' unless @result.exitcode.zero? expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'File', '/tmp/test-main')).to eq(true) end end @@ -75,7 +75,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should succeed' do - expect(@result.exitcode).to eq(2) + expect(@result.exitcode).to eq(0) end it 'should not raise any exceptions' do @@ -83,7 +83,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should contain representative resources' do - pending 'Catalog failed' unless @result.exitcode == 2 + pending 'Catalog failed' unless @result.exitcode.zero? expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'Exec', 'subscribe caller 1')).to eq(true) expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'Exec', 'subscribe target')).to eq(true) end @@ -179,7 +179,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should succeed' do - expect(@result.exitcode).to eq(2), OctocatalogDiff::Integration.format_exception(@result) + expect(@result.exitcode).to eq(0), OctocatalogDiff::Integration.format_exception(@result) end it 'should not raise error' do @@ -195,7 +195,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should succeed' do - expect(@result.exitcode).to eq(2) + expect(@result.exitcode).to eq(0) end it 'should not raise any exceptions' do @@ -203,7 +203,7 @@ def self.catalog_contains_resource(result, type, title) end it 'should contain representative resources' do - pending 'Catalog failed' unless @result.exitcode == 2 + pending 'Catalog failed' unless @result.exitcode.zero? expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'Exec', 'before alias caller')).to eq(true) expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'Exec', 'before alias target')).to eq(true) expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'Exec', 'the before alias target')).to eq(true) diff --git a/spec/octocatalog-diff/tests/spec_helper.rb b/spec/octocatalog-diff/tests/spec_helper.rb index df885b95..c60f6ff0 100644 --- a/spec/octocatalog-diff/tests/spec_helper.rb +++ b/spec/octocatalog-diff/tests/spec_helper.rb @@ -217,6 +217,11 @@ def self.setup_logger # remove the file and line locations. This takes the JSON result from catalog-diff and returns # a cleaned-up array of diffs. def self.remove_file_and_line(diff) + if diff.is_a?(OctocatalogDiff::API::V1::Diff) + result = diff.to_h.dup + %w(new_location old_location new_line old_line new_file old_file).each { |x| result.delete(x.to_sym) } + return result + end if diff.is_a?(Hash) result = diff.dup %w(new_location old_location new_line old_line new_file old_file).each { |x| result.delete(x) } @@ -228,6 +233,13 @@ def self.remove_file_and_line(diff) obj.map { |x| x[0] =~ /^[\-\+]$/ ? x[0..2] : x[0..3] } end + # Strip off timestamps and other extraneous content from log messages so that matching + # of individual elements can be done via string and not regexp. + def self.strip_log_message(message) + return message unless message.strip =~ /\A\w,\s*\[[^\]]+\]\s+(\w+)\s*--\s*:(.+)/ + "#{Regexp.last_match(1)} - #{Regexp.last_match(2).strip}" + end + # Get the Puppet version from the Puppet binary def self.puppet_version require require_path('util/puppetversion')