From 48f8ffa472650520caed0e99e9824837836b3f29 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 5 Jan 2021 20:01:54 +0100 Subject: [PATCH 1/2] (maint) - Use the correct matches in rspec Rather than checking the notify/require properties, it uses the matchers that check the dependency graph. That means it can also check transitive requires/notifies. It also uses compile and compile.and_raise_error as a shorter way to check for errors (or the absense of them). --- spec/classes/apache_spec.rb | 79 +++++++++++----------------- spec/classes/mod/authnz_ldap_spec.rb | 4 +- spec/classes/mod/data_spec.rb | 4 +- spec/classes/mod/peruser_spec.rb | 6 +-- spec/classes/mod/php_spec.rb | 2 +- spec/classes/mod/remoteip_spec.rb | 2 +- spec/classes/mod/ssl_spec.rb | 2 +- spec/classes/mod/status_spec.rb | 16 ++---- spec/classes/mod/wsgi_spec.rb | 4 +- 9 files changed, 44 insertions(+), 75 deletions(-) diff --git a/spec/classes/apache_spec.rb b/spec/classes/apache_spec.rb index 11865838f4..ba858ae0b6 100644 --- a/spec/classes/apache_spec.rb +++ b/spec/classes/apache_spec.rb @@ -35,29 +35,26 @@ it { is_expected.to contain_file('/etc/apache2/sites-enabled').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_file('/etc/apache2/mods-enabled').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_file('/etc/apache2/mods-available').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'false', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'false' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_concat('/etc/apache2/ports.conf').with( 'owner' => 'root', 'group' => 'root', - 'mode' => '0644', 'notify' => 'Class[Apache::Service]' - ) + 'mode' => '0644' + ).that_notifies('Class[Apache::Service]') } # Assert that load files are placed and symlinked for these mods, but no conf file. ['auth_basic', 'authn_file', 'authz_default', 'authz_groupfile', 'authz_host', 'authz_user', 'dav', 'env'].each do |modname| @@ -313,15 +310,14 @@ it { is_expected.to contain_file('/etc/httpd/conf.d').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_concat('/etc/httpd/conf/ports.conf').with( 'owner' => 'root', 'group' => 'root', - 'mode' => '0644', 'notify' => 'Class[Apache::Service]' - ) + 'mode' => '0644' + ).that_notifies('Class[Apache::Service]') } describe 'Alternate confd/mod/vhosts directory' do let :params do @@ -336,9 +332,8 @@ it { is_expected.to contain_file("/etc/httpd/#{dir}").with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } end @@ -509,9 +504,7 @@ it { is_expected.to contain_file('/opt/rh/root/etc/httpd/conf/httpd.conf').with( 'ensure' => 'file', - 'notify' => 'Class[Apache::Service]', - 'require' => ['Package[httpd]', 'Concat[/etc/httpd/conf/ports.conf]'], - ) + ).that_notifies('Class[Apache::Service]').that_requires(['Package[httpd]', 'Concat[/etc/httpd/conf/ports.conf]']) } end @@ -523,9 +516,8 @@ it { is_expected.to contain_file('/etc/httpd/special_conf.d').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true', + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } end @@ -729,22 +721,20 @@ it { is_expected.to contain_file('/usr/local/etc/apache24/Vhosts').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_file('/usr/local/etc/apache24/Modules').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_concat('/usr/local/etc/apache24/ports.conf').with( 'owner' => 'root', 'group' => 'wheel', - 'mode' => '0644', 'notify' => 'Class[Apache::Service]' - ) + 'mode' => '0644', + ).that_notifies('Class[Apache::Service]') } # Assert that load files are placed for these mods, but no conf file. ['auth_basic', 'authn_core', 'authn_file', 'authz_groupfile', 'authz_host', 'authz_user', 'dav', 'env'].each do |modname| @@ -798,22 +788,20 @@ it { is_expected.to contain_file('/etc/apache2/vhosts.d').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_file('/etc/apache2/modules.d').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', 'notify' => 'Class[Apache::Service]', - 'require' => 'Package[httpd]' - ) + 'purge' => 'true' + ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } it { is_expected.to contain_concat('/etc/apache2/ports.conf').with( 'owner' => 'root', 'group' => 'wheel', - 'mode' => '0644', 'notify' => 'Class[Apache::Service]' - ) + 'mode' => '0644', + ).that_notifies('Class[Apache::Service]') } end context 'on all OSes' do @@ -838,10 +826,9 @@ it { is_expected.to contain_package('httpd').with( - 'notify' => 'Class[Apache::Service]', 'ensure' => 'installed', 'name' => 'httpd24-httpd', - ) + ).that_notifies('Class[Apache::Service]') } end context 'with a custom file_mode parameter' do @@ -919,10 +906,6 @@ is_pe: false } end - it do - expect { - catalogue - }.to raise_error(Puppet::Error, %r{Unsupported osfamily}) - end + it { is_expected.to compile.and_raise_error(%r{Unsupported osfamily}) } end end diff --git a/spec/classes/mod/authnz_ldap_spec.rb b/spec/classes/mod/authnz_ldap_spec.rb index 307b16d795..e6ecc50f03 100644 --- a/spec/classes/mod/authnz_ldap_spec.rb +++ b/spec/classes/mod/authnz_ldap_spec.rb @@ -37,7 +37,7 @@ let(:params) { { verify_server_cert: 'wrong' } } it 'raises an error' do - expect { is_expected.to raise_error Puppet::Error } + is_expected.to compile.and_raise_error(%r{parameter 'verify_server_cert' expects a Boolean value, got String}) end end end # Debian @@ -74,7 +74,7 @@ let(:params) { { verify_server_cert: 'wrong' } } it 'raises an error' do - expect { is_expected.to raise_error Puppet::Error } + is_expected.to compile.and_raise_error(%r{parameter 'verify_server_cert' expects a Boolean value, got String}) end end diff --git a/spec/classes/mod/data_spec.rb b/spec/classes/mod/data_spec.rb index 226918d7e1..4511996f4f 100644 --- a/spec/classes/mod/data_spec.rb +++ b/spec/classes/mod/data_spec.rb @@ -26,9 +26,7 @@ { apache_version: '2.2' } end - it 'fails' do - expect { catalogue }.to raise_error(Puppet::Error, %r{mod_data is only available in Apache 2.3 and later}) - end + it { is_expected.to compile.and_raise_error(%r{mod_data is only available in Apache 2.3 and later}) } end end end diff --git a/spec/classes/mod/peruser_spec.rb b/spec/classes/mod/peruser_spec.rb index d5f2a93a1f..2533a28bab 100644 --- a/spec/classes/mod/peruser_spec.rb +++ b/spec/classes/mod/peruser_spec.rb @@ -20,11 +20,7 @@ } end - it do - expect { - catalogue - }.to raise_error(Puppet::Error, %r{Unsupported osfamily FreeBSD}) - end + it { is_expected.to compile.and_raise_error(%r{Unsupported osfamily FreeBSD}) } end context 'on a Gentoo OS' do let :facts do diff --git a/spec/classes/mod/php_spec.rb b/spec/classes/mod/php_spec.rb index a7d6fe88c4..05a6de020f 100644 --- a/spec/classes/mod/php_spec.rb +++ b/spec/classes/mod/php_spec.rb @@ -292,7 +292,7 @@ end it 'raises an error' do - expect { expect(subject).to contain_apache__mod('php5') }.to raise_error Puppet::Error, %r{mpm_module => 'prefork' or mpm_module => 'itk'} + is_expected.to compile.and_raise_error(%r{mpm_module => 'prefork' or mpm_module => 'itk'}) end end end diff --git a/spec/classes/mod/remoteip_spec.rb b/spec/classes/mod/remoteip_spec.rb index d9cc055eb3..3de472cbc9 100644 --- a/spec/classes/mod/remoteip_spec.rb +++ b/spec/classes/mod/remoteip_spec.rb @@ -119,7 +119,7 @@ { apache_version: '2.2' } end - it { expect { catalogue }.to raise_error(Puppet::Error, %r{mod_remoteip is only available in Apache 2.4}) } + it { is_expected.to compile.and_raise_error(%r{mod_remoteip is only available in Apache 2.4}) } end end end diff --git a/spec/classes/mod/ssl_spec.rb b/spec/classes/mod/ssl_spec.rb index 48f4683fd2..36972364ca 100644 --- a/spec/classes/mod/ssl_spec.rb +++ b/spec/classes/mod/ssl_spec.rb @@ -17,7 +17,7 @@ } end - it { expect { catalogue }.to raise_error(Puppet::Error, %r{Unsupported osfamily:}) } + it { is_expected.to compile.and_raise_error(%r{Unsupported osfamily:}) } end context 'on a RedHat' do diff --git a/spec/classes/mod/status_spec.rb b/spec/classes/mod/status_spec.rb index f4741a9991..11fb459935 100644 --- a/spec/classes/mod/status_spec.rb +++ b/spec/classes/mod/status_spec.rb @@ -274,9 +274,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to succeed array validation' do - expect { - is_expected.to contain_file('status.conf') - }.not_to raise_error + is_expected.to compile end end @@ -297,9 +295,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to fail array validation' do - expect { - is_expected.to contain_file('status.conf') - }.to raise_error(Puppet::Error) + is_expected.to compile.and_raise_error(/allow_from/) end end @@ -323,9 +319,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to succeed regular expression validation' do - expect { - is_expected.to contain_file('status.conf') - }.not_to raise_error + is_expected.to compile end end end @@ -348,9 +342,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to fail regular expression validation' do - expect { - is_expected.to contain_file('status.conf') - }.to raise_error(Puppet::Error) + is_expected.to compile.and_raise_error(/extended_status/) end end end diff --git a/spec/classes/mod/wsgi_spec.rb b/spec/classes/mod/wsgi_spec.rb index ba1314ebad..fb018099cd 100644 --- a/spec/classes/mod/wsgi_spec.rb +++ b/spec/classes/mod/wsgi_spec.rb @@ -128,7 +128,7 @@ } end - it { expect { catalogue }.to raise_error Puppet::Error, %r{apache::mod::wsgi - both package_name and mod_path must be specified!} } + it { is_expected.to compile.and_raise_error(%r{apache::mod::wsgi - both package_name and mod_path must be specified!}) } end describe 'with mod_path but no package_name' do let :params do @@ -137,7 +137,7 @@ } end - it { expect { catalogue }.to raise_error Puppet::Error, %r{apache::mod::wsgi - both package_name and mod_path must be specified!} } + it { is_expected.to compile.and_raise_error(%r{apache::mod::wsgi - both package_name and mod_path must be specified!}) } end end context 'on a FreeBSD OS' do From 16a4cfdb03ff6ba9212c73973bab8d5b07d8522c Mon Sep 17 00:00:00 2001 From: David Swan Date: Mon, 25 Jan 2021 09:48:55 +0000 Subject: [PATCH 2/2] (maint) - minor Rubocop fixes --- spec/classes/apache_spec.rb | 8 ++++---- spec/classes/mod/status_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/classes/apache_spec.rb b/spec/classes/apache_spec.rb index ba858ae0b6..f6a14c9747 100644 --- a/spec/classes/apache_spec.rb +++ b/spec/classes/apache_spec.rb @@ -503,7 +503,7 @@ it { is_expected.to contain_file('/opt/rh/root/etc/httpd/conf/httpd.conf').with( - 'ensure' => 'file', + 'ensure' => 'file', ).that_notifies('Class[Apache::Service]').that_requires(['Package[httpd]', 'Concat[/etc/httpd/conf/ports.conf]']) } end @@ -516,7 +516,7 @@ it { is_expected.to contain_file('/etc/httpd/special_conf.d').with( 'ensure' => 'directory', 'recurse' => 'true', - 'purge' => 'true', + 'purge' => 'true' ).that_notifies('Class[Apache::Service]').that_requires('Package[httpd]') } end @@ -733,7 +733,7 @@ it { is_expected.to contain_concat('/usr/local/etc/apache24/ports.conf').with( 'owner' => 'root', 'group' => 'wheel', - 'mode' => '0644', + 'mode' => '0644' ).that_notifies('Class[Apache::Service]') } # Assert that load files are placed for these mods, but no conf file. @@ -800,7 +800,7 @@ it { is_expected.to contain_concat('/etc/apache2/ports.conf').with( 'owner' => 'root', 'group' => 'wheel', - 'mode' => '0644', + 'mode' => '0644' ).that_notifies('Class[Apache::Service]') } end diff --git a/spec/classes/mod/status_spec.rb b/spec/classes/mod/status_spec.rb index 11fb459935..5db47bb4c6 100644 --- a/spec/classes/mod/status_spec.rb +++ b/spec/classes/mod/status_spec.rb @@ -295,7 +295,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to fail array validation' do - is_expected.to compile.and_raise_error(/allow_from/) + is_expected.to compile.and_raise_error(%r{allow_from}) end end @@ -342,7 +342,7 @@ def status_conf_spec_require(requires, extended_status, status_path) end it 'expects to fail regular expression validation' do - is_expected.to compile.and_raise_error(/extended_status/) + is_expected.to compile.and_raise_error(%r{extended_status}) end end end