Issue Details (XML | Word | Printable)

Key: CLOV-39
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Geoff Crain [Atlassian]
Reporter: Andy Armstrong
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Clover

Improving handling of new classes in the 'movers' report

Created: 10/Oct/07 11:12 AM   Updated: 06/May/08 12:54 AM
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3

Time Tracking:
Not Specified

Issue Links:
Reference
 

Participants: Andy Armstrong, Geoff Crain [Atlassian], Michael Studman [Atlassian], Nick Pellow [Atlassian] and Stefan Hansel
Since last comment: 26 weeks, 6 days ago
Resolution Date: 06/May/08 12:54 AM
Labels:


 Description  « Hide
It is possible that this has been fixed with Clover 2, in which case feel free to close this.

We are making a big push internally to track our Clover coverage, and we are finding the 'movers' report to be really helpful in that regard. We are sending out a nightly email showing the movers so that we can immediately see if developers are checking in code that is causing reductions in code coverage.

However, there is one oddity that is causing us trouble. When a new class is added to the code base, it appears that the report treats its 'old coverage' as being 0%. This means that a new class can be added without any tests at all, but it won't show up in the movers list. This is really bad for us, because we don't want to allow any code to be below our coverage threshold (we're aiming for 80% at the moment). Similarly, if a new class comes in with coverage at 50%, it is shown as an increase of 50%, even though this is still well below our current threshold. We really want classes coming in with no coverage to be considered a very bad thing in the movers report.

One simple option would be to assume that new classes come in with 100% coverage, so that if the coverage is not complete it will show up in the list of downward movers. A compromise would instead to take the previous average coverage (say 80%), and to require that the new class at least meets that level. So coming in at 100% would be seen as +20%, while coming in at 0% would show up as -80%.

Does this make sense? Has this already been implemented for Clover 2 reports?



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Nick Pellow [Atlassian] added a comment - 10/Oct/07 07:48 PM
Hi Andy,

Thank you for making this suggestion. Currently, the movers report will only display classes which are included in at least two of the previous history points.
The idea of making an introduced class having coverage of 100% and working from there is a good one.
ie. if a class is introduced with 0% coverage, it will appear as a downward mover by -100%.

The second suggestion you have is similar to something we have considered - ratchetting up of test coverage. This would be at the project level, and would potentially cause a build to fail (similar to the clover-check task) if the latest coverage is less than the previous coverage.

I'll see if I can plan at least the first of these suggestions to be implemented soon, as I think it would be really handy too.

Cheers,
Nick


Stefan Hansel added a comment - 11/Oct/07 01:20 AM
This remembers me of some old post concerning the same topic:

http://www.cenqua.com/forums/thread.jspa?messageID=12646&#12646

I wouldn't like it, if the base for new files was 100%, because in some of our projects we are far away from 80% or whatever and that would be very disappointing for developers to see -40% (where I'm quite happy when they reached +60% in legacy projects with a hell of smelling code where the average is 20% ).

If there'd be just an option to specify the treshould that'd be great. So then the movers are really helpfull, because they motivate to be above the minimum treshold (because your new class is listed in the movers with +x%) and keep people from falling below the treshold (because there are listed with negative amount in the movers).

Note that this threshould should be different from the miminum coverage that can make a build fail.
I'd be nice to specify a 'minimum coverage' and 'target coverage' seperately.

I could live as well with an option to automatically take the previous average coverage as 'default target coverage' for new classes.
This would make it easier to administer different projects because I didn't have to make different configurations for each team.


Michael Studman [Atlassian] added a comment - 31/Jan/08 08:43 AM
We've had to slip this to post-2.1. The work required to make this happen is now largely done but needs some polish so will likely be in 2.1.1 release.

Nick Pellow [Atlassian] added a comment - 07/Apr/08 02:30 AM
This is still on our priorities for things to improve in Clover Historical reporting, however we have unfortunately run out of time in the 2.2 release.
Since it will take quite some thinking to get right, we'd prefer to defer for now.
Sorry for any inconvenience caused.

Stefan Hansel added a comment - 07/Apr/08 03:02 AM
It's a pity ...

Recently I came up with a simpler solution - instead of thinking how to integrate it in the movers report (and what 'old' coverage should be considered if there are new classes) - why not just have a second report-part called:
'Top x New Files'. (Or 'Top x new Files with least coverage') and remove the new files from the movers completely.

I think the use-cases we think of (i.e. 'find bad new classes'' or 'honor good new classes') could be solved with such a simple chart as well.

Later on it could be thought again, what the best way was to integrate new files into the movers (which gets more complicated to do it 'right', I have to admit).
Anyway - since reports can be configured easily both simple - and later sophisticated - solutions don't come into each others way.


Nick Pellow [Atlassian] added a comment - 07/Apr/08 09:53 PM
Hi Stefan,

Excellent idea! I wish you'd mentioned it earlier
I am going to schedule this for a minor point release before we release Clover 2.3 as it is very doable and will indeed be very useful.

Cheers,
Nick