Support for minimal coverage and maximal coverage drop

[infertux]

Here is a proposal to implement two new features:

• return non-zero if the coverage is below a defined threshold (min_coverage)
• return non-zero if the coverage has dropped by more than a defined threshold since the last build (max_coverage_difference)

Please let me know if you're happy with this so I would add some explanations in the README.

Also, I extracted the JSON handling to a separate file to avoid code duplication between result_merger.rb and last_run.rb.

Thanks for this neat Gem :).

[infertux]

Link to the Travis build: http://travis-ci.org/#!/infertux/simplecov/builds/1927262

[colszowka]

Looks cool, thanks!

I'll give it a more thorough overview in the next days, but so from a brief glimpse it looks good. This deals with #11 and #96.

[colszowka]

Great stuff. I read through the code and you did well, special bonus points for nice cucumber features :)

That at_exit block in SimpleCov is getting a bit unwieldy right now, I think that will need some refactoring soon. I created a separate issue (#153) for that.

Two things:

1. Can you please add some kind of command line output (I guess preferably to STDERR) when one of the measurements fails and thus leads to non-zero exit status? In the current way it might not be obvious what the heck happend there :) Please also verify those in the cukes.
2. The naming of the config variables: For one I'd prefer long method names here, e.g. minimum_coverage. Also, I think the naming max_coverage_difference is a bit misleading. In your commit message you already wrote coverage_drop, and that is what it is, it's not a difference (otherwise it would have to also fail when the coverage raises too much ;) ). Not sure whether to call it maximum_coverage_drop or max_coverage_drop as on one hand the first one is a bit long, while the latter is inconsistent with what I suggested for the minimum_coverage. Go with whatever you think makes sense :)

Thanks for your effort!

[infertux]

That at_exit block in SimpleCov is getting a bit unwieldy right now, I think that will need some refactoring soon.

Yeah, I think this bit should be extracted to some separate file but I'll let you go with whatever makes more sense to you.

Can you please add some kind of command line output (I guess preferably to STDERR) when one of the measurements fails and thus leads to non-zero exit status? In the current way it might not be obvious what the heck happend there :) Please also verify those in the cukes.

Good call - also added different exit codes for each error.

The naming of the config variables: For one I'd prefer long method names here, e.g. minimum_coverage. Also, I think the naming max_coverage_difference is a bit misleading. In your commit message you already wrote coverage_drop, and that is what it is, it's not a difference (otherwise it would have to also fail when the coverage raises too much ;) ). Not sure whether to call it maximum_coverage_drop or max_coverage_drop as on one hand the first one is a bit long, while the latter is inconsistent with what I suggested for the minimum_coverage. Go with whatever you think makes sense :)

Agreed, maximum_coverage_drop is not that long after all.

Thanks for your effort!

No problem! :)

[colszowka]

Awesome!

[dkubb]

Is there any way to have it so fractional coverage thresholds can be specified, like 90.7?

I know there are problems comparing floats, but we could always coerce to a BigDecimal, possibly with rounding to to the 10th or 100th place, and compare those?

[colszowka]

Yeah, I saw in the original commits that it only works with fixnums and was thinking the same, but then I thought again: Who the hell would want to specify a non-int threshold? If there is a use case for that I guess we could make it do :)

[dkubb]

The way I usually use coverage testing is that when I come onto a project where I add coverage, my goal is to ensure that all the code I add never decreases coverage. Ideally it's increasing, but at the very least I don't want it to drop at all.

So what I do is the first time I run the coverage I take the coverage result that is returned and I set that as my threshold. I effectively draw a line in the sand and say "the coverage should never get any worse". Every time I increase the coverage I increase the threshold.

I actually do this with several different kinds of metrics. It's true sometimes there is a special case and I have to manually adjust them downwards, but when that happens it's not by accident. I think that's a key point, most drops in code quality happen when people aren't paying attention or when you get lazy. In my case I fail the build if anything drops below my thresholds and I have to make the call on whether or not to fix it or adjust the thresholds.

[dkubb]

So I guess to make a long story short, it would be really nice to be able to make tiny incremental increases in the thresholds over time, rather than having to make whole number jumps in the thresholds.

[DavidAllison]

+1 For @dkubb's use case :)

[infertux]

FIrst off all, I didn't think about this use case when I initially used Fixnum but turns out fractional thresholds can be useful indeed.

However, using BigDecimal here wouldn't be a bit overkill? I just pushed a new branch that allows to specify floats as thresholds (coercing to Float): https://github.com/infertux/simplecov/tree/fractional_thresholds - any chance you could give it a try?

Also, you may be interested by #11 (comment).

@colszowka: Is there a new release including the new features planned in the near future?

[dkubb]

@infertux I tested that branch and saw the following error:

Coverage (91.60%) is below the expected minimum coverage (91.60%)

I'm guessing that what is happening is that when the coverage is calculated it's using fractional numbers, like 91.600001 or anything above 91.6. I'm guessing that the coverage and the threshold should be rounded down and then compared. For my purposes anything rounded to the nearest 1/10th is probably good enough.

[infertux]

Yes, the actual comparison is correct but the output is misleading because %.2f rounds numbers:

>> '%.2f' % 91.596
=> "91.60"

So numbers are now rounded down to 2 digits then compared to avoid confusion. 1/100th should be accurate enough for all use cases and is consistent with the HTML report which displays 2 digits as well.

Thanks for the report! :)