Reduce coveralls flood?

[jbarnoud]

Currently, coveralls tells us about coverage change in PR using comments. This generates a lot of unhelpful comments.

It is now possible to configure coveralls to use the status API in the same fashion as travis or quantify code. Because we do not want a big red cross attached to a PR because of a minor drop of coverage, it is also possible to set a threshold of coverage decrease before failure.

Should we setup coveralls to use the status API rather than comments? What threshold should we use?

[richardjgowers]

It's kind of annoying yes. I think one of the reasons why it fluctuates is because we're using random numbers in the transformations tests, which end up going down slightly different code paths. If we fixed this then the comments would be a lot more useful.

[mnmelo]

Still, I vote for the conciseness of having it in the status window. As for a threshold, 0.5% seems to me to be a good marker. This might need to be lowered as the size of the code base increases.

[orbeckst]

I'd also prefer it in the status window instead of a comment. I don't really see any downsides.

If –0.5% drop is what we see due to random numbers then use it. But 0.5% is already a pretty big deal (I was very, very happy when I got coverage up by 1%...). I would err on the conservative/restrictive side for the tolerance.

In any case, can someone go ahead and make the changes. Start with 0.5% tolerance if we don't have any better numbers and we can always adjust if necessary.

[mnmelo]

Hmm, I guess you're right, @orbeckst, that 0.5% might be too much. Now that you bring it up I think I'd rather have the opposite: to start with a tight threshold (which gives us more failures but reminds us to adjust it), rather than the opposite, which is to have a too loose threshold that we'll forget to tighten because tests are always green.

In addition coveralls also has a total-coverage threshold, below which we always fail. I'd say something round like 80% sounds good right now. We can then step it up by 5% or so. It is also a nice way to set coverage goals for ourselves (@orbeckst bumps the threshold to 85% and opens an issue "85% coverage marathon!").

[orbeckst]

80% as the lower boundary sounds good. I can tell you that getting the green marker back for 90% will feel VERY satisfying.

As an aside: when I talked to Enthought folks at Scipy over testing and mentioned half-apologetically that we are climbing back towards 90% they were impressed. Apparently, for contract work a coverage of 70% or so is often deemed acceptable. So we are doing not too badly but of course we know that we need high coverage to sustain our liberal open source development model while also ensuring that we can trust the code for our own work.

Oliver Beckstein
email: orbeckst@gmail.com

Am Aug 26, 2016 um 4:43 schrieb mnmelo notifications@github.com:
In addition coveralls also has a total-coverage threshold, below which we always fail. I'd say something round like 80% sounds good right now. We can then step it up by 5% or so. It is also a nice way to set coverage goals for ourselves (@orbeckst bumps the threshold to 85% and opens an issue "85% coverage marathon!").

[mnmelo]

Ok, with #958 out of the way (via @richardjgowers' #961) I think we can be more stringent with the failure threshold. More precisely, we should aim for a loss of coverage no greater than 0.0217%. Details below.

On the less technical/more pragmatical side I'm not sure if this value is suitable at all. Anyway, as discussed above, we can always start strict and relax as we go.

The stats

From coveralls.io: as of the last build we have 13722 relevant lines in our code base (of which 11573 are covered). If we compare to when we started tracking the analysis module, we're up 999 relevant lines, from 12723. There were roughly 47 PR merges since ("roughly" because I can't easily identify the squashed merges as such from the log...). This gives an average 21.2 extra relevant lines per merge.

Now, if we want to enforce contributions of at-least-commercial-grade coverage (70%, as per @orbeckst's findings), of those 21.2 lines 14.9 must be covered. With the current code base size, having that minimal coverage would cause a drop in global coverage of... 0.0217%

[orbeckst]

I love @mnmelo's scientific approach. Therefore, 0.0217% looks like a great number to use.

(I also love the fact that here "research-grade" is defined as "better than commercial-grade" ;-) ).

[mnmelo]

Ok, all that's needed now is for whoever added MDAnalysis to coveralls to go there and log out / log in. This is needed so that coveralls prompts for the extra Github permission to post to commit status boxes. Was that you, @orbeckst?

[orbeckst]

I just logged out, logged in, logged in. Not sure if anything happened.

Is there a way to add someone else to a coveralls.io account?

On 30 Aug, 2016, at 17:53, mnmelo notifications@github.com wrote:

Ok, all that's needed now is for whoever added MDAnalysis to coveralls to go there and log out / log in. This is needed so that coveralls prompts for the extra Github permission to post to commit status boxes. Was that you, @orbeckst?

Oliver Beckstein * orbeckst@gmx.net
skype: orbeckst * orbeckst@gmail.com

[mnmelo]

Not sure. I guess not, from the looks of this issue where they had to centrally reassign the owner.