From 0cb23cdf55a0ee044bc64c5abac5c901bc7d04fd Mon Sep 17 00:00:00 2001 From: Jesse Scott Date: Thu, 30 May 2019 14:50:27 -0700 Subject: [PATCH 1/2] (MAINT) Update specs to resolve issues with new Cri release --- spec/unit/pdk/cli/build_spec.rb | 4 ++-- spec/unit/pdk/cli/convert_spec.rb | 18 +++++++++--------- spec/unit/pdk/cli/new/task_spec.rb | 4 ++-- spec/unit/pdk/cli/test/unit_spec.rb | 4 ++-- spec/unit/pdk/cli/update_spec.rb | 11 ++++++++--- spec/unit/pdk/cli/validate_spec.rb | 18 +++++++++--------- 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/spec/unit/pdk/cli/build_spec.rb b/spec/unit/pdk/cli/build_spec.rb index 8f8d37151..5755bc8ad 100644 --- a/spec/unit/pdk/cli/build_spec.rb +++ b/spec/unit/pdk/cli/build_spec.rb @@ -97,7 +97,7 @@ include_context 'exits cleanly' it 'invokes the builder with the default target directory' do - expect(PDK::Module::Build).to receive(:new).with(:'target-dir' => File.join(Dir.pwd, 'pkg')).and_return(mock_builder) + expect(PDK::Module::Build).to receive(:new).with(hash_including(:'target-dir' => File.join(Dir.pwd, 'pkg'))).and_return(mock_builder) end end @@ -107,7 +107,7 @@ let(:command_opts) { ['--target-dir', '/tmp/pdk_builds'] } it 'invokes the builder with the specified target directory' do - expect(PDK::Module::Build).to receive(:new).with(:'target-dir' => '/tmp/pdk_builds').and_return(mock_builder) + expect(PDK::Module::Build).to receive(:new).with(hash_including(:'target-dir' => '/tmp/pdk_builds')).and_return(mock_builder) end end diff --git a/spec/unit/pdk/cli/convert_spec.rb b/spec/unit/pdk/cli/convert_spec.rb index fe2112669..719d79b3e 100644 --- a/spec/unit/pdk/cli/convert_spec.rb +++ b/spec/unit/pdk/cli/convert_spec.rb @@ -20,7 +20,7 @@ context 'and provided no flags' do it 'invokes the converter with no template specified' do - expect(PDK::Module::Convert).to receive(:invoke).with({}) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_not_including(:'template-url')) PDK::CLI.run(['convert']) end @@ -28,7 +28,7 @@ context 'and the --template-url option has been passed' do it 'invokes the converter with the user supplied template' do - expect(PDK::Module::Convert).to receive(:invoke).with(:'template-url' => 'https://my/template') + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:'template-url' => 'https://my/template')) PDK::CLI.run(['convert', '--template-url', 'https://my/template']) end @@ -36,7 +36,7 @@ context 'and the --template-ref option has been passed' do it 'invokes the converter with the user supplied template' do - expect(PDK::Module::Convert).to receive(:invoke).with(:'template-url' => 'https://my/template', :'template-ref' => '1.0.0') + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:'template-url' => 'https://my/template', :'template-ref' => '1.0.0')) PDK::CLI.run(['convert', '--template-url', 'https://my/template', '--template-ref', '1.0.0']) end @@ -44,7 +44,7 @@ context 'and the --noop flag has been passed' do it 'passes the noop option through to the converter' do - expect(PDK::Module::Convert).to receive(:invoke).with(noop: true) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(noop: true)) PDK::CLI.run(['convert', '--noop']) end @@ -52,7 +52,7 @@ context 'and the --force flag has been passed' do it 'passes the force option through to the converter' do - expect(PDK::Module::Convert).to receive(:invoke).with(force: true) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(force: true)) PDK::CLI.run(['convert', '--force']) end @@ -68,7 +68,7 @@ context 'and the --skip-interview flag has been passed' do it 'passes the skip-interview option through to the converter' do - expect(PDK::Module::Convert).to receive(:invoke).with(:'skip-interview' => true) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:'skip-interview' => true)) PDK::CLI.run(['convert', '--skip-interview']) end @@ -76,7 +76,7 @@ context 'and the --full-interview flag has been passed' do it 'passes the full-interview option through to the converter' do - expect(PDK::Module::Convert).to receive(:invoke).with(:'full-interview' => true) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:'full-interview' => true)) PDK::CLI.run(['convert', '--full-interview']) end @@ -85,7 +85,7 @@ context 'and the --skip-interview and --full-interview flags have been passed' do it 'ignores full-interview and continues with a log message' do expect(logger).to receive(:info).with(a_string_matching(%r{Ignoring --full-interview and continuing with --skip-interview.}i)) - expect(PDK::Module::Convert).to receive(:invoke).with(:'skip-interview' => true, :'full-interview' => false) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:'skip-interview' => true, :'full-interview' => false)) PDK::CLI.run(['convert', '--skip-interview', '--full-interview']) end @@ -94,7 +94,7 @@ context 'and the --force and --full-interview flags have been passed' do it 'ignores full-interview and continues with a log message' do expect(logger).to receive(:info).with(a_string_matching(%r{Ignoring --full-interview and continuing with --force.}i)) - expect(PDK::Module::Convert).to receive(:invoke).with(:force => true, :'full-interview' => false) + expect(PDK::Module::Convert).to receive(:invoke).with(hash_including(:force => true, :'full-interview' => false)) PDK::CLI.run(['convert', '--force', '--full-interview']) end diff --git a/spec/unit/pdk/cli/new/task_spec.rb b/spec/unit/pdk/cli/new/task_spec.rb index 3d677ffaf..331107117 100644 --- a/spec/unit/pdk/cli/new/task_spec.rb +++ b/spec/unit/pdk/cli/new/task_spec.rb @@ -53,10 +53,10 @@ context 'and provided a valid task name' do let(:generator) { PDK::Generate::Task } let(:generator_double) { instance_double(generator) } - let(:generator_opts) { instance_of(Hash) } + let(:generator_opts) { {} } before(:each) do - allow(generator).to receive(:new).with(anything, 'test_task', generator_opts).and_return(generator_double) + allow(generator).to receive(:new).with(anything, 'test_task', hash_including(generator_opts)).and_return(generator_double) end it 'generates the task' do diff --git a/spec/unit/pdk/cli/test/unit_spec.rb b/spec/unit/pdk/cli/test/unit_spec.rb index adcb85121..4c3304879 100644 --- a/spec/unit/pdk/cli/test/unit_spec.rb +++ b/spec/unit/pdk/cli/test/unit_spec.rb @@ -101,7 +101,7 @@ context 'when passed --clean-fixtures' do it 'invokes the command with :clean-fixtures => true' do - expect(PDK::Test::Unit).to receive(:invoke).with(reporter, :puppet => puppet_version, :tests => anything, :'clean-fixtures' => true).once.and_return(0) + expect(PDK::Test::Unit).to receive(:invoke).with(reporter, hash_including(:puppet => puppet_version, :tests => anything, :'clean-fixtures' => true)).once.and_return(0) expect { test_unit_cmd.run_this(['--clean-fixtures']) }.to exit_zero @@ -110,7 +110,7 @@ context 'when not passed --clean-fixtures' do it 'invokes the command without :clean-fixtures' do - expect(PDK::Test::Unit).to receive(:invoke).with(reporter, puppet: puppet_version, tests: anything).once.and_return(0) + expect(PDK::Test::Unit).to receive(:invoke).with(reporter, hash_including(puppet: puppet_version, tests: anything)).once.and_return(0) expect { test_unit_cmd.run_this([]) }.to exit_zero diff --git a/spec/unit/pdk/cli/update_spec.rb b/spec/unit/pdk/cli/update_spec.rb index 5493c9ae6..4ab27e4bc 100644 --- a/spec/unit/pdk/cli/update_spec.rb +++ b/spec/unit/pdk/cli/update_spec.rb @@ -26,7 +26,12 @@ context 'and provided no flags' do it 'invokes the updater with no options' do - expect(PDK::Module::Update).to receive(:new).with({}).and_return(updater) + expect(PDK::Module::Update).to receive(:new) { |opts| + expect(opts[:noop]).to be false if opts.key?(:noop) + expect(opts[:force]).to be false if opts.key?(:false) # rubocop:disable Lint/BooleanSymbol + expect(opts).not_to include(:'template-ref') + }.and_return(updater) + expect(updater).to receive(:run) PDK::CLI.run(%w[update]) @@ -35,7 +40,7 @@ context 'and the --noop flag has been passed' do it 'passes the noop option through to the updater' do - expect(PDK::Module::Update).to receive(:new).with(noop: true).and_return(updater) + expect(PDK::Module::Update).to receive(:new).with(hash_including(noop: true)).and_return(updater) expect(updater).to receive(:run) PDK::CLI.run(%w[update --noop]) @@ -44,7 +49,7 @@ context 'and the --force flag has been passed' do it 'passes the force option through to the updater' do - expect(PDK::Module::Update).to receive(:new).with(force: true).and_return(updater) + expect(PDK::Module::Update).to receive(:new).with(hash_including(force: true)).and_return(updater) expect(updater).to receive(:run) PDK::CLI.run(%w[update --force]) diff --git a/spec/unit/pdk/cli/validate_spec.rb b/spec/unit/pdk/cli/validate_spec.rb index 26b586bf4..f9934814d 100644 --- a/spec/unit/pdk/cli/validate_spec.rb +++ b/spec/unit/pdk/cli/validate_spec.rb @@ -25,7 +25,7 @@ context 'when no arguments or options are provided' do it 'invokes each validator with no report and no options and exits zero' do - expect(validators).to all(receive(:invoke).with(report, puppet: puppet_version).and_return(0)) + expect(validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0)) expect(logger).to receive(:info).with('Running all available validators...') @@ -61,7 +61,7 @@ let(:validator) { PDK::Validate::MetadataValidator } it 'only invokes the given validator and exits zero' do - expect(validator).to receive(:invoke).with(report, puppet: puppet_version).and_return(0) + expect(validator).to receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0) validators.reject { |r| r == validator }.each do |v| expect(v).not_to receive(:invoke) @@ -80,7 +80,7 @@ end it 'invokes each given validator and exits zero' do - expect(invoked_validators).to all(receive(:invoke).with(report, puppet: puppet_version).and_return(0)) + expect(invoked_validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0)) (validators | invoked_validators).each do |validator| expect(validator).not_to receive(:invoke) @@ -95,7 +95,7 @@ it 'warns about unknown validators, invokes known validators, and exits zero' do expect(logger).to receive(:warn).with(%r{Unknown validator 'bad-val'. Available validators: #{validator_names}}i) - expect(validator).to receive(:invoke).with(report, puppet: puppet_version).and_return(0) + expect(validator).to receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0) expect { PDK::CLI.run(['validate', 'puppet,bad-val']) }.to exit_zero end @@ -105,7 +105,7 @@ let(:validator) { PDK::Validate::MetadataValidator } it 'invokes the specified validator with the target as an option' do - expect(validator).to receive(:invoke).with(report, puppet: puppet_version, targets: ['lib/', 'manifests/']).and_return(0) + expect(validator).to receive(:invoke).with(report, hash_including(puppet: puppet_version, targets: ['lib/', 'manifests/'])).and_return(0) expect { PDK::CLI.run(['validate', 'metadata', 'lib/', 'manifests/']) }.to exit_zero end @@ -113,7 +113,7 @@ context 'when targets are provided as arguments and no validators are specified' do it 'invokes all validators with the target as an option' do - expect(validators).to all(receive(:invoke).with(report, puppet: puppet_version, targets: ['lib/', 'manifests/']).and_return(0)) + expect(validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version, targets: ['lib/', 'manifests/'])).and_return(0)) expect(logger).to receive(:info).with('Running all available validators...') @@ -123,7 +123,7 @@ context 'when no report formats are specified' do it 'reports to stdout as text' do - expect(validators).to all(receive(:invoke).with(report, puppet: puppet_version).and_return(0)) + expect(validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0)) expect(report).to receive(:write_text).with($stdout) expect(report).not_to receive(:write_junit) @@ -133,7 +133,7 @@ context 'when a report format is specified' do it 'reports to stdout as the specified format' do - expect(validators).to all(receive(:invoke).with(report, puppet: puppet_version).and_return(0)) + expect(validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0)) expect(report).to receive(:write_junit).with($stdout) expect(report).not_to receive(:write_text) @@ -143,7 +143,7 @@ context 'when multiple report formats are specified' do it 'reports to each target as the specified format' do - expect(validators).to all(receive(:invoke).with(report, puppet: puppet_version).and_return(0)) + expect(validators).to all(receive(:invoke).with(report, hash_including(puppet: puppet_version)).and_return(0)) expect(report).to receive(:write_text).with($stderr) expect(report).to receive(:write_text).with($stdout) expect(report).to receive(:write_junit).with('testfile.xml') From dda53e2e3c7e2a79a6a78ca349b96a7e1f989492 Mon Sep 17 00:00:00 2001 From: Jesse Scott Date: Thu, 30 May 2019 15:06:20 -0700 Subject: [PATCH 2/2] (MAINT) Fix up places where we used .key? to check if flag was set Apparently this was never the documented Cri behavior and they recently fixed this so that flags and other "no argument" options will have keys set in the options hash with a value of "false" instead of simply being absent from the options hash. --- lib/pdk/cli/test/unit.rb | 2 +- lib/pdk/cli/validate.rb | 4 ++-- lib/pdk/generate/module.rb | 2 +- lib/pdk/tests/unit.rb | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/pdk/cli/test/unit.rb b/lib/pdk/cli/test/unit.rb index 0d2300b50..a13648243 100644 --- a/lib/pdk/cli/test/unit.rb +++ b/lib/pdk/cli/test/unit.rb @@ -35,7 +35,7 @@ module PDK::CLI # Ensure that the bundled gems are up to date and correct Ruby is activated before running or listing tests. puppet_env = PDK::CLI::Util.puppet_from_opts_or_env(opts) - PDK::Util::PuppetVersion.fetch_puppet_dev if opts.key?(:'puppet-dev') + PDK::Util::PuppetVersion.fetch_puppet_dev if opts[:'puppet-dev'] PDK::Util::RubyVersion.use(puppet_env[:ruby_version]) opts.merge!(puppet_env[:gemset]) diff --git a/lib/pdk/cli/validate.rb b/lib/pdk/cli/validate.rb index c0a1836a2..2a96e6f3a 100644 --- a/lib/pdk/cli/validate.rb +++ b/lib/pdk/cli/validate.rb @@ -85,11 +85,11 @@ module PDK::CLI end options = targets.empty? ? {} : { targets: targets } - options[:auto_correct] = true if opts.key?(:'auto-correct') + options[:auto_correct] = true if opts[:'auto-correct'] # Ensure that the bundled gems are up to date and correct Ruby is activated before running any validations. puppet_env = PDK::CLI::Util.puppet_from_opts_or_env(opts) - PDK::Util::PuppetVersion.fetch_puppet_dev if opts.key?(:'puppet-dev') + PDK::Util::PuppetVersion.fetch_puppet_dev if opts[:'puppet-dev'] PDK::Util::RubyVersion.use(puppet_env[:ruby_version]) options.merge!(puppet_env[:gemset]) diff --git a/lib/pdk/generate/module.rb b/lib/pdk/generate/module.rb index 44f54254c..c071f8ad8 100644 --- a/lib/pdk/generate/module.rb +++ b/lib/pdk/generate/module.rb @@ -253,7 +253,7 @@ def self.module_interview(metadata, opts = {}) else questions.reject! { |q| q[:name] == 'module_name' } if opts.key?(:module_name) questions.reject! { |q| q[:name] == 'license' } if opts.key?(:license) - questions.reject! { |q| q[:forge_only] } unless opts.key?(:'full-interview') + questions.reject! { |q| q[:forge_only] } unless opts[:'full-interview'] end interview.add_questions(questions) diff --git a/lib/pdk/tests/unit.rb b/lib/pdk/tests/unit.rb index c2dfb9691..a681297e7 100644 --- a/lib/pdk/tests/unit.rb +++ b/lib/pdk/tests/unit.rb @@ -7,7 +7,7 @@ module PDK module Test class Unit def self.cmd(tests, opts = {}) - rake_args = opts.key?(:parallel) ? 'parallel_spec_standalone' : 'spec_standalone' + rake_args = opts[:parallel] ? 'parallel_spec_standalone' : 'spec_standalone' rake_args += "[#{tests}]" unless tests.nil? rake_args end @@ -72,23 +72,23 @@ def self.invoke(report, options = {}) environment = { 'CI_SPEC_OPTIONS' => '--format j' } environment['PUPPET_GEM_VERSION'] = options[:puppet] if options[:puppet] - spinner_msg = options.key?(:parallel) ? _('Running unit tests in parallel.') : _('Running unit tests.') + spinner_msg = options[:parallel] ? _('Running unit tests in parallel.') : _('Running unit tests.') result = rake(cmd(tests, options), spinner_msg, environment) - json_result = if options.key?(:parallel) + json_result = if options[:parallel] PDK::Util.find_all_json_in(result[:stdout]) else PDK::Util.find_first_json_in(result[:stdout]) end - if parallel_with_no_tests?(options.key?(:parallel), json_result, result) + if parallel_with_no_tests?(options[:parallel], json_result, result) json_result = [{ 'messages' => ['No examples found.'] }] result[:exit_code] = 0 end raise PDK::CLI::FatalError, _('Unit test output did not contain a valid JSON result: %{output}') % { output: result[:stdout] } if json_result.nil? || json_result.empty? - json_result = merge_json_results(json_result) if options.key?(:parallel) + json_result = merge_json_results(json_result) if options[:parallel] parse_output(report, json_result, result[:duration])