Implement support for method coverage

[PragTob]

Ruby 2.5 didn't just add branch coverage but also method coverage (see here for instance: https://blog.bigbinary.com/2018/04/11/ruby-2-5-supports-measuring-branch-and-method-coverages.html)

Would be nice to have some support for it as well.

cc: @tycooon (you mentioned you had it implemented in your fork already)

[tycooon]

Yes, I am going to work on creating PR with that functionality.

[PragTob]

@tycooon that'd be amazing, but don't feel pressured. Just mentioned you since you said you had it :)

[klyonrad]

That's another thing where you/we might discuss on how to actually show the coverage of methods.

Currently: when a file is loaded, all the def method_name lines are considered as a hit, the actual code in the method is shown as red and the closing of the method is also not considered as a hit (from the article)

Coverage was disabled for line number 7 as it contains just end keyword.

It always itched me that just declaring the method names are recorded as a hit or miss (except when nothing loads the file, which is the default for a rails project).

Anyway, that's maybe a distraction from the original topic...

What exactly should be shown for the method coverage? How does it affect the total big number of coverage a file/the project (that metric that we all care about). How do dynamically defined methods work with this? Playing devil's advocate: What does it even matter - I mean, what decisions would made knowing that a file has x amount of uncovered methods?

[tycooon]

In my experience, method coverage mostly helps finding unused methods that can be safely deleted in case you have 100% line (and probably branch) coverage.

For example you might have attr_reader :foo, :bar where only :foo is actually used. Or simply attr_reader :bar that is never used. In both cases only method coverage can find such stuff.

[klyonrad]

Wait a second, you can detect unused attr_reader methods? That's amazing! Thanks for clearing that up.

Then I would say that for an unused method the def method_name line should also stay red - even when the LineCoverage API reports it as covered

[tycooon]

Well, I guess it's possible to do that, but maybe we can somehow separate the presentation of different coverage types so that they don't interfere.

For example, in HTML report we could add some extra styling for method names that are not covered. Not sure about that though 🤔

[klyonrad]

But that would make it complicated for the users. Why should they care if it is LineCoverage or MethodCoverage? It doesn't matter. The method is not called, so it is not covered.

The actual complicated part for display would be to show the different coverage of methods from the line attr_reader :foo, :bar (for example the :foo is red and the rest is green)

[PragTob]

Everything is complicated :sweat_smile:

I'm still not fully sure how to deal with all these different coverage types also display wise, which is probably the hardest.

Right now the approach I'm going for is that you can configure which coverage criteria to use and then those will be displayed along with potentially a "primary coverage criterion" which will be the biggest number displayed/checked against. Not 100% sure.

As for why defs are considered "covered" - that's the data the underlying coverage library returns to us. Wouldn't want to mess with those (usually).

[tycooon]

Personally I would prefer to be able to configure each minimal coverage percentage independently and see them all in results. This gives more flexibility and control than selecting just one "primary" coverage type. Method coverage, for example, does not suit that role well :)

[PragTob]

@tycooon whatever I do, that will also be possible at some point. But usually you want to report one coverage as "overall" coverage (on CLI, big and bold at the top of the HTML report).

[tycooon]

@PragTob Just wanted to let you know that I am currently working on method coverage and it's close to be ready (however, I have problem with cucumber tests, see https://github.com/simplecov-ruby/simplecov/issues/925).

By the way, I decided to add all coverage info to the console output, so it looks like this:

$ rspec
........

Finished in 0.02921 seconds (files took 0.50518 seconds to load)
8 examples, 0 failures

Coverage report generated for RSpec to /Users/tycooon/code/simplecov/tmp/testproj/coverage
Line coverage: 56 / 61 (91.80%)
Branch coverage: 2 / 4 (50.00%)
Method coverage: 10 / 13 (76.92%)

What do you think about it? Also it's a bit odd, that this is actually implemented in simplecov-html gem, maybe we should move it to the main gem?

[PragTob]

Hi @tycooon

sorry for the long silence. Being new at Shopify is a lot so I often don't have the power in the evening :D + some busy weekends, I tried to spend what time I could at fixing the tests that were bogging you down but got nowhere but now... they seemingly started working again? So I got to finally look at all the stuff (my thinking: if the tests don't work I can't merge PRs anyhow...).

yes that it lives in the HTML-formatter doesn't make a ton of sense... not sure it's worth moving now (if you want to, sure) - I want to move it to more of a mono repo anyhow but still moving it would be nice. If I had one wish though (hard to fulfill!) it'd be a separate PR because method coverage itself is likely already HUGE.

Also thanks for your work :green_heart: :tada: