From 63ed34f354ed86f81c859cb07569b2e359ff6a59 Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Fri, 24 May 2024 10:43:35 +0200 Subject: [PATCH 1/3] Add rubocop config previously this gem only contained a rubocop config that's used in our modules. We didn't run rubocop for the ruby code in this exact gem. This is changed with this commit. --- .github/workflows/test.yml | 12 ++++++++++++ .rubocop.yml | 38 ++++++++++++++++++++++++++++++++++++++ Rakefile | 10 ++++++++++ 3 files changed, 60 insertions(+) create mode 100644 .rubocop.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ace3130..6d10338 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,17 @@ env: BUNDLE_WITHOUT: release jobs: + rubocop: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install Ruby ${{ matrix.ruby }} + uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.3" + bundler-cache: true + - name: Run Rubocop + run: bundle exec rake rubocop test: runs-on: ubuntu-latest strategy: @@ -45,6 +56,7 @@ jobs: run: bundle exec rake spec tests: needs: + - rubocop - test runs-on: ubuntu-latest name: Test suite diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..3953d40 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,38 @@ +--- +require: + - rubocop-rake + - rubocop-rspec + +# this alligns with our settings in voxpupuli-rubocop +# but we cannot depend on that because it pulls in newer rubocop versions, +# which would break our modules on legacy puppetserver +AllCops: + NewCops: enable + DisplayCopNames: true + ExtraDetails: true + DisplayStyleGuide: true + +# this currently doesn't work with the way we handle our secrets +Gemspec/RequireMFA: + Enabled: false + +# Vox Pupuli default is to use `add_development_dependency` in the gemspec +Gemspec/DevelopmentDependencies: + Enabled: false + +Style/TernaryParentheses: + EnforcedStyle: require_parentheses_when_complex + +Style/TrailingCommaInHashLiteral: + Enabled: true + EnforcedStyleForMultiline: consistent_comma + +Style/TrailingCommaInArrayLiteral: + Enabled: true + EnforcedStyleForMultiline: consistent_comma + +Style/TrailingCommaInArguments: + Enabled: true + EnforcedStyleForMultiline: comma + +inherit_from: .rubocop_todo.yml diff --git a/Rakefile b/Rakefile index d316bb5..f4990f3 100644 --- a/Rakefile +++ b/Rakefile @@ -17,3 +17,13 @@ begin rescue LoadError # Optional gem, release group is probably disabled end + +# this is identical to our config in voxpupuli-rubocop, but that gem targets Ruby 2.7 +# and voxupuli-test depends on an older rubocop version because we provide it for our modules +require 'rubocop/rake_task' +RuboCop::RakeTask.new(:rubocop) do |task| + # These make the rubocop experience maybe slightly less terrible + task.options = ['--display-cop-names', '--display-style-guide', '--extra-details'] + # Use Rubocop's Github Actions formatter if possible + task.formatters << 'github' if ENV['GITHUB_ACTIONS'] == 'true' +end From cd8688fc5f9351ca80c29ef521421d4608cb2508 Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Fri, 24 May 2024 10:55:17 +0200 Subject: [PATCH 2/3] rubocop: fix save violations --- .rubocop_todo.yml | 94 +++++++++++++++++++++++++++++++++++++ Gemfile | 10 ++-- Rakefile | 6 ++- lib/voxpupuli/test.rb | 0 lib/voxpupuli/test/facts.rb | 7 +-- lib/voxpupuli/test/rake.rb | 3 +- spec/facts_spec.rb | 54 ++++++++++----------- spec/spec_helper.rb | 2 + voxpupuli-test.gemspec | 4 +- 9 files changed, 141 insertions(+), 39 deletions(-) create mode 100644 .rubocop_todo.yml delete mode 100644 lib/voxpupuli/test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..69b8d56 --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,94 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2024-06-21 09:28:53 UTC using RuboCop version 1.50.2. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 2 +# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. +Lint/DuplicateBranch: + Exclude: + - 'lib/voxpupuli/test/facts.rb' + +# Offense count: 1 +# Configuration parameters: AllowComments, AllowNil. +Lint/SuppressedException: + Exclude: + - 'Rakefile' + +# Offense count: 1 +# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. +Metrics/AbcSize: + Max: 19 + +# Offense count: 1 +# Configuration parameters: AllowedMethods, AllowedPatterns. +Metrics/CyclomaticComplexity: + Max: 12 + +# Offense count: 1 +# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. +Metrics/MethodLength: + Max: 28 + +# Offense count: 1 +# Configuration parameters: Prefixes, AllowedPatterns. +# Prefixes: when, with, without +RSpec/ContextWording: + Exclude: + - 'spec/facts_spec.rb' + +# Offense count: 2 +# Configuration parameters: IgnoredMetadata. +RSpec/DescribeClass: + Exclude: + - '**/spec/features/**/*' + - '**/spec/requests/**/*' + - '**/spec/routing/**/*' + - '**/spec/system/**/*' + - '**/spec/views/**/*' + - 'spec/facts_spec.rb' + +# Offense count: 1 +# Configuration parameters: CountAsOne. +RSpec/ExampleLength: + Max: 7 + +# Offense count: 6 +# Configuration parameters: . +# SupportedStyles: have_received, receive +RSpec/MessageSpies: + EnforcedStyle: receive + +# Offense count: 1 +RSpec/MultipleDescribes: + Exclude: + - 'spec/facts_spec.rb' + +# Offense count: 1 +RSpec/MultipleExpectations: + Max: 4 + +# Offense count: 3 +# This cop supports unsafe autocorrection (--autocorrect-all). +# Configuration parameters: EnforcedStyle. +# SupportedStyles: always, always_true, never +Style/FrozenStringLiteralComment: + Exclude: + - 'lib/voxpupuli/test/facts.rb' + - 'lib/voxpupuli/test/rake.rb' + - 'lib/voxpupuli/test/spec_helper.rb' + +# Offense count: 1 +Style/MixinUsage: + Exclude: + - 'lib/voxpupuli/test/facts.rb' + +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns. +# URISchemes: http, https +Layout/LineLength: + Max: 136 diff --git a/Gemfile b/Gemfile index 5b2c938..d383607 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec @@ -7,10 +9,10 @@ group :release do gem 'github_changelog_generator', '~> 1.16.4', require: false end -group :coverage, optional: ENV['COVERAGE']!='yes' do - gem 'simplecov-console', :require => false - gem 'codecov', :require => false +group :coverage, optional: ENV['COVERAGE'] != 'yes' do + gem 'codecov', require: false + gem 'simplecov-console', require: false end # Override gemspec for CI matrix builds. -gem 'puppet', ENV.fetch('PUPPET_VERSION', '>= 7.24'), :require => false +gem 'puppet', ENV.fetch('PUPPET_VERSION', '>= 7.24'), require: false diff --git a/Rakefile b/Rakefile index f4990f3..968cfd0 100644 --- a/Rakefile +++ b/Rakefile @@ -1,15 +1,17 @@ +# frozen_string_literal: true + require 'rspec/core/rake_task' RSpec::Core::RakeTask.new(:spec) -task :default => :spec +task default: :spec begin require 'github_changelog_generator/task' GitHubChangelogGenerator::RakeTask.new :changelog do |config| config.header = "# Changelog\n\nAll notable changes to this project will be documented in this file." - config.exclude_labels = %w{duplicate question invalid wontfix wont-fix skip-changelog} + config.exclude_labels = %w[duplicate question invalid wontfix wont-fix skip-changelog] config.user = 'voxpupuli' config.project = 'voxpupuli-test' config.future_release = "v#{Gem::Specification.load("#{config.project}.gemspec").version}" diff --git a/lib/voxpupuli/test.rb b/lib/voxpupuli/test.rb deleted file mode 100644 index e69de29..0000000 diff --git a/lib/voxpupuli/test/facts.rb b/lib/voxpupuli/test/facts.rb index c78906d..e8fa424 100644 --- a/lib/voxpupuli/test/facts.rb +++ b/lib/voxpupuli/test/facts.rb @@ -60,6 +60,7 @@ def add_facts_for_metadata(metadata) def normalize_module_name(name) return unless name + name.sub('-', '/') end @@ -70,7 +71,7 @@ def add_stdlib_facts # Rough conversion of grepping in the puppet source: # grep defaultfor lib/puppet/provider/service/*.rb - add_custom_fact :service_provider, ->(_os, facts) do + add_custom_fact :service_provider, lambda { |_os, facts| os = RSpec.configuration.facterdb_string_keys ? facts['os'] : facts[:os] case os['family'].downcase when 'archlinux', 'debian', 'redhat' @@ -84,11 +85,11 @@ def add_stdlib_facts when 'openbsd' 'openbsd' when 'suse' - os['release']['major'].to_i >= 12 ? 'systemd' : 'redhat' + (os['release']['major'].to_i >= 12) ? 'systemd' : 'redhat' when 'windows' 'windows' else 'init' end - end + } end diff --git a/lib/voxpupuli/test/rake.rb b/lib/voxpupuli/test/rake.rb index 92ec99e..4d6270a 100644 --- a/lib/voxpupuli/test/rake.rb +++ b/lib/voxpupuli/test/rake.rb @@ -1,6 +1,6 @@ require 'puppetlabs_spec_helper/rake_tasks' -PuppetLint.configuration.log_format = '%{path}:%{line}:%{check}:%{KIND}:%{message}' +PuppetLint.configuration.log_format = '%s:%s:%s:%s:%s' # without this, puppet-lint always gives an exit code of 0 PuppetLint.configuration.fail_on_warnings = true @@ -16,6 +16,7 @@ Dir.glob('**/*.md', File::FNM_DOTMATCH).sort.each do |filename| next if filename =~ %r{^((modules|acceptance|\.?vendor|spec/fixtures|pkg)/|REFERENCE.md)} + File.foreach(filename).each_with_index do |line, index| if line =~ %r{\s\n$} errors << "#{filename} has trailing whitespace on line #{index + 1}" diff --git a/spec/facts_spec.rb b/spec/facts_spec.rb index 924e918..29d8c90 100644 --- a/spec/facts_spec.rb +++ b/spec/facts_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe 'override_facts' do @@ -9,9 +11,9 @@ 'release' => { 'full' => '7.7.1908', 'major' => '7', - 'minor' => '7' + 'minor' => '7', }, - } + }, } end @@ -24,9 +26,9 @@ 'release' => { 'full' => '7.7.1908', 'major' => '7', - 'minor' => '7' + 'minor' => '7', }, - } + }, } end @@ -42,16 +44,16 @@ 'release' => { 'full' => '7.7.1908', 'major' => '7', - 'minor' => '7' + 'minor' => '7', }, }, ruby: { 'sitedir' => '/usr/local/share/ruby/site_ruby', - } + }, } end - it { expect(override_facts(base_facts, ruby: {sitedir: '/usr/local/share/ruby/site_ruby'})).to eq(expected) } + it { expect(override_facts(base_facts, ruby: { sitedir: '/usr/local/share/ruby/site_ruby' })).to eq(expected) } end describe 'with deep merging' do @@ -63,13 +65,13 @@ 'release' => { 'full' => '7.7.1908', 'major' => '7', - 'minor' => '8' + 'minor' => '8', }, - } + }, } end - it { expect(override_facts(base_facts, os: {release: {minor: '8'}})).to eq(expected) } + it { expect(override_facts(base_facts, os: { release: { minor: '8' } })).to eq(expected) } end describe 'with strings' do @@ -81,19 +83,19 @@ 'release' => { 'full' => '7.7.1908', 'major' => '7', - 'minor' => '8' + 'minor' => '8', }, - } + }, } end - it { expect(override_facts(base_facts, os: {'release' => {minor: '8'}})).to eq(expected) } + it { expect(override_facts(base_facts, os: { 'release' => { minor: '8' } })).to eq(expected) } end end describe 'add_facts_for_metadata' do - before(:each) { RspecPuppetFacts.reset } - after(:each) { RspecPuppetFacts.reset } + before { RspecPuppetFacts.reset } + after { RspecPuppetFacts.reset } let(:metadata) do { 'dependencies' => dependencies } @@ -111,7 +113,7 @@ context 'with systemd' do let(:dependencies) do [ - {'name' => 'puppet/systemd'}, + { 'name' => 'puppet/systemd' }, ] end @@ -123,24 +125,20 @@ context 'and stdlib' do let(:dependencies) do [ - {'name' => 'puppetlabs/stdlib'}, - {'name' => 'puppet/systemd'}, + { 'name' => 'puppetlabs/stdlib' }, + { 'name' => 'puppet/systemd' }, ] end it 'has systemd on Red Hat 9' do add_facts_for_metadata(metadata) - facts = RspecPuppetFacts.with_custom_facts('redhat-9-x86_64', { - os: { 'family' => 'RedHat', 'release' => { 'major' => '9' } } - }) + facts = RspecPuppetFacts.with_custom_facts('redhat-9-x86_64', { os: { 'family' => 'RedHat', 'release' => { 'major' => '9' } } }) expect(facts[:systemd]).to be true end it 'has no systemd on openbsd' do add_facts_for_metadata(metadata) - facts = RspecPuppetFacts.with_custom_facts('openbsd-7-x86_64', { - os: { 'family' => 'OpenBSD' } - }) + facts = RspecPuppetFacts.with_custom_facts('openbsd-7-x86_64', { os: { 'family' => 'OpenBSD' } }) expect(facts[:systemd]).to be false end end @@ -149,13 +147,15 @@ context 'with stdlib' do let(:dependencies) do [ - {'name' => 'puppetlabs/stdlib'}, + { 'name' => 'puppetlabs/stdlib' }, ] end it 'adds the systemd fact' do - expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:puppet_environmentpath, '/etc/puppetlabs/code/environments', {}) - expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:puppet_vardir, '/opt/puppetlabs/puppet/cache', {}) + expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:puppet_environmentpath, + '/etc/puppetlabs/code/environments', {}) + expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:puppet_vardir, '/opt/puppetlabs/puppet/cache', + {}) expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:root_home, '/root', {}) expect(RspecPuppetFacts).to receive(:register_custom_fact).with(:service_provider, instance_of(Proc), {}) add_facts_for_metadata(metadata) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f1b254d..ea94163 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + begin require 'simplecov' require 'simplecov-console' diff --git a/voxpupuli-test.gemspec b/voxpupuli-test.gemspec index 41def7a..7e18776 100644 --- a/voxpupuli-test.gemspec +++ b/voxpupuli-test.gemspec @@ -1,4 +1,4 @@ -# -*- encoding: utf-8 -*- +# frozen_string_literal: true Gem::Specification.new do |s| s.name = 'voxpupuli-test' @@ -32,8 +32,8 @@ Gem::Specification.new do |s| # newest versions that still support Ruby 2.6 # jruby 9.3 in Puppetserver 7 is compatible with C Ruby 2.6 s.add_runtime_dependency 'rubocop', '~> 1.50.0' - s.add_runtime_dependency 'rubocop-rspec', '~> 2.20.0' s.add_runtime_dependency 'rubocop-rake', '~> 0.6.0' + s.add_runtime_dependency 'rubocop-rspec', '~> 2.20.0' # Linting # meta gem to pull in all puppet-lint plugins + puppet-lint itself From aa61dc417dcf9d16f37d7a982fe77d9e363128c2 Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Fri, 21 Jun 2024 12:25:04 +0200 Subject: [PATCH 3/3] rubocop: Fix Style/FrozenStringLiteralComment --- .rubocop.yml | 3 +++ .rubocop_todo.yml | 30 ++++-------------------------- lib/voxpupuli/test/facts.rb | 2 ++ lib/voxpupuli/test/rake.rb | 4 +++- lib/voxpupuli/test/spec_helper.rb | 2 ++ 5 files changed, 14 insertions(+), 27 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 3953d40..8c9c81e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -35,4 +35,7 @@ Style/TrailingCommaInArguments: Enabled: true EnforcedStyleForMultiline: comma +Style/IfUnlessModifier: + Enabled: false + inherit_from: .rubocop_todo.yml diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 69b8d56..e2058e2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,37 +1,25 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-06-21 09:28:53 UTC using RuboCop version 1.50.2. +# on 2024-06-21 10:27:03 UTC using RuboCop version 1.50.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 -# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. -Lint/DuplicateBranch: - Exclude: - - 'lib/voxpupuli/test/facts.rb' - -# Offense count: 1 -# Configuration parameters: AllowComments, AllowNil. -Lint/SuppressedException: - Exclude: - - 'Rakefile' - # Offense count: 1 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 19 + Max: 18 # Offense count: 1 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/CyclomaticComplexity: - Max: 12 + Max: 10 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: - Max: 28 + Max: 24 # Offense count: 1 # Configuration parameters: Prefixes, AllowedPatterns. @@ -71,16 +59,6 @@ RSpec/MultipleDescribes: RSpec/MultipleExpectations: Max: 4 -# Offense count: 3 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: always, always_true, never -Style/FrozenStringLiteralComment: - Exclude: - - 'lib/voxpupuli/test/facts.rb' - - 'lib/voxpupuli/test/rake.rb' - - 'lib/voxpupuli/test/spec_helper.rb' - # Offense count: 1 Style/MixinUsage: Exclude: diff --git a/lib/voxpupuli/test/facts.rb b/lib/voxpupuli/test/facts.rb index e8fa424..057b396 100644 --- a/lib/voxpupuli/test/facts.rb +++ b/lib/voxpupuli/test/facts.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rspec-puppet-facts' include RspecPuppetFacts diff --git a/lib/voxpupuli/test/rake.rb b/lib/voxpupuli/test/rake.rb index 4d6270a..52dd3b4 100644 --- a/lib/voxpupuli/test/rake.rb +++ b/lib/voxpupuli/test/rake.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'puppetlabs_spec_helper/rake_tasks' PuppetLint.configuration.log_format = '%s:%s:%s:%s:%s' @@ -18,7 +20,7 @@ next if filename =~ %r{^((modules|acceptance|\.?vendor|spec/fixtures|pkg)/|REFERENCE.md)} File.foreach(filename).each_with_index do |line, index| - if line =~ %r{\s\n$} + if line =~ /\s\n$/ errors << "#{filename} has trailing whitespace on line #{index + 1}" end end diff --git a/lib/voxpupuli/test/spec_helper.rb b/lib/voxpupuli/test/spec_helper.rb index 68b030a..bffda89 100644 --- a/lib/voxpupuli/test/spec_helper.rb +++ b/lib/voxpupuli/test/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'voxpupuli/test/facts' require 'puppetlabs_spec_helper/module_spec_helper'