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.

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

Allow ::php::config::setting value to be "undef" #654

Closed
wants to merge 2 commits into from

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jan 13, 2022

This is necessary so that config settings can be removed (at least as a workaround for #653).

This is necessary so that config settings can be removed (at least as a workaround for voxpupuli#653).
@mpdude
Copy link
Contributor Author

mpdude commented Jan 13, 2022

I think I need some help or starting pointers regarding tests... :-(

@smortex
Copy link
Member

smortex commented Jan 14, 2022

php::config::setting is private so cannot be tested directly. You can however add tests for php::config: they live in spec/defines/config_spec.rb. Here is an example around existing code, feel free to add more test cases:

diff --git a/spec/defines/config_spec.rb b/spec/defines/config_spec.rb
index 77976bb..5cc15e1 100644
--- a/spec/defines/config_spec.rb
+++ b/spec/defines/config_spec.rb
@@ -83,6 +83,15 @@ describe 'php::config' do
           end
 
           it { is_expected.to contain_php__config('unique-name').with_file('/etc/php5/conf.d/unique-name.ini') }
+          it do
+            is_expected.to contain_ini_setting('/etc/php5/conf.d/unique-name.ini: apc.enabled').with(
+              'ensure'  => 'present',
+              'path'    => '/etc/php5/conf.d/unique-name.ini',
+              'section' => '',
+              'setting' => 'apc.enabled',
+              'value'   => 1,
+            )
+          end
         end
 
         context 'empty array' do

@mpdude
Copy link
Contributor Author

mpdude commented Jan 17, 2022

@smortex Thank you!

I have looked at that file, but cannot find another example of how to initialize (stage; setup fixture) pre-exisiting config file contents.

Also, what is contain_ini_setting from your diff? I cannot find that being used elsewhere.

@smortex
Copy link
Member

smortex commented Jan 17, 2022

The example in my diff above is I think the kind of test that is missing: your new parameter allows to manage the ensure parameter of the ini_setting resource, and a test should me added to test this. But since there are currently no tests for the ini_setting resource, I added one for the first test case to show how it works and so you can write more.

After applying this diff, you can run the test suite with:

bundle install # if you have not already done this
bundle exec rake spec

Tests are performed with https://rspec-puppet.com/ and contain_ini_setting() is used to test for a ini_setting resource, see: https://rspec-puppet.com/documentation/classes/#test-a-resource

Does it help?

@root-expert
Copy link
Member

Was implemented at #647

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants