Refactor plugin specs to reduce boiler plate #697
Conversation
This removes a lot of the boiler plate in our plugin tests.
|
voxpupuli/rspec-puppet-facts#132 is the same thing, but more generalized. |
| describe_plugin 'foreman_proxy::plugin::abrt' do | ||
| describe 'with default settings' do | ||
| include_examples 'a plugin with a settings file', 'abrt' do | ||
| let(:expected_config) { /:enabled: https/ } |
There was a problem hiding this comment.
Generally I like the examples tactic here as it simplifies the boilerplate. I think it can take some experience to unwind include_examples sometimes. I would probably prefer if each spec declared this in the same way with my preference being https://github.com/theforeman/puppet-foreman_proxy/pull/697/files#diff-622e312bc4439aacb45352c217bcd7880af426efd40e1e8f6dc27501eab16769R8-R14 because it makes it clear its a yaml file and declares it -- less mental overhead when reading through these or as developers copy/paste for new plugins.
There was a problem hiding this comment.
For what it's worth, I kept the change minimal here. The old example was also this regex. I'm not opposed to an explicit long form everywhere, at least for default settings. For the case where we disable it and only want to ensure it's :enabled: false I think the regex form is still better.
I'm not entirely sure if this is a good idea. It does have some magic feeling to it. On the other hand, specs are more readable this way (IMHO). I'm interested in feedback.
I only converted a few specs, but hopefully that's enough to see the pattern.