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.fainb...@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 <bo...@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-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
