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

Boris Pavlovic boris at pavlovic.me
Mon Apr 20 17:18:09 UTC 2015


Clint,


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.


Diffs between reports is almost ready.


Best regards,
Boris Pavlovic


On Mon, Apr 20, 2015 at 7:59 PM, Clint Byrum <clint at fewbar.com> 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.
>
> __________________________________________________________________________
> 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/0249d7cd/attachment.html>


More information about the OpenStack-dev mailing list