<div dir="ltr">Clint,<div><br></div><div><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:12.8000001907349px">Anyway, interesting thoughts from everyone. I have to agree with those<br></span><span style="font-size:12.8000001907349px">that say this isn't reliable enough to make it vote. Non-voting would be<br></span><span style="font-size:12.8000001907349px">interesting though, if it gave a clear score difference, and a diff of<br></span><span style="font-size:12.8000001907349px">the two coverage reports. I think this is more useful as an automated<br></span><span style="font-size:12.8000001907349px">pointer to how things probably should be, but sometimes it's entirely<br></span><span style="font-size:12.8000001907349px">o-k to regress this number a few points.</span></blockquote><div><div><br></div><div>Diffs between reports is almost ready. </div></div></div><div><br></div><div><br></div><div>Best regards,</div><div>Boris Pavlovic </div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 20, 2015 at 7:59 PM, Clint Byrum <span dir="ltr"><<a href="mailto:clint@fewbar.com" target="_blank">clint@fewbar.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Excerpts from Boris Pavlovic's message of 2015-04-18 18:30:02 -0700:<br>
<div><div class="h5">> Hi stackers,<br>
><br>
> Code coverage is one of the very important metric of overall code quality<br>
> especially in case of Python. It's quite important to ensure that code is<br>
> covered fully with well written unit tests.<br>
><br>
> One of the nice thing is coverage job.<br>
><br>
> In Rally we are running it against every check which allows us to get<br>
> detailed information about<br>
> coverage before merging patch:<br>
> <a href="http://logs.openstack.org/84/175084/1/check/rally-coverage/c61d5e1/cover/" target="_blank">http://logs.openstack.org/84/175084/1/check/rally-coverage/c61d5e1/cover/</a><br>
><br>
> This helped Rally core team to automate checking that new/changed code is<br>
> covered by unit tests and we raised unit test coverage from ~78% to almost<br>
> 91%.<br>
><br>
> But it produces few issues:<br>
> 1) >9k nitpicking - core reviewers have to put -1 if something is not<br>
> covered by unit tests<br>
> 2) core team scaling issues - core team members spend a lot of time just<br>
> checking that whole code is covered by unit test and leaving messages like<br>
> this should be covered by unit test<br>
> 3) not friendly community - it's not nice to get on your code -1 from<br>
> somebody that is asking just to write unit tests<br>
> 4) coverage regressions - sometimes we accidentally accept patches that<br>
> reduce coverage<br>
><br>
> To resolve this issue I improved a bit coverage job in Rally project, and<br>
> now it compares master vs master + patch coverage. If new coverage is less<br>
> than master job is marked as -1.<br>
><br>
> Here is the patch for job enhancement:<br>
> <a href="https://review.openstack.org/#/c/174645/" target="_blank">https://review.openstack.org/#/c/174645/</a><br>
><br>
> Here is coverage job in action:<br>
> patch <a href="https://review.openstack.org/#/c/174677/" target="_blank">https://review.openstack.org/#/c/174677/</a><br>
> job message<br>
> <a href="http://logs.openstack.org/77/174677/4/check/rally-coverage/ba49c90/console.html#_2015-04-17_15_57_17_695" target="_blank">http://logs.openstack.org/77/174677/4/check/rally-coverage/ba49c90/console.html#_2015-04-17_15_57_17_695</a><br>
><br>
<br>
</div></div>The link to the important line was key, because without it, just clicking<br>
through from the review was incomprehensible to me. Can I suggest some<br>
whitespace or bordering so we can see where the error is easily?<br>
<br>
Anyway, interesting thoughts from everyone. I have to agree with those<br>
that say this isn't reliable enough to make it vote. Non-voting would be<br>
interesting though, if it gave a clear score difference, and a diff of<br>
the two coverage reports. I think this is more useful as an automated<br>
pointer to how things probably should be, but sometimes it's entirely<br>
o-k to regress this number a few points.<br>
<br>
Also graphing this over time in a post-commit job seems like a no-brainer.<br>
<div class="HOEnZb"><div class="h5"><br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>