From a261a3d493b661057576292ec967f9d344b43359 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Mon, 18 Jan 2016 12:19:21 +0000 Subject: [PATCH 1/3] (maint) fix logic for choosing rspec version Depending on the ruby version, we either need 3.1.7 to support ruby 1.8, but since (at least) ruby 2.2 a newer version of rspec is required. --- Gemfile | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 839dcc0..ee382e2 100644 --- a/Gemfile +++ b/Gemfile @@ -11,11 +11,12 @@ def location_for(place, fake_version = nil) end group :development, :unit_tests do - # rspec must be v2 for ruby 1.8.7 - if RUBY_VERSION >= '1.8.7' and RUBY_VERSION < '1.9' - gem 'rspec', '~> 2.0' + # rspec-core 3.1.7 is the last version to support ruby 1.8 + if RUBY_VERSION < '1.9' + gem 'rspec-core', '3.1.7' else - gem 'rspec-core', '3.1.7', :require => false + # newer version required to avoid BKR-537 + gem 'rspec-core', '>= 3.4' end gem 'puppetlabs_spec_helper', :require => false From 2de4597ecd5daafdbe8647206edae045531a400e Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Mon, 18 Jan 2016 14:21:51 +0000 Subject: [PATCH 2/3] (maint) refactor main acceptance suite This reduces duplication of testing code and makes the intent of the tests clearer. --- spec/acceptance/mysql_db_spec.rb | 46 ++++++++++++---------------- spec/acceptance/mysql_server_spec.rb | 38 +++++++++-------------- spec/spec_helper_acceptance.rb | 10 ++++++ 3 files changed, 45 insertions(+), 49 deletions(-) diff --git a/spec/acceptance/mysql_db_spec.rb b/spec/acceptance/mysql_db_spec.rb index 88ded3a..8c57160 100644 --- a/spec/acceptance/mysql_db_spec.rb +++ b/spec/acceptance/mysql_db_spec.rb @@ -2,28 +2,26 @@ require 'spec_helper_acceptance' describe 'mysql::db define' do describe 'creating a database' do - # Using puppet_apply as a helper - it 'should work with no errors' do - pp = <<-EOS + let(:pp) do + <<-EOS class { 'mysql::server': root_password => 'password' } mysql::db { 'spec1': user => 'root1', password => 'password', } EOS + end + it_behaves_like "a idempotent resource" - # Run it twice and test for idempotency - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - - expect(shell("mysql -e 'show databases;'|grep spec1").exit_code).to be_zero + describe command("mysql -e 'show databases;'") do + its(:exit_status) { is_expected.to eq 0 } + its(:stdout) { is_expected.to match /^spec1$/ } end end describe 'creating a database with post-sql' do - # Using puppet_apply as a helper - it 'should work with no errors' do - pp = <<-EOS + let(:pp) do + <<-EOS class { 'mysql::server': override_options => { 'root_password' => 'password' } } file { '/tmp/spec.sql': ensure => file, @@ -36,21 +34,19 @@ describe 'mysql::db define' do sql => '/tmp/spec.sql', } EOS - - # Run it twice and test for idempotency - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) end + it_behaves_like "a idempotent resource" - it 'should have the table' do - expect(shell("mysql -e 'show tables;' spec2|grep table1").exit_code).to be_zero + describe command("mysql -e 'show tables;' spec2") do + its(:exit_status) { is_expected.to eq 0 } + its(:stdout) { is_expected.to match /^table1$/ } end end describe 'creating a database with dbname parameter' do - # Using puppet_apply as a helper - it 'should work with no errors' do - pp = <<-EOS + let(:check_command) { " | grep realdb" } + let(:pp) do + <<-EOS class { 'mysql::server': override_options => { 'root_password' => 'password' } } mysql::db { 'spec1': user => 'root1', @@ -58,14 +54,12 @@ describe 'mysql::db define' do dbname => 'realdb', } EOS - - # Run it twice and test for idempotency - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) end + it_behaves_like "a idempotent resource" - it 'should have the database named realdb' do - expect(shell("mysql -e 'show databases;'|grep realdb").exit_code).to be_zero + describe command("mysql -e 'show databases;'") do + its(:exit_status) { is_expected.to eq 0 } + its(:stdout) { is_expected.to match /^realdb$/ } end end end diff --git a/spec/acceptance/mysql_server_spec.rb b/spec/acceptance/mysql_server_spec.rb index e19be55..8589ea2 100644 --- a/spec/acceptance/mysql_server_spec.rb +++ b/spec/acceptance/mysql_server_spec.rb @@ -1,12 +1,10 @@ require 'spec_helper_acceptance' describe 'mysql class' do - - describe 'running puppet code' do - # Using puppet_apply as a helper - it 'should work with no errors' do - tmpdir = default.tmpdir('mysql') - pp = <<-EOS + describe 'advanced config' do + let(:tmpdir) { default.tmpdir('mysql') } + let(:pp) do + <<-EOS class { 'mysql::server': config_file => '#{tmpdir}/my.cnf', includedir => '#{tmpdir}/include', @@ -46,39 +44,33 @@ describe 'mysql class' do } } EOS - - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) end + + it_behaves_like "a idempotent resource" end - describe 'configuration needed for syslog' do - it 'should work with no errors' do - pp = <<-EOS + describe 'syslog configuration' do + let(:pp) do + <<-EOS class { 'mysql::server': override_options => { 'mysqld' => { 'log-error' => undef }, 'mysqld_safe' => { 'log-error' => false, 'syslog' => true }}, } EOS - - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) end + + it_behaves_like "a idempotent resource" end - describe 'when changing the password' do + context 'when changing the password' do let(:password) { 'THE NEW SECRET' } - let(:manifest) { "class { 'mysql::server': root_password => '#{password}' }" } + let(:pp) { "class { 'mysql::server': root_password => '#{password}' }" } it 'should not display the password' do - result = apply_manifest(manifest, :expect_changes => true) + result = apply_manifest(pp, :catch_failures => true) # this does not actually prove anything, as show_diff in the puppet config defaults to false. expect(result.stdout).not_to match /#{password}/ end - it 'should be idempotent' do - result = apply_manifest(manifest, :catch_changes => true) - end - + it_behaves_like "a idempotent resource" end - end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index d232013..e9f138d 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -47,3 +47,13 @@ RSpec.configure do |c| end end end + +shared_examples "a idempotent resource" do + it 'should apply with no errors' do + apply_manifest(pp, :catch_failures => true) + end + + it 'should apply a second time without changes' do + apply_manifest(pp, :catch_changes => true) + end +end From d89062faff9d62808c8b47f3404a3323f6153104 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Mon, 18 Jan 2016 15:06:00 +0000 Subject: [PATCH 3/3] (FM-4563) skip idempotency tests on test cells that do have PUP-5016 unfixed Arguably this decision should be extracted to a more central place, like puppetlabs_spec_helper or the CI config. --- spec/spec_helper_acceptance.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index e9f138d..d25f6a8 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -12,6 +12,12 @@ RSpec.configure do |c| # Readable test descriptions c.formatter = :documentation + # detect the situation where PUP-5016 is triggered and skip the idempotency tests in that case + # also note how fact('puppetversion') is not available because of PUP-4359 + if fact('osfamily') == 'Debian' && fact('operatingsystemmajrelease') == '8' && shell('puppet --version').stdout =~ /^4\.2/ + c.filter_run_excluding :skip_pup_5016 => true + end + # Configure all nodes in nodeset c.before :suite do # Install module and dependencies @@ -53,7 +59,7 @@ shared_examples "a idempotent resource" do apply_manifest(pp, :catch_failures => true) end - it 'should apply a second time without changes' do + it 'should apply a second time without changes', :skip_pup_5016 do apply_manifest(pp, :catch_changes => true) end end