<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 7, 2016 at 8:57 AM, Knight, Clinton <span dir="ltr"><<a href="mailto:Clinton.Knight@netapp.com" target="_blank">Clinton.Knight@netapp.com</a>></span> wrote:<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"><div class=""><div class="h5"><br>
<br>
On 3/7/16, 10:45 AM, "Eric Harney" <<a href="mailto:eharney@redhat.com">eharney@redhat.com</a>> wrote:<br>
<br>
>On 03/06/2016 09:35 PM, John Griffith wrote:<br>
>> On Sat, Mar 5, 2016 at 4:27 PM, Jay S. Bryant<br>
>><<a href="mailto:jsbryant@electronicjungle.net">jsbryant@electronicjungle.net</a><br>
>>> wrote:<br>
>><br>
>>> Ivan,<br>
>>><br>
>>> I agree that our testing needs improvement. Thanks for starting this<br>
>>> thread.<br>
>>><br>
>>> With regards to adding a hacking check for tests that run too long ...<br>
>>>are<br>
>>> you thinking that we would have a timer that checks or long running<br>
>>>jobs or<br>
>>> something that checks for long sleeps in the testing code? Just<br>
>>>curious<br>
>>> your ideas for tackling that situation. Would be interested in helping<br>
>>> with that, perhaps.<br>
>>><br>
>>> Thanks!<br>
>>> Jay<br>
>>><br>
>>><br>
>>> On 03/02/2016 05:25 AM, Ivan Kolodyazhny wrote:<br>
>>><br>
>>> Hi Team,<br>
>>><br>
>>> Here are my thoughts and proposals how to make Cinder testing process<br>
>>> better. I won't cover "3rd party CI's" topic here. I will share my<br>
>>>opinion<br>
>>> about current and feature jobs.<br>
>>><br>
>>><br>
>>> Unit-tests<br>
>>><br>
>>> - Long-running tests. I hope, everybody will agree that unit-tests<br>
>>> must be quite simple and very fast. Unit tests which takes more<br>
>>>than 3-5<br>
>>> seconds should be refactored and/or moved to 'integration' tests.<br>
>>> Thanks to Tom Barron for several fixes like [1]. IMO, we it would be<br>
>>> good to have some hacking checks to prevent such issues in a future.<br>
>>><br>
>>> - Tests coverage. We don't check it in an automatic way on gates.<br>
>>> Usually, we require to add some unit-tests during code review<br>
>>>process. Why<br>
>>> can't we add coverage job to our CI and do not merge new patches,<br>
>>>with<br>
>>> will decrease tests coverage rate? Maybe, such job could be voting<br>
>>>in a<br>
>>> future to not ignore it. For now, there is not simple way to check<br>
>>>coverage<br>
>>> because 'tox -e cover' output is not useful [2].<br>
<br>
</div></div>The Manila project has a coverage job that may be of interest to Cinder.<br>
It’s not perfect, because sometimes the periodic loopingcall routines run<br>
during the test run and sometimes not, leading to false negatives. But<br>
most of the time it’s a handy confirmation that the unit test coverage<br>
didn’t decline due to a patch. Look at the manila-coverage job in this<br>
example: <a href="https://review.openstack.org/#/c/287575/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/287575/</a><br>
<div class=""><div class="h5"><br>
>>><br>
>>><br>
>>> Functional tests for Cinder<br>
>>><br>
>>> We introduced some functional tests last month [3]. Here is a patch to<br>
>>> infra to add new job [4]. Because these tests were moved from<br>
>>>unit-tests, I<br>
>>> think we're OK to make this job voting. Such tests should not be a<br>
>>> replacement for Tempest. They even could tests Cinder with Fake Driver<br>
>>>to<br>
>>> make it faster and not related on storage backends issues.<br>
>>><br>
>>><br>
>>> Tempest in-tree tests<br>
>>><br>
>>> Sean started work on it [5] and I think it's a good idea to get them in<br>
>>> Cinder repo to run them on Tempest jobs and 3-rd party CIs against a<br>
>>>real<br>
>>> backend.<br>
>>><br>
>>><br>
>>> Functional tests for python-brick-cinderclient-ext<br>
>>><br>
>>> There are patches that introduces functional tests [6] and new job [7].<br>
>>><br>
>>><br>
>>> Functional tests for python-cinderclient<br>
>>><br>
>>> We've got a very limited set of such tests and non-voting job. IMO, we<br>
>>>can<br>
>>> run them even with Cinder Fake Driver to make them not depended on a<br>
>>> storage backend and make it faster. I believe, we can make this job<br>
>>>voting<br>
>>> soon. Also, we need more contributors to this kind of tests.<br>
>>><br>
>>><br>
>>> Integrated tests for python-cinderclient<br>
>>><br>
>>> We need such tests to make sure that we won't break Nova, Heat or other<br>
>>> python-cinderclient consumers with a next merged patch. There is a<br>
>>>thread<br>
>>> in openstack-dev ML about such tests [8] and proposal [9] to introduce<br>
>>>them<br>
>>> to python-cinderclient.<br>
>>><br>
>>><br>
>>> Rally tests<br>
>>><br>
>>> IMO, it would be good to have new Rally scenarios for every patches<br>
>>>like<br>
>>> 'improves performance', 'fixes concurrency issues', etc. Even if we as<br>
>>>a<br>
>>> Cinder community don't have enough time to implement them, we have to<br>
>>>ask<br>
>>> for them in reviews, openstack-dev ML, file Rally bugs and blueprints<br>
>>>if<br>
>>> needed.<br>
>>><br>
>>><br>
>>> [1] <a href="https://review.openstack.org/#/c/282861/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/282861/</a><br>
>>> [2] <a href="http://paste.openstack.org/show/488925/" rel="noreferrer" target="_blank">http://paste.openstack.org/show/488925/</a><br>
>>> [3] <a href="https://review.openstack.org/#/c/267801/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/267801/</a><br>
>>> [4] <a href="https://review.openstack.org/#/c/287115/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/287115/</a><br>
>>> [5] <a href="https://review.openstack.org/#/c/274471/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/274471/</a><br>
>>> [6] <a href="https://review.openstack.org/#/c/265811/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/265811/</a><br>
>>> [7] <a href="https://review.openstack.org/#/c/265925/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/265925/</a><br>
>>> [8]<br>
>>><br>
>>><a href="http://lists.openstack.org/pipermail/openstack-dev/2016-March/088027.htm" rel="noreferrer" target="_blank">http://lists.openstack.org/pipermail/openstack-dev/2016-March/088027.htm</a><br>
>>>l<br>
>>> [9] <a href="https://review.openstack.org/#/c/279432/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/279432/</a><br>
>>><br>
>>><br>
>>> Regards,<br>
>>> Ivan Kolodyazhny,<br>
>>> <a href="http://blog.e0ne.info/" rel="noreferrer" target="_blank">http://blog.e0ne.info/</a><br>
>>><br>
>>><br>
>>><br>
>>> We could just parse out the tox slowest tests output we already have.<br>
>>> Do<br>
>> something like pylint where we look at existing/current slowest test and<br>
>> balk if that's exceeded.<br>
>><br>
>> Thoughts?<br>
>><br>
>> John<br>
>><br>
><br>
>I'm not really sure that writing a "hacking" check for this is a<br>
>worthwhile investment. (It's not a hacking check really, but something<br>
>more like what you're describing, but that's beside the point.)<br>
><br>
>We should just be looking for large, complex unit tests in review, and<br>
>the ones that we already have should be moving towards the functional<br>
>test area anyway.<br>
><br>
>So what would the objective here be exactly?<br>
><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>
__________________________________________________________________________<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>
</div></div></blockquote></div><div class="gmail_default" style="font-family:monospace,monospace">Actually, mtreinish just pointed out we already have the check in place :)</div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace">We're just using 1 minute as our timeout, so that's kinda dumb.</div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace"><a href="https://github.com/openstack/cinder/blob/master/cinder/test.py#L158-L165">https://github.com/openstack/cinder/blob/master/cinder/test.py#L158-L165</a></div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace">and the setting of the var:</div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace"><a href="https://github.com/openstack/cinder/blob/master/.testr.conf#L4">https://github.com/openstack/cinder/blob/master/.testr.conf#L4</a></div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace">So just change that. No?</div><br></div></div>