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

Remove SimpleCove filter #263

Merged
merged 1 commit into from
Nov 6, 2016
Merged

Remove SimpleCove filter #263

merged 1 commit into from
Nov 6, 2016

Conversation

dhoppe
Copy link
Member

@dhoppe dhoppe commented Nov 6, 2016

The SimpleCov filter is not necessary and will result in the following report.

Coverage report generated for RSpec to ../coverage. 0.0 / 0.0 LOC (100.0%) covered.

COVERAGE: 100.00% -- 0.0/0.0 lines in 0 files

As you can see, no file will be processed and the module will always have a coverage of 100%.

@bastelfreak
Copy link
Member

what is the purpose of the change? Do you want to run simplecove on the spec tests? If so, we should exclude /spec/fixtures?

@dhoppe
Copy link
Member Author

dhoppe commented Nov 6, 2016

@bastelfreak You are right, but the filter still needs to be adjusted.

@bastelfreak bastelfreak merged commit 7feeeee into voxpupuli:master Nov 6, 2016
@alexjfisher
Copy link
Member

Simplecov is only for ruby code coverage. If a module has no native ruby code (types/providers, functions etc), then I'd expect it to report 0.0/0.0 lines in 0 files. Getting it to see what lines of code in spec have been covered doesn't make sense to me.
It might make sense to remove the coveralls badge from modules containing no ruby code.

@dhoppe
Copy link
Member Author

dhoppe commented Nov 7, 2016

@alexjfisher I disagree, because RSpec tests are written in Ruby (.rb) and this issue was only about a broken filter.

add_filter '/spec'

Coverage report generated for RSpec to /Users/dhoppe/Projects/puppet/voxpupuli/puppet-rundeck/coverage. 0.0 / 0.0 LOC (100.0%) covered.

COVERAGE: 100.00% -- 0.0/0.0 lines in 0 files

add_filter 'spec/fixtures'

Coverage report generated for RSpec to /Users/dhoppe/Projects/puppet/voxpupuli/puppet-rundeck/coverage. 734 / 736 LOC (99.73%) covered.

COVERAGE:  99.73% -- 734/736 lines in 22 files

+----------+-------------------------------------------+-------+--------+---------+
| coverage | file                                      | lines | missed | missing |
+----------+-------------------------------------------+-------+--------+---------+
|  85.71%  | spec/defines/config/securityroles_spec.rb | 14    | 2      | 11, 19  |
+----------+-------------------------------------------+-------+--------+---------+
21 file(s) with 100% coverage not shown

@alexjfisher
Copy link
Member

We're interested in the coverage of the module code (the code under test), not the test code itself.

@dhoppe
Copy link
Member Author

dhoppe commented Nov 7, 2016

Ah, now Puppet Labs has spoken. So if we do not care about the code coverage of tests, why do not we just remove the build step of Rubocop at Travis CI?

@alexjfisher
Copy link
Member

We do care about the code coverage of types and providers, functions, facts. eg https://travis-ci.org/voxpupuli/puppet-corosync/jobs/171357494#L1512

@alexjfisher
Copy link
Member

I'm keen on reverting this. I really don't think we want the end coverage figure to be polluted by the coverage of the ruby in the spec tests themselves (which is likely to always be 100%?). For example, a module might contain a type/provider with no (or very limited) unit tests. With this in, every time a new rspec-puppet spec test is added, the coverage report will show increased code coverage. In reality, the type/provider, (the thing we care about testing), is still not tested, but the stats look good/better anyway.

I'm going to open a PR to revert, but don't want it merged if there isn't a consensus.

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