From 769d68c809139bde61ffb5066c99307805b34bf6 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 22 Feb 2022 12:14:29 +0100 Subject: [PATCH 1/4] Deal with non-AIO installs in acceptance tests Archlinux doesn't have an AIO installation so the puppet_gem provider doesn't work. Using the regular gem provider should work. The aio_agent_version fact is used to determine whether it's an AIO install. --- spec/acceptance/suites/default/redisget_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/acceptance/suites/default/redisget_spec.rb b/spec/acceptance/suites/default/redisget_spec.rb index cae32421..6f7eeb25 100644 --- a/spec/acceptance/suites/default/redisget_spec.rb +++ b/spec/acceptance/suites/default/redisget_spec.rb @@ -10,7 +10,7 @@ package { 'redis-rubygem' : ensure => '3.3.3', name => 'redis', - provider => 'puppet_gem', + provider => if fact('aio_agent_version') =~ String[1] { 'puppet_gem' } else { 'gem' }, } EOS From 45fac54ff4d047274ad5d40dc63e60b8eca4ebdf Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 7 Feb 2022 19:46:28 +0100 Subject: [PATCH 2/4] Drop Debian backport acceptance test Originally introduced in 63251bcf37005f0372cfe2a5fc95f432235f4b40. However, on current Debian systems the /var/run/redis directory is no longer shared. bc37527b0b01ff203afd3248e8e5a65edab47a4e took care of that. The versions that affect are no longer from backports anyway. Fixes: bc37527b0b01ff203afd3248e8e5a65edab47a4e --- .../default/redis_debian_run_dir_spec.rb | 57 ------------------- 1 file changed, 57 deletions(-) delete mode 100644 spec/acceptance/suites/default/redis_debian_run_dir_spec.rb diff --git a/spec/acceptance/suites/default/redis_debian_run_dir_spec.rb b/spec/acceptance/suites/default/redis_debian_run_dir_spec.rb deleted file mode 100644 index 857518a0..00000000 --- a/spec/acceptance/suites/default/redis_debian_run_dir_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper_acceptance' - -# since this test polutes others, we'll only run it if specifically asked -if ENV['RUN_BACKPORT_TEST'] == 'yes' - describe 'redis', if: (fact('operatingsystem') == 'Debian') do - it 'runs with newer Debian package' do - pp = <<-EOS - - include ::apt - - class {'::apt::backports':} - -> - file { '/usr/sbin/policy-rc.d': - ensure => present, - content => "/usr/bin/env sh\nexit 101", - mode => '0755', - } - -> - package { 'redis-server': - ensure => 'latest', - install_options => { - '-t' => "${::lsbdistcodename}-backports", - }, - } - -> - class { 'redis': - manage_package => false, - } - EOS - - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_change: true) - end - - describe package('redis-server') do - it { is_expected.to be_installed } - end - - describe service('redis-server') do - it { is_expected.to be_running } - end - - context 'redis should respond to ping command' do - describe command('redis-cli ping') do - its(:stdout) { is_expected.to match %r{PONG} } - end - end - - context 'redis log should be clean' do - describe command('journalctl --no-pager') do - its(:stdout) { is_expected.not_to match %r{Failed at step RUNTIME_DIRECTORY} } - end - end - end -end From 99e8e2f32875081faebc5c6ddf6f4379714690a3 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 7 Feb 2022 16:45:14 +0100 Subject: [PATCH 3/4] Modernize the acceptance tests --- spec/acceptance/redis_cli_task_spec.rb | 28 +++--- .../default/redis_adminstration_spec.rb | 32 ++++--- .../redis_multi_instances_one_host_spec.rb | 93 +++++++++---------- .../default/redis_sentinel_one_node_spec.rb | 66 +++++-------- spec/acceptance/suites/default/redis_spec.rb | 33 ++----- .../suites/default/redisget_spec.rb | 36 +++---- spec/acceptance/suites/scl/redis5_spec.rb | 29 ++---- 7 files changed, 135 insertions(+), 182 deletions(-) diff --git a/spec/acceptance/redis_cli_task_spec.rb b/spec/acceptance/redis_cli_task_spec.rb index 0eb3ea0e..5e74218e 100644 --- a/spec/acceptance/redis_cli_task_spec.rb +++ b/spec/acceptance/redis_cli_task_spec.rb @@ -9,22 +9,18 @@ let(:task_name) { 'redis::redis_cli' } - it 'install redis-cli with the class' do - pp = <<-EOS - include redis - EOS - - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) - end - unless fact('os.family') == 'RedHat' && fact('os.release.major').to_i >= 9 + include_examples 'an idempotent resource' do + let(:manifest) { 'include redis' } + end + describe 'ping' do let(:params) { 'command="ping"' } it 'execute ping' do - is_expected.to match(%r{{\s*"status":\s*"PONG"\s*}}) - is_expected.to match(%r{Ran on 1 target in .+ sec}) + is_expected. + to match(%r{{\s*"status":\s*"PONG"\s*}}). + and match(%r{Ran on 1 target in .+ sec}) end end @@ -33,8 +29,9 @@ let(:params) { 'command="ping; cat /etc/passwd"' } it 'stops script injections and escapes' do - is_expected.to match(%r!{\s*"status":\s*"ERR unknown command ('|`)ping; cat /etc/passwd('|`)!) - is_expected.to match(%r{Ran on 1 target in .+ sec}) + is_expected. + to match(%r!{\s*"status":\s*"ERR unknown command ('|`)ping; cat /etc/passwd('|`)!). + and match(%r{Ran on 1 target in .+ sec}) end end @@ -42,8 +39,9 @@ let(:params) { 'command="ping && cat /etc/passwd"' } it 'stops script injections and escapes' do - is_expected.to match(%r!{\s*"status":\s*"ERR unknown command ('|`)ping && cat /etc/passwd('|`)!) - is_expected.to match(%r{Ran on 1 target in .+ sec}) + is_expected. + to match(%r!{\s*"status":\s*"ERR unknown command ('|`)ping && cat /etc/passwd('|`)!). + and match(%r{Ran on 1 target in .+ sec}) end end end diff --git a/spec/acceptance/suites/default/redis_adminstration_spec.rb b/spec/acceptance/suites/default/redis_adminstration_spec.rb index ca60d08d..16841eb6 100644 --- a/spec/acceptance/suites/default/redis_adminstration_spec.rb +++ b/spec/acceptance/suites/default/redis_adminstration_spec.rb @@ -2,29 +2,31 @@ require 'spec_helper_acceptance' +RSpec::Matchers.define_negated_matcher :execute_without_warning, :execute_with_warning + # systcl settings are untestable in docker describe 'redis::administration', unless: default['hypervisor'] =~ %r{docker} do - it 'runs successfully' do - pp = <<-EOS - include redis - include redis::administration - EOS + def execute_with_warning + have_attributes(stderr: %r{WARNING}) + end - # Apply twice to ensure no errors the second time. - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) + include_examples 'an idempotent resource' do + let(:manifest) { 'include redis, redis::administration' } end - describe file('/proc/sys/vm/overcommit_memory') do - its(:content) { is_expected.to eq("1\n") } + specify do + expect(file('/proc/sys/vm/overcommit_memory')). + to have_attributes(content: "1\n") end - describe file('/proc/sys/net/core/somaxconn') do - its(:content) { is_expected.to eq("65535\n") } + specify do + expect(file('/proc/sys/net/core/somaxconn')). + to have_attributes(content: "65535\n") end - describe command('timeout 1s redis-server --port 7777 --loglevel verbose') do - its(:stderr) { is_expected.not_to match(%r{WARNING}) } - its(:exit_status) { is_expected.to eq(124) } + specify do + expect(command('timeout 1s redis-server --port 7777 --loglevel verbose')). + to execute_without_warning. + and have_attributes(exit_status: 124) end end diff --git a/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb b/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb index 24307317..5c4b5c21 100644 --- a/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb +++ b/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb @@ -29,66 +29,61 @@ 'redis' end - it 'runs successfully' do - pp = <<-EOS - $listening_ports = #{instances} - - class { 'redis': - default_install => false, - service_enable => false, - service_ensure => 'stopped', - protected_mode => false, - bind => [], - } - - $listening_ports.each |$port| { - $port_string = sprintf('%d',$port) - redis::instance { $port_string: - service_enable => true, - service_ensure => 'running', - port => $port, - bind => $facts['networking']['ip'], - dbfilename => "${port}-dump.rdb", - appendfilename => "${port}-appendonly.aof", - appendfsync => 'always', - require => Class['Redis'], - } - } - - EOS - - # Apply twice to ensure no errors the second time. - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) + include_examples 'an idempotent resource' do + let(:manifest) do + <<~PUPPET + $listening_ports = #{instances} + + class { 'redis': + default_install => false, + service_enable => false, + service_ensure => 'stopped', + protected_mode => false, + bind => [], + } + + $listening_ports.each |$port| { + $port_string = sprintf('%d',$port) + redis::instance { $port_string: + service_enable => true, + service_ensure => 'running', + port => $port, + bind => $facts['networking']['ip'], + dbfilename => "${port}-dump.rdb", + appendfilename => "${port}-appendonly.aof", + appendfsync => 'always', + require => Class['Redis'], + } + } + PUPPET + end end - describe package(redis_name) do - it { is_expected.to be_installed } - end + specify { expect(package(redis_name)).to be_installed } - describe service(redis_name) do - it { is_expected.not_to be_enabled } - it { is_expected.not_to be_running } + specify do + expect(service(redis_name)).not_to be_enabled + expect(service(redis_name)).not_to be_running end instances.each do |instance| - describe file("/etc/systemd/system/redis-server-#{instance}.service") do - its(:content) { is_expected.to match %r{redis-server-#{instance}.conf} } + specify do + expect(file("/etc/systemd/system/redis-server-#{instance}.service")). + to be_file. + and have_attributes(content: include("redis-server-#{instance}.conf")) end - describe service("redis-server-#{instance}") do - it { is_expected.to be_enabled } - it { is_expected.to be_running } - end + specify { expect(service("redis-server-#{instance}")).to be_enabled.and be_running } - describe file("#{config_path}/redis-server-#{instance}.conf") do - its(:content) { is_expected.to match %r{port #{instance}} } + specify do + expect(file("#{config_path}/redis-server-#{instance}.conf")). + to be_file. + and have_attributes(content: include("port #{instance}")) end - context "redis instance #{instance} should respond to ping command" do - describe command("redis-cli -h #{fact('networking.ip')} -p #{instance} ping") do - its(:stdout) { is_expected.to match %r{PONG} } - end + specify "redis instance #{instance} should respond to ping command" do + expect(command("redis-cli -h #{fact('networking.ip')} -p #{instance} ping")). + to have_attributes(stdout: %r{PONG}) end end end diff --git a/spec/acceptance/suites/default/redis_sentinel_one_node_spec.rb b/spec/acceptance/suites/default/redis_sentinel_one_node_spec.rb index daf1463e..d061e26e 100644 --- a/spec/acceptance/suites/default/redis_sentinel_one_node_spec.rb +++ b/spec/acceptance/suites/default/redis_sentinel_one_node_spec.rb @@ -3,54 +3,38 @@ require 'spec_helper_acceptance' describe 'redis::sentinel' do - redis_name = case fact('osfamily') - when 'Debian' - 'redis-server' - else - 'redis' - end - - it 'runs successfully' do - pp = <<-EOS - class { 'redis::sentinel': - master_name => 'mymaster', - redis_host => '127.0.0.1', - failover_timeout => 10000, - } - EOS - - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) - end - - describe package(redis_name) do - it { is_expected.to be_installed } + redis_name = fact('os.family') == 'Debian' ? 'redis-server' : 'redis' + + include_examples 'an idempotent resource' do + let(:manifest) do + <<-PUPPET + class { 'redis::sentinel': + master_name => 'mymaster', + redis_host => '127.0.0.1', + failover_timeout => 10000, + } + PUPPET + end end - describe service(redis_name) do - it { is_expected.to be_running } - end + specify { expect(package(redis_name)).to be_installed } + specify { expect(service(redis_name)).to be_running } - describe service('redis-sentinel'), :sentinel do - it { is_expected.to be_running } + specify 'redis should respond to ping command' do + expect(command('redis-cli ping')). + to have_attributes(stdout: %r{PONG}) end - case fact('osfamily') - when 'Debian' - describe package('redis-sentinel') do - it { is_expected.to be_installed } - end - end + specify { expect(service('redis-sentinel')).to be_running } - context 'redis should respond to ping command' do - describe command('redis-cli ping') do - its(:stdout) { is_expected.to match %r{PONG} } - end + if redis_name == 'redis-server' + specify { expect(package('redis-sentinel')).to be_installed } + else + specify { expect(package('redis-sentinel')).not_to be_installed } end - context 'redis-sentinel should return correct sentinel master' do - describe command('redis-cli -p 26379 SENTINEL masters') do - its(:stdout) { is_expected.to match %r{^mymaster} } - end + specify 'redis-sentinel should return correct sentinel master' do + expect(command('redis-cli -p 26379 SENTINEL masters')). + to have_attributes(stdout: %r{^mymaster}) end end diff --git a/spec/acceptance/suites/default/redis_spec.rb b/spec/acceptance/suites/default/redis_spec.rb index 58d04414..21aa7d21 100644 --- a/spec/acceptance/suites/default/redis_spec.rb +++ b/spec/acceptance/suites/default/redis_spec.rb @@ -3,21 +3,10 @@ require 'spec_helper_acceptance' describe 'redis' do - redis_name = case fact('osfamily') - when 'Debian' - 'redis-server' - else - 'redis' - end - - it 'runs successfully' do - pp = <<-EOS - include redis - EOS + redis_name = fact('os.family') == 'Debian' ? 'redis-server' : 'redis' - # Apply twice to ensure no errors the second time. - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) + include_examples 'an idempotent resource' do + let(:manifest) { 'include redis' } end it 'returns a fact' do @@ -31,18 +20,12 @@ end end - describe package(redis_name) do - it { is_expected.to be_installed } - end - - describe service(redis_name) do - it { is_expected.to be_running } - end + specify { expect(package(redis_name)).to be_installed } + specify { expect(service(redis_name)).to be_running } - context 'redis should respond to ping command' do - describe command('redis-cli ping') do - its(:stdout) { is_expected.to match %r{PONG} } - end + specify 'redis should respond to ping command' do + expect(command('redis-cli ping')). + to have_attributes(stdout: %r{PONG}) end it 'runs successfully when using Redis apt repository', if: (fact('os.family') == 'Debian') do diff --git a/spec/acceptance/suites/default/redisget_spec.rb b/spec/acceptance/suites/default/redisget_spec.rb index 6f7eeb25..312f105a 100644 --- a/spec/acceptance/suites/default/redisget_spec.rb +++ b/spec/acceptance/suites/default/redisget_spec.rb @@ -3,28 +3,28 @@ require 'spec_helper_acceptance' describe 'redis::get() function' do - it 'runs successfully' do - pp = <<-EOS - include redis - - package { 'redis-rubygem' : - ensure => '3.3.3', - name => 'redis', - provider => if fact('aio_agent_version') =~ String[1] { 'puppet_gem' } else { 'gem' }, - } - EOS - - # Apply twice to ensure no errors the second time. - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) + include_examples 'an idempotent resource' do + let(:manifest) do + <<-PUPPET + include redis + + package { 'redis-rubygem' : + ensure => '3.3.3', + name => 'redis', + provider => if fact('aio_agent_version') =~ String[1] { 'puppet_gem' } else { 'gem' }, + } + PUPPET + end end - describe command('redis-cli SET mykey "Hello"') do - its(:stdout) { is_expected.to match(%r{OK}) } + specify do + expect(command('redis-cli SET mykey "Hello"')). + to have_attributes(stdout: %r{OK}) end - describe command('redis-cli GET mykey') do - its(:stdout) { is_expected.to match('Hello') } + specify do + expect(command('redis-cli GET mykey')). + to have_attributes(stdout: %r{Hello}) end context 'with mykey set to Hello' do diff --git a/spec/acceptance/suites/scl/redis5_spec.rb b/spec/acceptance/suites/scl/redis5_spec.rb index ca3a7699..7379a382 100644 --- a/spec/acceptance/suites/scl/redis5_spec.rb +++ b/spec/acceptance/suites/scl/redis5_spec.rb @@ -11,33 +11,24 @@ apply_manifest('service{"rh-redis5-redis" : ensure => stopped, enable => false}') end - it 'runs successfully' do - pp = <<-PUPPET + include_examples 'an idempotent resource' do + let(:manifest) do + <<-PUPPET class { 'redis::globals': scl => 'rh-redis5', } class { 'redis': manage_repo => true, } - PUPPET - - # Apply twice to ensure no errors the second time. - apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: true) - end - - describe package('rh-redis5-redis') do - it { is_expected.to be_installed } + PUPPET + end end - describe service('rh-redis5-redis') do - it { is_expected.to be_running } - it { is_expected.to be_enabled } - end + specify { expect(package('rh-redis5-redis')).to be_installed } + specify { expect(service('rh-redis5-redis')).to be_running.and be_enabled } - context 'redis should respond to ping command' do - describe command('scl enable rh-redis5 -- redis-cli ping') do - its(:stdout) { is_expected.to match %r{PONG} } - end + it 'redis should respond to ping command' do + expect(command('scl enable rh-redis5 -- redis-cli ping')). + to have_attributes(stdout: %r{PONG}) end end From 5047ec5213d04442104571b74bced2a2126e7d9c Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sat, 17 Feb 2024 01:46:58 +0100 Subject: [PATCH 4/4] Move away from legacy facts in the test suite --- spec/classes/redis_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/classes/redis_spec.rb b/spec/classes/redis_spec.rb index 9629a42a..6d72a062 100644 --- a/spec/classes/redis_spec.rb +++ b/spec/classes/redis_spec.rb @@ -74,7 +74,7 @@ class { 'redis::globals': it { is_expected.to compile.with_all_deps } - if facts[:operatingsystem] == 'CentOS' + if facts[:os]['name'] == 'CentOS' it { is_expected.to contain_package('centos-release-scl-rh') } else it { is_expected.not_to contain_package('centos-release-scl-rh') } @@ -501,7 +501,7 @@ class { 'redis': describe 'with parameter: manage_repo' do let(:params) { { manage_repo: true } } - if facts[:osfamily] == 'RedHat' && facts[:os]['release']['major'].to_i <= 7 + if facts[:os]['family'] == 'RedHat' && facts[:os]['release']['major'].to_i <= 7 it { is_expected.to contain_class('epel') } else it { is_expected.not_to contain_class('epel') } @@ -510,7 +510,7 @@ class { 'redis': describe 'with ppa' do let(:params) { super().merge(ppa_repo: 'ppa:rwky/redis') } - if facts[:operatingsystem] == 'Ubuntu' + if facts[:os]['name'] == 'Ubuntu' it { is_expected.to contain_apt__ppa('ppa:rwky/redis') } else it { is_expected.not_to contain_apt__ppa('ppa:rwky/redis') }