[openstack-dev] [all][code quality] Voting coverage job (-1 if coverage get worse after patch)

Boris Pavlovic boris at pavlovic.me
Mon Apr 20 20:10:27 UTC 2015


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.hayes at 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-request at 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-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150420/37665939/attachment.html>


More information about the OpenStack-dev mailing list