Open Stack

Morgan,


Good catch. This can be easily fixed if we add special tag in commit
message: e.g. #no-coverage-check


Best regards,
Boris Pavlovic

On Sun, Apr 19, 2015 at 9:33 AM, Morgan Fainberg <morgan.fainberg at gmail.com>
wrote:

> This is an interesting idea, but just a note on implementation:
>
> It is absolutely possible to reduce the % of coverage without losing (or
> even gaining) coverage of the code base. This can occur if deprecated code
> is removed and no new unit tests are added. Overall % of code covered by
> tests can decline since covered code has been removed with its unit tests,
> and non-covered code remains the same. In reality, coverage has not changed
> (or has improved). It is simply a limitation in going purely by % of code
> covered.
>
> I suggest that this move towards checking for classes/methods and make
> sure that coverage for those do not change between the two runs. If a given
> method or class has 100% coverage prior to a patch set, and in the new
> revision, it drops to less than 100% it should at least flag that there was
> a change in coverage. If a class, function, or method is removed it won't
> be in the new coverage report, and should not impact the test.
>
> --Morgan
>
> Sent via mobile
>
> On Apr 18, 2015, at 18:30, Boris Pavlovic <boris at pavlovic.me> wrote:
>
> 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
>
>
> Best regards,
> Boris Pavlovic
>
> __________________________________________________________________________
> 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/20150419/b89f2f50/attachment.html>