Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Puppet >= 4.10 #50

Merged
merged 4 commits into from Apr 17, 2017
Merged

Support Puppet >= 4.10 #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb276f7
Support Puppet >= 4.10
Feb 28, 2017
e6472cc
'pe' is no longer supported by the Forge
Apr 11, 2017
5aeaafb
Deoxidise Rubocop
Apr 11, 2017
25a40c6
Satisfy Rubocop
Apr 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions .rubocop.yml
View file
Open in desktop
Expand Up @@ -9,10 +9,21 @@ AllCops:

# Cop's to ignore


# It thinks that Ruby version dependend definitions are duplicates
Bundler/DuplicatedGem:
Exclude:
- "Gemfile"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see rubocop/rubocop#3752 if interessted in more details

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, what a mess. Thanks for figuring this out.

# With this enabled it suggests a change that will break the Gemfile
Lint/AssignmentInCondition:
Enabled: false

# It would need a complete rewrite of the test to get this cop satisfied
Metrics/BlockLength:
Exclude:
- "spec/**/*.rb"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see rubocop/rubocop#3772 if interessted in more details

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why disable entirely on spec tests? Is the idea that we just use it for actual ruby code in lib/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cop get's angry about code blocks with more than 25 lines. It would need a complete refactor to satisfy him. The longest block is 1487 lines at the moment.

I think this cop is good for real ruby code but not needed on the spec tests itself.

Metrics/LineLength:
Enabled: false

Expand All @@ -28,6 +39,11 @@ Style/Next:
Style/NonNilCheck:
Enabled: false

# This cop isn't compatible with Ruby < 2.0
Style/SymbolArray:
Exclude:
- "Rakefile"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see https://travis-ci.org/ghoneycutt/puppet-module-nscd/builds/220867192

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ignore the Rakefile? I thought that since it is ruby, it should be tested. I'm not sure what I was supposed to get from looking at that Travis link.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cop enforces to use a syntax that is not available on Ruby < 2.0.
You can see the actual error message in one of the failed tests [1].

[1] https://travis-ci.org/ghoneycutt/puppet-module-nscd/jobs/220867193#L213-L216


Style/TrailingCommaInLiteral:
Enabled: false

Expand Down
18 changes: 18 additions & 0 deletions .travis.yml
View file
Open in desktop
Expand Up @@ -28,6 +28,8 @@ env:
- PUPPET_GEM_VERSION="~> 4.6.0" CHECK=test
- PUPPET_GEM_VERSION="~> 4.7.0" CHECK=test
- PUPPET_GEM_VERSION="~> 4.8.0" CHECK=test
- PUPPET_GEM_VERSION="~> 4.9.0" CHECK=test
- PUPPET_GEM_VERSION="~> 4.10.0" CHECK=test
- PUPPET_GEM_VERSION="~> 4" CHECK=test
- PUPPET_GEM_VERSION="~> 4" CHECK=rubocop

Expand Down Expand Up @@ -69,6 +71,22 @@ matrix:
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.8.0" CHECK=test
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.9.0" CHECK=test
- rvm: 1.9.3
env: PUPPET_GEM_VERSION="~> 4.9.0" CHECK=test
- rvm: 2.0.0
env: PUPPET_GEM_VERSION="~> 4.9.0" CHECK=test
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.10.0" CHECK=test
- rvm: 1.9.3
env: PUPPET_GEM_VERSION="~> 4.10.0" CHECK=test
- rvm: 2.0.0
env: PUPPET_GEM_VERSION="~> 4.10.0" CHECK=test
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4" CHECK=test
- rvm: 1.9.3
env: PUPPET_GEM_VERSION="~> 4" CHECK=test
- rvm: 2.0.0
env: PUPPET_GEM_VERSION="~> 4" CHECK=test
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4" CHECK=rubocop
Expand Down
22 changes: 13 additions & 9 deletions Gemfile
View file
Open in desktop
Expand Up @@ -6,9 +6,7 @@ else
gem 'puppet', :require => false
end

gem 'puppetlabs_spec_helper', '>= 1.2.0'
gem 'facter', '>= 1.7.0'
gem 'rspec-puppet'
gem 'puppet-lint', '~> 2.0'
gem 'puppet-lint-absolute_classname-check'
gem 'puppet-lint-alias-check'
Expand All @@ -21,11 +19,17 @@ gem 'puppet-lint-trailing_comma-check'
gem 'puppet-lint-undef_in_function-check'
gem 'puppet-lint-unquoted_string-check'
gem 'puppet-lint-variable_contains_upcase'
gem 'rspec-puppet'

gem 'json', '<= 1.8', :require => false if RUBY_VERSION < '2.0.0'
gem 'json_pure', '<= 2.0.1', :require => false if RUBY_VERSION < '2.0.0'
gem 'metadata-json-lint', '0.0.11', :require => false if RUBY_VERSION < '1.9'
gem 'metadata-json-lint' if RUBY_VERSION >= '1.9'
gem 'parallel_tests', '<= 2.9.0', :require => false if RUBY_VERSION < '2.0.0' # [1]
gem 'puppetlabs_spec_helper', '2.0.2', :require => false if RUBY_VERSION >= '1.8.7' && RUBY_VERSION < '1.9' # [1]
gem 'puppetlabs_spec_helper', '>= 2.0.0', :require => false if RUBY_VERSION >= '1.9' # [1]
gem 'rake', '~> 10.0', :require => false if RUBY_VERSION >= '1.8.7' && RUBY_VERSION < '1.9'
gem 'rspec', '~> 2.0', :require => false if RUBY_VERSION >= '1.8.7' && RUBY_VERSION < '1.9'
gem 'rubocop', :require => false if RUBY_VERSION >= '2.0.0'

gem 'rspec', '~> 2.0' if RUBY_VERSION >= '1.8.7' && RUBY_VERSION < '1.9'
gem 'rake', '~> 10.0' if RUBY_VERSION >= '1.8.7' && RUBY_VERSION < '1.9'
gem 'json', '<= 1.8' if RUBY_VERSION < '2.0.0'
gem 'json_pure', '<= 2.0.1' if RUBY_VERSION < '2.0.0'
gem 'rubocop' if RUBY_VERSION >= '2.0.0'
gem 'metadata-json-lint', '0.0.11' if RUBY_VERSION < '1.9'
gem 'metadata-json-lint' if RUBY_VERSION >= '1.9'
# [1]: Puppetlabs is dropping support for Ruby 1.8.7 in latests releases, pin to last supported version when running on Ruby 1.8.7
4 changes: 0 additions & 4 deletions metadata.json
View file
Open in desktop
Expand Up @@ -9,10 +9,6 @@
"issues_url": "https://github.com/ghoneycutt/puppet-module-nscd/issues",
"description": "Manage every aspect of NSCD - name service cache daemon",
"requirements": [
{
"name": "pe",
"version_requirement": ">= 3.2.0 < 5.0.0"
},
{
"name": "puppet",
"version_requirement": ">= 3.0.0 < 5.0.0"
Expand Down
42 changes: 21 additions & 21 deletions spec/classes/init_spec.rb
View file
Open in desktop
Expand Up @@ -399,7 +399,7 @@
},
}

services_solaris = %w(
services_solaris = %w[
audit_user
auth_attr
bootparams
Expand All @@ -416,7 +416,7 @@
tnrhdb
tnrhtp
user_attr
)
]

platforms.sort.each do |_k, v|
describe "on #{v[:osfamily]} #{v[:operatingsystemmajrelease]}#{v[:operatingsystemrelease]} with default values for all parameters" do
Expand Down Expand Up @@ -740,7 +740,7 @@
end

context 'as an array' do
let(:params) { { :package_name => %w(nscd foo) } }
let(:params) { { :package_name => %w[nscd foo] } }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like rubocop nonsense

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't like that, I can turn of the responding cop too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do turn off the cop that wants to change all the %w() to %w[]. This way we reduce code churn and make other PR's easier to merge. Funny thing is we went to %w() because of rubocop :>

Other than that, everything looks good! Thanks!

let(:facts) { { :osfamily => 'Debian' } }

it { should contain_package('nscd').with({ 'ensure' => 'present' }) }
Expand All @@ -761,7 +761,7 @@

describe 'with package_ensure parameter' do
context 'set to all possible valid values' do
%w(present installed absent).each do |ensure_value|
%w[present installed absent].each do |ensure_value|
context "package_ensure => #{ensure_value}" do
let(:params) { { :package_ensure => ensure_value } }
let(:facts) { { :osfamily => 'Debian' } }
Expand Down Expand Up @@ -812,7 +812,7 @@
end

context 'as an invalid type' do
let(:params) { { :config_owner => %w(invalid root) } }
let(:params) { { :config_owner => %w[invalid root] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand All @@ -832,7 +832,7 @@
end

context 'as an invalid type' do
let(:params) { { :config_group => %w(invalid root) } }
let(:params) { { :config_group => %w[invalid root] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand Down Expand Up @@ -896,7 +896,7 @@

describe 'with service_ensure parameter' do
context 'set to all possible valid values' do
%w(present running absent stopped).each do |ensure_value|
%w[present running absent stopped].each do |ensure_value|
context "service_ensure => #{ensure_value}" do
let(:params) { { :service_ensure => ensure_value } }
let(:facts) { { :osfamily => 'Debian' } }
Expand Down Expand Up @@ -942,7 +942,7 @@
end

context 'set to invalid type' do
let(:params) { { :service_enable => %w(invalid type) } }
let(:params) { { :service_enable => %w[invalid type] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand All @@ -962,7 +962,7 @@
end

context 'as an invalid type' do
let(:params) { { :service_provider => %w(not a string) } }
let(:params) { { :service_provider => %w[not a string] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand All @@ -973,7 +973,7 @@
end
end

%w(passwd group hosts services netgroup).each do |service|
%w[passwd group hosts services netgroup].each do |service|
describe "with enable_db_#{service}" do
[true, 'true', false, 'false'].each do |value|
context "set to valid value #{value}" do
Expand All @@ -985,7 +985,7 @@
end

context 'set to an invalid type (non-boolean or string convertible to boolean)' do
let(:params) { { :"enable_db_#{service}" => %w(invalid type) } }
let(:params) { { :"enable_db_#{service}" => %w[invalid type] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand All @@ -1008,7 +1008,7 @@
end

context 'set to an invalid type (non-boolean or string convertible to boolean)' do
let(:params) { { :enable_opt_auto_propagate => %w(invalid type) } }
let(:params) { { :enable_opt_auto_propagate => %w[invalid type] } }
let(:facts) { { :osfamily => 'Debian' } }

it 'should fail' do
Expand Down Expand Up @@ -1220,7 +1220,7 @@
end

describe 'with paranoia parameter specified' do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let(:params) { { :paranoia => value } }
let(:facts) { { :osfamily => 'Debian' } }
Expand Down Expand Up @@ -1274,9 +1274,9 @@
end
end

%w(passwd group hosts services netgroup).each do |service|
%w[passwd group hosts services netgroup].each do |service|
describe "with #{service}_enable_cache specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let :params do
{
Expand Down Expand Up @@ -1413,7 +1413,7 @@
end

describe "with #{service}_check_files specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let :params do
{
Expand Down Expand Up @@ -1442,7 +1442,7 @@
end

describe "with #{service}_persistent specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let :params do
{
Expand Down Expand Up @@ -1471,7 +1471,7 @@
end

describe "with #{service}_shared specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let :params do
{
Expand Down Expand Up @@ -1538,7 +1538,7 @@
# only the passwd and group services use auto-propogate
if service == 'passwd' || service == 'group'
describe "with #{service}_auto_propagate specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let :params do
{
Expand Down Expand Up @@ -1570,7 +1570,7 @@

services_solaris.each do |service|
describe "with #{service}_enable_cache specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let(:params) { { :"#{service}_enable_cache" => value } }
let(:facts) { { :osfamily => 'Solaris', :kernelrelease => '5.10' } }
Expand Down Expand Up @@ -1656,7 +1656,7 @@
end

describe "with #{service}_check_files specified" do
%w(yes no).each do |value|
%w[yes no].each do |value|
context "as valid value #{value}" do
let(:params) { { :"#{service}_check_files" => value } }
let(:facts) { { :osfamily => 'Solaris', :kernelrelease => '5.10' } }
Expand Down