From 4551a0e925c86f90dd6872a4cf051c77974a5e2c Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 20 Feb 2020 14:23:04 +0000 Subject: [PATCH 1/2] (maint) update to rubocop 0.68 * fix new and updated rules * hardcode version numbers for dependabot to keep failures out of other PRs --- .rubocop.yml | 16 ++++++++++------ Gemfile | 10 +++++----- .../puppet_litmus/inventory_manipulation_spec.rb | 12 ++++++------ spec/lib/puppet_litmus/puppet_helpers_spec.rb | 8 ++++---- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 5a46d4ac..b3abc244 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,7 +13,7 @@ AllCops: - pkg/**/* - spec/fixtures/**/* - vendor/**/* -Metrics/LineLength: +Layout/LineLength: Description: People have wide screens, use them. Max: 200 RSpec/BeforeAfterAll: @@ -122,11 +122,6 @@ Lint/NestedMethodDefinition: RSpec/MessageSpies: Enabled: false -# If neighbouring parameters are both hashes, it helps to have braces round both. -Style/BracesAroundHashParameters: - Exclude: - - 'spec/puppet/resource_api/base_context_spec.rb' - # requires 2.3's squiggly HEREDOC support, which we can't use, yet # see http://www.virtuouscode.com/2016/01/06/about-the-ruby-squiggly-heredoc-syntax/ Layout/HeredocIndentation: @@ -162,3 +157,12 @@ Layout/ClosingHeredocIndentation: # This cop breaks code coverage :( RSpec/AnyInstance: Enabled: false + +Style/HashEachMethods: + Enabled: true + +Style/HashTransformKeys: + Enabled: true + +Style/HashTransformValues: + Enabled: true diff --git a/Gemfile b/Gemfile index e889ca9b..58051b54 100644 --- a/Gemfile +++ b/Gemfile @@ -7,11 +7,11 @@ group :test do gem 'rspec', '~> 3.0' gem 'rspec-collection_matchers', '~> 1.0' gem 'rspec-its', '~> 1.0' - gem 'rubocop' - gem 'rubocop-rspec' - gem 'simplecov' - gem 'codecov' -end + gem 'rubocop', '~> 0.68' + gem 'rubocop-rspec', '~> 1.38' + gem 'codecov', '~> 0.1' + gem 'simplecov', '~> 0.18' + end group :development do # TODO: Use gem instead of git. Section mapping is merged into master, but not yet released diff --git a/spec/lib/puppet_litmus/inventory_manipulation_spec.rb b/spec/lib/puppet_litmus/inventory_manipulation_spec.rb index 50b0a5dd..c07b3a9f 100644 --- a/spec/lib/puppet_litmus/inventory_manipulation_spec.rb +++ b/spec/lib/puppet_litmus/inventory_manipulation_spec.rb @@ -115,11 +115,11 @@ end it 'no feature exists for the group, and returns hash with feature added' do - expect(described_class.add_feature_to_group(no_feature_hash, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => ['puppet-agent'], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.add_feature_to_group(no_feature_hash, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => ['puppet-agent'], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'feature exists for the group, and returns hash with feature removed' do - expect(described_class.remove_feature_from_group(feature_hash_group, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => [], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.remove_feature_from_group(feature_hash_group, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => [], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'write from inventory_hash to inventory_yaml file feature_hash_group' do @@ -127,15 +127,15 @@ end it 'empty feature exists for the group, and returns hash with feature added' do - expect(described_class.add_feature_to_group(empty_feature_hash_group, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => ['puppet-agent'], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.add_feature_to_group(empty_feature_hash_group, 'puppet-agent', 'ssh_nodes')).to eq('groups' => [{ 'features' => ['puppet-agent'], 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net' }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'no feature exists for the node, and returns hash with feature added' do - expect(described_class.add_feature_to_node(no_feature_hash, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => ['puppet-agent'] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.add_feature_to_node(no_feature_hash, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => ['puppet-agent'] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'feature exists for the node, and returns hash with feature removed' do - expect(described_class.remove_feature_from_node(feature_hash_node, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => [] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.remove_feature_from_node(feature_hash_node, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => [] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'write from inventory_hash to inventory_yaml file feature_hash_node' do @@ -143,7 +143,7 @@ end it 'empty feature exists for the node, and returns hash with feature added' do - expect(described_class.add_feature_to_node(empty_feature_hash_node, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => ['puppet-agent'] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Metrics/LineLength: Line is too long + expect(described_class.add_feature_to_node(empty_feature_hash_node, 'puppet-agent', 'test.delivery.puppetlabs.net')).to eq('groups' => [{ 'name' => 'ssh_nodes', 'targets' => [{ 'config' => { 'ssh' => { 'host-key-check' => false, 'password' => 'Qu@lity!', 'user' => 'root' }, 'transport' => 'ssh' }, 'facts' => { 'platform' => 'centos-5-x86_64', 'provisioner' => 'vmpooler' }, 'uri' => 'test.delivery.puppetlabs.net', 'features' => ['puppet-agent'] }] }, { 'name' => 'winrm_nodes', 'targets' => [] }]) # rubocop:disable Layout/LineLength: Line is too long end it 'write from inventory_hash to inventory_yaml file no feature_hash' do diff --git a/spec/lib/puppet_litmus/puppet_helpers_spec.rb b/spec/lib/puppet_litmus/puppet_helpers_spec.rb index 46ee148b..8cf12d7d 100644 --- a/spec/lib/puppet_litmus/puppet_helpers_spec.rb +++ b/spec/lib/puppet_litmus/puppet_helpers_spec.rb @@ -122,10 +122,10 @@ let(:local) { '/tmp' } let(:remote) { '/remote_tmp' } # Ignore rubocop because these hashes are representative of output from an external method and editing them leads to test failures. - # rubocop:disable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Metrics/LineLength, Layout/SpaceAfterComma + # rubocop:disable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Layout/LineLength, Layout/SpaceAfterComma let(:result_success) {[{'node'=>'some.host','target'=>'some.host','action'=>'upload','object'=>'C:\foo\bar.ps1','status'=>'success','result'=>{'_output'=>'Uploaded \'C:\foo\bar.ps1\' to \'some.host:C:\bar\''}}]} let(:result_failure) {[{'node'=>'some.host','target'=>'some.host','action'=>nil,'object'=>nil,'status'=>'failure','result'=>{'_error'=>{'kind'=>'puppetlabs.tasks/task_file_error','msg'=>'No such file or directory @ rb_sysopen - /nonexistant/file/path','details'=>{},'issue_code'=>'WRITE_ERROR'}}}]} - # rubocop:enable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Metrics/LineLength, Layout/SpaceAfterComma + # rubocop:enable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Layout/LineLength, Layout/SpaceAfterComma let(:inventory_hash) { { 'groups' => [{ 'name' => 'local', 'nodes' => [{ 'name' => 'some.host', 'config' => { 'transport' => 'local' } }] }] } } let(:localhost_inventory_hash) { { 'groups' => [{ 'name' => 'local', 'nodes' => [{ 'name' => 'litmus_localhost', 'config' => { 'transport' => 'local' } }] }] } } @@ -228,11 +228,11 @@ let(:params) { { 'action' => 'install', 'name' => 'foo' } } let(:config_data) { { 'modulepath' => File.join(Dir.pwd, 'spec', 'fixtures', 'modules') } } # Ignore rubocop because these hashes are representative of output from an external method and editing them leads to test failures. - # rubocop:disable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceBeforeBlockBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Metrics/LineLength, Layout/SpaceAfterComma + # rubocop:disable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceBeforeBlockBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Layout/LineLength, Layout/SpaceAfterComma let(:result_unstructured_task_success){ [{'node'=>'some.host','target'=>'some.host','action'=>'task','object'=>'testtask::unstructured','status'=>'success','result'=>{'_output'=>'SUCCESS!'}}]} let(:result_structured_task_success){ [{'node'=>'some.host','target'=>'some.host','action'=>'task','object'=>'testtask::structured','status'=>'success','result'=>{'key1'=>'foo','key2'=>'bar'}}]} let(:result_failure) {[{'node'=>'some.host','target'=>'some.host','action'=>'task','object'=>'testtask::unstructured','status'=>'failure','result'=>{'_error'=>{'msg'=>'FAILURE!','kind'=>'puppetlabs.tasks/task-error','details'=>{'exitcode'=>123}}}}]} - # rubocop:enable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceBeforeBlockBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Metrics/LineLength, Layout/SpaceAfterComma + # rubocop:enable Layout/SpaceInsideHashLiteralBraces, Layout/SpaceBeforeBlockBraces, Layout/SpaceInsideBlockBraces, Layout/SpaceAroundOperators, Layout/LineLength, Layout/SpaceAfterComma let(:inventory_hash) { { 'groups' => [{ 'name' => 'local', 'nodes' => [{ 'name' => 'some.host', 'config' => { 'transport' => 'local' } }] }] } } it 'responds to bolt_run_task' do From 755607b0587d982d2f0c0d7032858b21866f8ac9 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 20 Feb 2020 12:13:19 +0000 Subject: [PATCH 2/2] Make InventoryManipulation available to all rake tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously this was guarded by the existence of a inventory.yaml file, which could lead to errors when trying to chain operations: ``` ~/git/puppetlabs-motd$ rm inventory.yaml ~/git/puppetlabs-motd$ bundle exec rake litmus:provision[docker,litmusimage/debian:9] litmus:install_agent litmus:install_module Provisioning litmusimage/debian:9 using docker provisioner.[✔] localhost:2223, litmusimage/debian:9 rake aborted! NameError: undefined local variable or method `inventory_hash_from_inventory_file' for main:Object /home/david/gems/ruby/2.5.0/gems/puppet_litmus-0.15.0/lib/puppet_litmus/rake_tasks.rb:104:in `block (2 levels) in ' /home/david/gems/ruby/2.5.0/gems/rake-12.3.3/exe/rake:27:in `' Tasks: TOP => litmus:install_agent (See full trace by running task with --trace) ~/git/puppetlabs-motd$ ``` --- lib/puppet_litmus/inventory_manipulation.rb | 2 ++ lib/puppet_litmus/rake_tasks.rb | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/puppet_litmus/inventory_manipulation.rb b/lib/puppet_litmus/inventory_manipulation.rb index 8cb4a3ca..2c139637 100644 --- a/lib/puppet_litmus/inventory_manipulation.rb +++ b/lib/puppet_litmus/inventory_manipulation.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +module PuppetLitmus; end # rubocop:disable Style/Documentation + # helper functions for manipulating and reading a bolt inventory file module PuppetLitmus::InventoryManipulation # Creates an inventory hash from the inventory.yaml. diff --git a/lib/puppet_litmus/rake_tasks.rb b/lib/puppet_litmus/rake_tasks.rb index 91fa96ff..9045088e 100644 --- a/lib/puppet_litmus/rake_tasks.rb +++ b/lib/puppet_litmus/rake_tasks.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true namespace :litmus do + require 'puppet_litmus/inventory_manipulation' require 'puppet_litmus/rake_helper' + include PuppetLitmus::InventoryManipulation include PuppetLitmus::RakeHelper # Prints all supported OSes from metadata.json file. desc 'print all supported OSes from metadata' @@ -324,8 +326,6 @@ namespace :acceptance do require 'rspec/core/rake_task' if File.file?('inventory.yaml') - require 'puppet_litmus/inventory_manipulation' - include PuppetLitmus::InventoryManipulation inventory_hash = inventory_hash_from_inventory_file targets = find_targets(inventory_hash, nil)