<div dir="ltr">Hi Timur,<div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Thanks.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 2, 2015 at 8:55 PM, Timur Nurlygayanov <span dir="ltr"><<a href="mailto:tnurlygayanov@mirantis.com" target="_blank">tnurlygayanov@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div>Hi all,<br><br></div>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).<br></div>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.<br><br></div>we can just copy this job from Rally and start to use it for Sahara:<br></div>Coverage control script: <a href="https://github.com/openstack/rally/blob/master/tests/ci/cover.sh" target="_blank">https://github.com/openstack/rally/blob/master/tests/ci/cover.sh</a><br></div>Configuration file for coverage plugin (to exclude code which shouldn't be affected): <a href="https://github.com/openstack/rally/blob/master/.coveragerc" target="_blank">https://github.com/openstack/rally/blob/master/.coveragerc</a><br></div>Example of job in infra repository: <a href="https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088" target="_blank">https://github.com/openstack-infra/project-config/blob/master/zuul/layout.yaml#L4088</a><br clear="all"><div><br></div>I expect that it will help to increase the tests coverage by unit tests.<br><br>Do we have any objections?<span class="HOEnZb"><font color="#888888"><br clear="all"><br>-- <br><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><font color="#888888"><font color="#888888"><br></font></font><div style="font-family:arial;font-size:small">Timur,</div><div style="font-family:arial;font-size:small">Senior QA Engineer</div><div style="font-family:arial;font-size:small">OpenStack Projects</div><div style="font-family:arial;font-size:small">Mirantis Inc</div></div></div></div></div></div></div>
</font></span></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Sincerely yours,<br>Sergey Lukjanov<br>Sahara Technical Lead<br>(OpenStack Data Processing)<br>Principal Software Engineer<br>Mirantis Inc.</div>
</div>