Add options for rspec-puppet coverage reports #174
Conversation
|
This is an alternative to #149 |
|
I created https://tickets.puppetlabs.com/browse/PDK-1249 to document the inability to test this. |
README.md
Outdated
| @@ -168,6 +168,8 @@ Travis uses a .travis.yml file in the root of your repository to learn about you | |||
| |mock_with|Defaults to `':mocha'`. Recommended to be set to `':rspec'`, which uses RSpec's built-in mocking library, instead of a third-party one.| | |||
| |spec_overrides|An array of extra lines to add into your `spec_helper.rb`. Can be used as an alternative to `spec_helper_local`| | |||
| |strict_level| Defines the [Puppet Strict configuration parameter](https://puppet.com/docs/puppet/5.4/configuration.html#strict). Defaults to `:warning`. Other values are: `:error` and `:off`. `:error` provides strictest level checking and is encouraged.| | |||
| |enable_coverage_reports|Enable [rspec-puppet coverage reports](https://rspec-puppet.com/documentation/coverage/). Defaults to `false`| | |||
There was a problem hiding this comment.
This is a nit, but since it will be a public option I prefer thecoverage_report option name from #149 , especially with it being a boolean value.
There was a problem hiding this comment.
can do, will push a fix for that
moduleroot/spec/spec_helper.rb.erb
Outdated
| @@ -43,6 +43,11 @@ RSpec.configure do |c| | |||
| Puppet.settings[:strict] = <%= @configs['strict_level'] %> | |||
| end | |||
| <%- end -%> | |||
| c.after(:suite) do | |||
| <%- if @configs['enable_coverage_reports'] -%> | |||
There was a problem hiding this comment.
I think the conditional should wrap the entire c.after(:suite) block, so that it's not an empty block if coverage_report is false.
There was a problem hiding this comment.
I put it inside the block since an empty block shouldn't hurt anything and this sets the file up for others adding additional things to this block. If it wraps the entire thing then the next contributor would have to adjust this code too. If you feel strongly about it I will change it though.
There was a problem hiding this comment.
Mostly just a stylistic nit, since the rendered file could potentially have an empty block, which isn't a big deal. No change necessary.
genebean
force-pushed
the
code_coverage_reports
branch
from
4ab2c50
to
ef5f962
Compare
genebean
force-pushed
the
code_coverage_reports
branch
2 times, most recently
from
a5976be
to
7342405
Compare
|
Rebased to resolve merge conflict |
Two options are added: - coverage_report: Defaults to false - minimum_code_coverage_percentage: Defaults to 0
genebean
force-pushed
the
code_coverage_reports
branch
from
7342405
to
d3004fa
Compare
Two options are added:
Wasn't sure how to test this since all the things I could find for using a custom template only allowed pulling from master...