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

:nocov: is ignored with simplecov #246

Open
ereslibre opened this issue Oct 26, 2017 · 9 comments
Open

:nocov: is ignored with simplecov #246

ereslibre opened this issue Oct 26, 2017 · 9 comments

Comments

@ereslibre
Copy link

ereslibre commented Oct 26, 2017

Hello,

This project is supposed to have 100% coverage. If I execute it locally:

smartbox-io/cell@master * make spec
docker run --rm -v `pwd`:/cell -it cell:latest bundle exec rspec spec
(snip)
Coverage report generated for RSpec to /cell/coverage. 173 / 173 LOC (100.0%) covered.

Travis has also this result: https://www.travis-ci.org/smartbox-io/cell/builds/292355378#L714

One line is marked as red in code climate: https://codeclimate.com/github/smartbox-io/cell/coverage/59efccf0f817224314000779, the problem is that as you can see, this method is wrapped with :nocov:, and the coverage percentage drops when it should not.

My best guess is that .resultset.json does not report this line is skipped in any way, only that it was not covered (0 passes):

      "/cell/app/controllers/api/v1/objects_controller.rb": [
(snip)
        0,
(snip)

Simplecov on its UI (the static page it generates) mark them as skipped, but I don't know if this only happens on the html renderer (I didn't look at simplecov source code).

@ale7714
Copy link
Contributor

ale7714 commented Oct 26, 2017

Hi @ereslibre, thanks for trying out the new test_reporter and reporting this issue.

I think your hunch is on the right direction. I will do a little more investigation and discuss with the team a plan to address this.

Can you share with us your simplecov output? It's usually really helpful to have real life examples on where to test our fixes. If you don't feel comfortable sharing it here, you can also do it on our support channel at http://www.codeclimate.com/help.

Again thank you and hopefully we will have a resolution for this soon.

@ereslibre
Copy link
Author

The project is libre software so you should be able to even reproduce the problem yourselves: https://github.com/smartbox-io/cell

$ make build
$ make spec

Attaching the output here too: coverage.zip

I'm wondering if I can open a PR against simplecov so :nocov: surrounded lines are outputted on the json file with a -1 count, effectively marking that we don't care about the passes they got. I don't know if such a change would be accepted in simplecov though.

@ale7714
Copy link
Contributor

ale7714 commented Oct 26, 2017

@ereslibre gracias!
This a good point and I wonder if the desired behavior on Simplecov would be returning null instead of 0 for line 34 which is inside of the :nocov: tag.
Simplecov returns null for lines where coverage does not matter like comments or blank lines and i think that would apply for lines inside :nocov: as well.

I think is worth opening an issue on Simplecov and discuss what is the best path forward with the maintainer. What do you think?

@ereslibre
Copy link
Author

I think is worth opening an issue on Simplecov and discuss what is the best path forward with the maintainer. What do you think?

Sure, I will do that and have a look. Thank you for your wonderful feedback!

@ale7714
Copy link
Contributor

ale7714 commented Oct 26, 2017

I'm going to keep this issue open while we find a resolution for this.

PS: going to remove the bug label for now. Since it looks like simplecov is reporting an inaccurate coverage value on .resultset.json

@ereslibre
Copy link
Author

ereslibre commented Oct 27, 2017

@ale7714 As you can see I opened a PR.

I open a discussion, because maybe what makes more sense is to add an option to generate a nocov file that on codeclimate you can suggest to turn on. This file would be similar to the resultset.json file, only that it would mark ignored lines.

This way, if a user wants to use simplecov in the general case they would have this nocov file option disabled (also would be the default).

@ale7714
Copy link
Contributor

ale7714 commented Oct 28, 2017

@ereslibre thank you for opening that PR and starting the discussion.

@wfleming wfleming removed their assignment Nov 20, 2017
@DannyBen
Copy link

DannyBen commented Apr 18, 2018

Hey guys,

Following a request from @efueger, I am posting here another example of this failure, if it helps to bring this issue back to the radar, since I am a bit OCD when it comes to my test coverage being at 100%...

  • The repo in question is loadrunner
  • The coverage is 100% in development
  • The coverage is 100% on Travis, as can be seen here
  • The only single line that is not covered is in the ServerBase class
  • CodeClimate "salts my game" by complaining about this line, as can be seen on this CodeClimate report
  • I marked this line # :nocov: - eventhough Travis reported 100% coverage even without it.

If any additional information is needed, I am happy to provide.

@wfleming
Copy link
Contributor

wfleming commented Sep 4, 2018

I'm marking this as help wanted. Simplecov has made it clear they don't intend to stabilize on a file format that includes this information and can be used by external tools, and their expectation is that any tools integrating with Simplecov should be a ruby gem that can work as a formatter for simplecov. Such a gem is unfortunately beyond the scope of what our test-reporter can include out-of-the-box.

If anybody impacted by this issue is interested in helping out by writing a custom formatter for Simplecov and working with us to upgrade the test reporter to work with the output of that formatter, please let us know.

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

No branches or pull requests

6 participants