Morgan, Thank you for your input. I improved coverage job in this patch: https://review.openstack.org/#/c/175557/1
Now: * It is based on missing lines and not coverage percentage. * It shows nice messages and coverage diffs: Allowed to introduce missing lines : 8 Missing lines in master : 649 Missing lines in proposed change : 669 Name Stmts Miss Branch BrMiss Cover @@ -43 +43 @@ -rally/benchmark/processing/utils 52 0 30 1 98.780% +rally/benchmark/processing/utils 52 20 30 15 57.317% @@ -190 +190 @@ -TOTAL 9400 649 2284 394 91.073% +TOTAL 9400 669 2284 408 90.782% Please write more unit tests, we should keep our test coverage :( Best regards, Boris Pavlovic On Mon, Apr 20, 2015 at 9:11 PM, Hayes, Graham <graham.ha...@hp.com> wrote: > On 20/04/15 18:01, Clint Byrum wrote: > > Excerpts from Boris Pavlovic's message of 2015-04-18 18:30:02 -0700: > >> Hi stackers, > >> > >> Code coverage is one of the very important metric of overall code > quality > >> especially in case of Python. It's quite important to ensure that code > is > >> covered fully with well written unit tests. > >> > >> One of the nice thing is coverage job. > >> > >> In Rally we are running it against every check which allows us to get > >> detailed information about > >> coverage before merging patch: > >> > http://logs.openstack.org/84/175084/1/check/rally-coverage/c61d5e1/cover/ > >> > >> This helped Rally core team to automate checking that new/changed code > is > >> covered by unit tests and we raised unit test coverage from ~78% to > almost > >> 91%. > >> > >> But it produces few issues: > >> 1) >9k nitpicking - core reviewers have to put -1 if something is not > >> covered by unit tests > >> 2) core team scaling issues - core team members spend a lot of time just > >> checking that whole code is covered by unit test and leaving messages > like > >> this should be covered by unit test > >> 3) not friendly community - it's not nice to get on your code -1 from > >> somebody that is asking just to write unit tests > >> 4) coverage regressions - sometimes we accidentally accept patches that > >> reduce coverage > >> > >> To resolve this issue I improved a bit coverage job in Rally project, > and > >> now it compares master vs master + patch coverage. If new coverage is > less > >> than master job is marked as -1. > >> > >> Here is the patch for job enhancement: > >> https://review.openstack.org/#/c/174645/ > >> > >> Here is coverage job in action: > >> patch https://review.openstack.org/#/c/174677/ > >> job message > >> > http://logs.openstack.org/77/174677/4/check/rally-coverage/ba49c90/console.html#_2015-04-17_15_57_17_695 > >> > > > > The link to the important line was key, because without it, just clicking > > through from the review was incomprehensible to me. Can I suggest some > > whitespace or bordering so we can see where the error is easily? > > > > Anyway, interesting thoughts from everyone. I have to agree with those > > that say this isn't reliable enough to make it vote. Non-voting would be > > interesting though, if it gave a clear score difference, and a diff of > > the two coverage reports. I think this is more useful as an automated > > pointer to how things probably should be, but sometimes it's entirely > > o-k to regress this number a few points. > > > > Also graphing this over time in a post-commit job seems like a > no-brainer. > > > Designate has started doing this - it is still a WIP as we continue > tweaking settings, but we have a dashboard here - > > http://sonar.designate-ci.com/ > > > > > > __________________________________________________________________________ > > OpenStack Development Mailing List (not for usage questions) > > Unsubscribe: > openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev