Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Hi, here is the commit on review: https://review.openstack.org/#/c/200433 We already added non-voting job in infra repository, need to merge this script. On Fri, Jul 3, 2015 at 1:29 PM, Anastasia Kuznetsova < akuznets...@mirantis.com> wrote: > Boris, > > thanks for an explanation! I will take a closer look at the cover.sh > script. > > On Fri, Jul 3, 2015 at 12:07 AM, Boris Pavlovic > wrote: > >> Anastasia, >> >> because new patch may not be just a new code, committer may delete >>> something or fix typos in docsting, etc. >> >> >> This job compares amount of non covered lines (before and after patch). >> If you just remove code there will be less lines that should be covered >> so amount of non covered lines will be less or the same (if everything was >> covered before) >> >> Fixing typos in docstrings won't introduce new lines. >> >> Btw job allows you to introduce N (few) new lines that are not covered >> by unit tests that are uncovered in some cases. >> >> >> Best regards, >> Boris Pavlovic >> >> On Thu, Jul 2, 2015 at 10:46 AM, Anastasia Kuznetsova < >> akuznets...@mirantis.com> wrote: >> >>> Hi Timur, >>> >>> Generally I think that it is a good idea to have a gate that will check >>> whether new code is covered by unit tests or not. But I am not sure that >>> this gate should be voting (if I understand you correct), >>> because new patch may not be just a new code, committer may delete >>> something or fix typos in docsting, etc. >>> >>> On Thu, Jul 2, 2015 at 8:15 PM, Timur Nurlygayanov < >>> tnurlygaya...@mirantis.com> wrote: >>> Hi all, I suggest to add CI job which will check the unit tests coverage for Sahara repository and will set -1 for commits with new code and without unit tests (if we have some degradation of tests coverage). This job successfully works for Rally project and it helps to organize the right code development process when developers write new unit tests for new functionality. we can just copy this job from Rally and start to use it for Sahara: Coverage control script: https://github.com/openstack/rally/blob/master/tests/ci/cover.sh Configuration file for coverage plugin (to exclude code which shouldn't be affected): https://github.com/openstack/rally/blob/master/.coveragerc Example of job in infra repository: https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 I expect that it will help to increase the tests coverage by unit tests. Do we have any objections? -- Timur, Senior QA Engineer OpenStack Projects Mirantis Inc __ 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 >>> >>> >>> -- >>> Best regards, >>> Anastasia Kuznetsova >>> >>> >>> __ >>> 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 >> >> > > > -- > Best regards, > Anastasia Kuznetsova > > __ > 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 > > -- Timur, Senior QA Engineer OpenStack Projects Mirantis Inc __ 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
Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Boris, thanks for an explanation! I will take a closer look at the cover.sh script. On Fri, Jul 3, 2015 at 12:07 AM, Boris Pavlovic wrote: > Anastasia, > > because new patch may not be just a new code, committer may delete >> something or fix typos in docsting, etc. > > > This job compares amount of non covered lines (before and after patch). > If you just remove code there will be less lines that should be covered so > amount of non covered lines will be less or the same (if everything was > covered before) > > Fixing typos in docstrings won't introduce new lines. > > Btw job allows you to introduce N (few) new lines that are not covered by > unit tests that are uncovered in some cases. > > > Best regards, > Boris Pavlovic > > On Thu, Jul 2, 2015 at 10:46 AM, Anastasia Kuznetsova < > akuznets...@mirantis.com> wrote: > >> Hi Timur, >> >> Generally I think that it is a good idea to have a gate that will check >> whether new code is covered by unit tests or not. But I am not sure that >> this gate should be voting (if I understand you correct), >> because new patch may not be just a new code, committer may delete >> something or fix typos in docsting, etc. >> >> On Thu, Jul 2, 2015 at 8:15 PM, Timur Nurlygayanov < >> tnurlygaya...@mirantis.com> wrote: >> >>> Hi all, >>> >>> I suggest to add CI job which will check the unit tests coverage for >>> Sahara repository and will set -1 for commits with new code and without >>> unit tests (if we have some degradation of tests coverage). >>> This job successfully works for Rally project and it helps to organize >>> the right code development process when developers write new unit tests for >>> new functionality. >>> >>> we can just copy this job from Rally and start to use it for Sahara: >>> Coverage control script: >>> https://github.com/openstack/rally/blob/master/tests/ci/cover.sh >>> Configuration file for coverage plugin (to exclude code which shouldn't >>> be affected): https://github.com/openstack/rally/blob/master/.coveragerc >>> Example of job in infra repository: >>> https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 >>> >>> I expect that it will help to increase the tests coverage by unit tests. >>> >>> Do we have any objections? >>> >>> -- >>> >>> Timur, >>> Senior QA Engineer >>> OpenStack Projects >>> Mirantis Inc >>> >>> >>> __ >>> 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 >>> >>> >> >> >> -- >> Best regards, >> Anastasia Kuznetsova >> >> __ >> 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 > > -- Best regards, Anastasia Kuznetsova __ 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
Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Hi Sergey, Looks like I suggested job which we already have :) so, I think non-voiting coverage job is ok for Sahara project and we can just continue to use it. Thank you! On Thu, Jul 2, 2015 at 11:06 PM, Sergey Lukjanov wrote: > Hi Timur, > > I absolutely disagree with this approach. IMO such checks just helps to > organize the unhealthy dev process when contributors will write tests only > to pass this check. We have a non-voting coverage job to track the unit > tests coverage in addition to the code review itself. It's the > responsibility of the core team to ask for additional unit tests if they > are missed. For Sahara project specifically there are tons of places where > unit tests will be just mocks testing and such tests are mostly useless. > For the places not covered by unit tests we have tons of integration tests. > Currently, we trying to force contributors to cover the new code with unit > tests if it's really applicable. > > Let my add some numbers. We have ~ 55% total code coverage by unit tests > and ~70% by integration tests (significantly shifted in terms of code > blocks coverage comparing to the unit tests). If we'll ignore the plugins > dir that contains mostly deployment code fully tested by integration tests > we'll have ~70% unit tests coverage. > > Thanks. > > On Thu, Jul 2, 2015 at 8:55 PM, Timur Nurlygayanov < > tnurlygaya...@mirantis.com> wrote: > >> Hi all, >> >> I suggest to add CI job which will check the unit tests coverage for >> Sahara repository and will set -1 for commits with new code and without >> unit tests (if we have some degradation of tests coverage). >> This job successfully works for Rally project and it helps to organize >> the right code development process when developers write new unit tests for >> new functionality. >> >> we can just copy this job from Rally and start to use it for Sahara: >> Coverage control script: >> https://github.com/openstack/rally/blob/master/tests/ci/cover.sh >> Configuration file for coverage plugin (to exclude code which shouldn't >> be affected): https://github.com/openstack/rally/blob/master/.coveragerc >> Example of job in infra repository: >> https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 >> >> I expect that it will help to increase the tests coverage by unit tests. >> >> Do we have any objections? >> >> -- >> >> Timur, >> Senior QA Engineer >> OpenStack Projects >> Mirantis Inc >> >> __ >> 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 >> >> > > > -- > Sincerely yours, > Sergey Lukjanov > Sahara Technical Lead > (OpenStack Data Processing) > Principal Software Engineer > Mirantis Inc. > > __ > 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 > > -- Timur, Senior QA Engineer OpenStack Projects Mirantis Inc __ 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
Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Anastasia, because new patch may not be just a new code, committer may delete > something or fix typos in docsting, etc. This job compares amount of non covered lines (before and after patch). If you just remove code there will be less lines that should be covered so amount of non covered lines will be less or the same (if everything was covered before) Fixing typos in docstrings won't introduce new lines. Btw job allows you to introduce N (few) new lines that are not covered by unit tests that are uncovered in some cases. Best regards, Boris Pavlovic On Thu, Jul 2, 2015 at 10:46 AM, Anastasia Kuznetsova < akuznets...@mirantis.com> wrote: > Hi Timur, > > Generally I think that it is a good idea to have a gate that will check > whether new code is covered by unit tests or not. But I am not sure that > this gate should be voting (if I understand you correct), > because new patch may not be just a new code, committer may delete > something or fix typos in docsting, etc. > > On Thu, Jul 2, 2015 at 8:15 PM, Timur Nurlygayanov < > tnurlygaya...@mirantis.com> wrote: > >> Hi all, >> >> I suggest to add CI job which will check the unit tests coverage for >> Sahara repository and will set -1 for commits with new code and without >> unit tests (if we have some degradation of tests coverage). >> This job successfully works for Rally project and it helps to organize >> the right code development process when developers write new unit tests for >> new functionality. >> >> we can just copy this job from Rally and start to use it for Sahara: >> Coverage control script: >> https://github.com/openstack/rally/blob/master/tests/ci/cover.sh >> Configuration file for coverage plugin (to exclude code which shouldn't >> be affected): https://github.com/openstack/rally/blob/master/.coveragerc >> Example of job in infra repository: >> https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 >> >> I expect that it will help to increase the tests coverage by unit tests. >> >> Do we have any objections? >> >> -- >> >> Timur, >> Senior QA Engineer >> OpenStack Projects >> Mirantis Inc >> >> __ >> 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 >> >> > > > -- > Best regards, > Anastasia Kuznetsova > > __ > 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
Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Hi Timur, I absolutely disagree with this approach. IMO such checks just helps to organize the unhealthy dev process when contributors will write tests only to pass this check. We have a non-voting coverage job to track the unit tests coverage in addition to the code review itself. It's the responsibility of the core team to ask for additional unit tests if they are missed. For Sahara project specifically there are tons of places where unit tests will be just mocks testing and such tests are mostly useless. For the places not covered by unit tests we have tons of integration tests. Currently, we trying to force contributors to cover the new code with unit tests if it's really applicable. Let my add some numbers. We have ~ 55% total code coverage by unit tests and ~70% by integration tests (significantly shifted in terms of code blocks coverage comparing to the unit tests). If we'll ignore the plugins dir that contains mostly deployment code fully tested by integration tests we'll have ~70% unit tests coverage. Thanks. On Thu, Jul 2, 2015 at 8:55 PM, Timur Nurlygayanov < tnurlygaya...@mirantis.com> wrote: > Hi all, > > I suggest to add CI job which will check the unit tests coverage for > Sahara repository and will set -1 for commits with new code and without > unit tests (if we have some degradation of tests coverage). > This job successfully works for Rally project and it helps to organize the > right code development process when developers write new unit tests for new > functionality. > > we can just copy this job from Rally and start to use it for Sahara: > Coverage control script: > https://github.com/openstack/rally/blob/master/tests/ci/cover.sh > Configuration file for coverage plugin (to exclude code which shouldn't be > affected): https://github.com/openstack/rally/blob/master/.coveragerc > Example of job in infra repository: > https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 > > I expect that it will help to increase the tests coverage by unit tests. > > Do we have any objections? > > -- > > Timur, > Senior QA Engineer > OpenStack Projects > Mirantis Inc > > __ > 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 > > -- Sincerely yours, Sergey Lukjanov Sahara Technical Lead (OpenStack Data Processing) Principal Software Engineer Mirantis Inc. __ 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
Re: [openstack-dev] [Sahara] [QA] [tests coverage] Can we add CI job to control the unit tests coverage?
Hi Timur, Generally I think that it is a good idea to have a gate that will check whether new code is covered by unit tests or not. But I am not sure that this gate should be voting (if I understand you correct), because new patch may not be just a new code, committer may delete something or fix typos in docsting, etc. On Thu, Jul 2, 2015 at 8:15 PM, Timur Nurlygayanov < tnurlygaya...@mirantis.com> wrote: > Hi all, > > I suggest to add CI job which will check the unit tests coverage for > Sahara repository and will set -1 for commits with new code and without > unit tests (if we have some degradation of tests coverage). > This job successfully works for Rally project and it helps to organize the > right code development process when developers write new unit tests for new > functionality. > > we can just copy this job from Rally and start to use it for Sahara: > Coverage control script: > https://github.com/openstack/rally/blob/master/tests/ci/cover.sh > Configuration file for coverage plugin (to exclude code which shouldn't be > affected): https://github.com/openstack/rally/blob/master/.coveragerc > Example of job in infra repository: > https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088 > > I expect that it will help to increase the tests coverage by unit tests. > > Do we have any objections? > > -- > > Timur, > Senior QA Engineer > OpenStack Projects > Mirantis Inc > > __ > 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 > > -- Best regards, Anastasia Kuznetsova __ 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
