Merge pull request #672 from scotje/maint_fixup_cri_option_expects

(MAINT) Fix issues related to Cri behavior change with options hash

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])

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])

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)

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])
 

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
 

spec/unit/pdk/cli/convert_spec.rb

@@ -20,39 +20,39 @@
 
     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
     end
 
     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
     end
 
     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
     end
 
     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
     end
 
     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,15 +68,15 @@
 
     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
     end
 
     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

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

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

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])

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,15 +105,15 @@
     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
   end
 
   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')