Support for minimal coverage and maximal coverage drop #151

Merged
merged 1 commit into from Aug 2, 2012

Conversation

Projects
None yet
4 participants
Contributor

infertux commented Jul 22, 2012

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 :).

colszowka referenced this pull request Jul 31, 2012

Merged

Just some typos #152

Owner

colszowka commented Jul 31, 2012

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.

Owner

colszowka commented Jul 31, 2012

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!

Contributor

infertux commented Aug 2, 2012

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 merged commit 439b05b into colszowka:master Aug 2, 2012

Owner

colszowka commented Aug 2, 2012

Awesome!

dkubb commented Aug 30, 2012

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?

Owner

colszowka commented Aug 30, 2012

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 commented Aug 31, 2012

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 commented Aug 31, 2012

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.

+1 For @dkubb's use case :)

Contributor

infertux commented Sep 7, 2012

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 commented Sep 10, 2012

@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.

Contributor

infertux commented Sep 11, 2012

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! :)

@jperkin jperkin pushed a commit to joyent/pkgsrc that referenced this pull request Dec 9, 2013

taca Update ruby-simplecov.
Make this package to Ruby 1.9.3 only.

v0.7.1, 2012-10-12 ([changes](colszowka/simplecov@v0.7.0...v0.7.1))
-------------------

  * [BUGFIX] The gem packages of 0.7.0 (both simplecov and simplecov-html) pushed to Rubygems had some file
    permission issues, leading to problems when installing SimpleCov in a root/system Rubygems install and then
    trying to use it as a normal user (see colszowka/simplecov#171, thanks @envygeeks
    for bringing it up). The gem build process has been changed to always enforce proper permissions before packaging
    to avoid this issue in the future.


v0.7.0, 2012-10-10 ([changes](colszowka/simplecov@v0.6.4...v0.7.0))
-------------------

  * [FEATURE] The new `maximum_coverage_drop` and `minimum_coverage` now allow you to fail your build when the
    coverage dropped by more than what you allowed or is below a minimum value required. Also, `refuse_coverage_drop` disallows
    any coverage drops between test runs.
    See colszowka/simplecov#151, colszowka/simplecov#11,
    colszowka/simplecov#90, and colszowka/simplecov#96 (thanks to @infertux)
  * [FEATURE] SimpleCov now ships with a built-in MultiFormatter which allows the easy usage of multiple result formatters at
    the same time without the need to write custom wrapper code.
    See colszowka/simplecov#158 (thanks to @nikitug)
  * [BUGFIX] The usage of digits, hyphens and underscores in group names could lead to broken tab navigation
    in the default simplecov-html reports. See colszowka/simplecov-html#14 (thanks to @ebelgarts)
  * [REFACTORING] A few more ruby warnings removed. See colszowka/simplecov#106 and
    colszowka/simplecov#139. (thanks to @lukejahnke)
  * A [Pledgie button](colszowka/simplecov@63cfa99) for those that
    feel generous :)
  * The usual bunch of README fixes and documentation tweaks. Thanks to everyone who contributed those!
9027cf8

@jsonn jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Mar 12, 2014

taca Update ruby-simplecov.
Make this package to Ruby 1.9.3 only.

v0.7.1, 2012-10-12 ([changes](colszowka/simplecov@v0.7.0...v0.7.1))
-------------------

  * [BUGFIX] The gem packages of 0.7.0 (both simplecov and simplecov-html) pushed to Rubygems had some file
    permission issues, leading to problems when installing SimpleCov in a root/system Rubygems install and then
    trying to use it as a normal user (see colszowka/simplecov#171, thanks @envygeeks
    for bringing it up). The gem build process has been changed to always enforce proper permissions before packaging
    to avoid this issue in the future.


v0.7.0, 2012-10-10 ([changes](colszowka/simplecov@v0.6.4...v0.7.0))
-------------------

  * [FEATURE] The new `maximum_coverage_drop` and `minimum_coverage` now allow you to fail your build when the
    coverage dropped by more than what you allowed or is below a minimum value required. Also, `refuse_coverage_drop` disallows
    any coverage drops between test runs.
    See colszowka/simplecov#151, colszowka/simplecov#11,
    colszowka/simplecov#90, and colszowka/simplecov#96 (thanks to @infertux)
  * [FEATURE] SimpleCov now ships with a built-in MultiFormatter which allows the easy usage of multiple result formatters at
    the same time without the need to write custom wrapper code.
    See colszowka/simplecov#158 (thanks to @nikitug)
  * [BUGFIX] The usage of digits, hyphens and underscores in group names could lead to broken tab navigation
    in the default simplecov-html reports. See colszowka/simplecov-html#14 (thanks to @ebelgarts)
  * [REFACTORING] A few more ruby warnings removed. See colszowka/simplecov#106 and
    colszowka/simplecov#139. (thanks to @lukejahnke)
  * A [Pledgie button](colszowka/simplecov@63cfa99) for those that
    feel generous :)
  * The usual bunch of README fixes and documentation tweaks. Thanks to everyone who contributed those!
4ceab0f

@jsonn jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Oct 11, 2014

taca Update ruby-simplecov.
Make this package to Ruby 1.9.3 only.

v0.7.1, 2012-10-12 ([changes](colszowka/simplecov@v0.7.0...v0.7.1))
-------------------

  * [BUGFIX] The gem packages of 0.7.0 (both simplecov and simplecov-html) pushed to Rubygems had some file
    permission issues, leading to problems when installing SimpleCov in a root/system Rubygems install and then
    trying to use it as a normal user (see colszowka/simplecov#171, thanks @envygeeks
    for bringing it up). The gem build process has been changed to always enforce proper permissions before packaging
    to avoid this issue in the future.


v0.7.0, 2012-10-10 ([changes](colszowka/simplecov@v0.6.4...v0.7.0))
-------------------

  * [FEATURE] The new `maximum_coverage_drop` and `minimum_coverage` now allow you to fail your build when the
    coverage dropped by more than what you allowed or is below a minimum value required. Also, `refuse_coverage_drop` disallows
    any coverage drops between test runs.
    See colszowka/simplecov#151, colszowka/simplecov#11,
    colszowka/simplecov#90, and colszowka/simplecov#96 (thanks to @infertux)
  * [FEATURE] SimpleCov now ships with a built-in MultiFormatter which allows the easy usage of multiple result formatters at
    the same time without the need to write custom wrapper code.
    See colszowka/simplecov#158 (thanks to @nikitug)
  * [BUGFIX] The usage of digits, hyphens and underscores in group names could lead to broken tab navigation
    in the default simplecov-html reports. See colszowka/simplecov-html#14 (thanks to @ebelgarts)
  * [REFACTORING] A few more ruby warnings removed. See colszowka/simplecov#106 and
    colszowka/simplecov#139. (thanks to @lukejahnke)
  * A [Pledgie button](colszowka/simplecov@63cfa99) for those that
    feel generous :)
  * The usual bunch of README fixes and documentation tweaks. Thanks to everyone who contributed those!
8758c51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment