<div dir="ltr">Dan, <div><br></div><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">IMHO, most of the test coverage we have for nova's neutronapi is more<br></span><span style="font-size:12.8000001907349px">than useless. It's so synthetic that it provides no regression<br></span><span style="font-size:12.8000001907349px">protection, and often requires significantly more work than the change<br></span><span style="font-size:12.8000001907349px">that is actually being added. It's a huge maintenance burden with very<br></span><span style="font-size:12.8000001907349px">little value, IMHO. Good tests for that code would be very valuable of<br></span><span style="font-size:12.8000001907349px">course, but what is there now is not.</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">I think there are cases where going from 90 to 91% mean adding a ton of<br></span><span style="font-size:12.8000001907349px">extra spaghetti just to satisfy a bot, which actually adds nothing but<br></span><span style="font-size:12.8000001907349px">bloat to maintain.</span></blockquote><div><br></div><div>Let's not mix the bad unit tests in Nova with the fact that code should be fully covered by well written unit tests. </div><div>This big task can be split into 2 smaller tasks: <br></div><div>1) Bot that will check that we are covering new code by tests and don't introduce regressions </div><div>2) Core and other reviewers ensures that tests are well written. </div><div><br></div><div>P.S. Unit tests are the first criteria of code quality: if it is hard to cover code by unit tests, code is bad written</div><div>and should be refactored. </div><div><br></div><div>Best regards,</div><div>Boris Pavlovic </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 20, 2015 at 5:26 PM, Dan Smith <span dir="ltr"><<a href="mailto:dms@danplanet.com" target="_blank">dms@danplanet.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> Well, I think there are very few cases where *less* coverage is better.<br>
<br>
</span>IMHO, most of the test coverage we have for nova's neutronapi is more<br>
than useless. It's so synthetic that it provides no regression<br>
protection, and often requires significantly more work than the change<br>
that is actually being added. It's a huge maintenance burden with very<br>
little value, IMHO. Good tests for that code would be very valuable of<br>
course, but what is there now is not.<br>
<br>
I think there are cases where going from 90 to 91% mean adding a ton of<br>
extra spaghetti just to satisfy a bot, which actually adds nothing but<br>
bloat to maintain.<br>
<span class=""><br>
> This I completely agree with. Asking for unit tests is a common thing,<br>
> and if done early in the review process, is not a "non-friendly" thing.<br>
> It's just matter of fact. And if the comment is given with some example<br>
> unit test code -- or a pointer to a unit test that could be used as an<br>
> example -- even better. It grows the submitter.<br>
<br>
</span>+1<br>
<span class=""><br>
> I certainly am not opposed to introducing coverage regression jobs that<br>
> produce a history of coverage. I would not personally support them being<br>
> a blocking/gate job, though.<br>
<br>
</span>As Gordon said elsewhere in this thread, I'm not even sure I want to see<br>
it reporting as PASS/FAIL. It sounds like this would end up being like<br>
our pylint job, which was utterly confused by a lot of things and<br>
started to be something that wasn't even reliable enough to use as an<br>
advisory test.<br>
<br>
Recording the data for long-term analysis would be excellent though.<br>
It'd be nice to see that we increased coverage over a cycle.<br>
<span class="HOEnZb"><font color="#888888"><br>
--Dan<br>
</font></span><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>