<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 18/07/14 00:44, Joe Gordon wrote:<br>
</div>
<blockquote
cite="mid:CAHXdxOepR-5BUvibuB_bPC_zwXzOrbB4Vv8a+Xj=fR19KeMJWg@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Wed, Jul 16, 2014 at 11:28 PM,
Steve Baker <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:sbaker@redhat.com" target="_blank">sbaker@redhat.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div class="">
<div>On 12/07/14 09:25, Joe Gordon wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Fri, Jul 11, 2014 at
4:42 AM, Jeremy Stanley <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:fungi@yuggoth.org"
target="_blank">fungi@yuggoth.org</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>On 2014-07-11 11:21:19 +0200 (+0200),
Matthias Runge wrote:<br>
> this broke horizon stable and master;
heat stable is affected as<br>
> well.<br>
</div>
[...]<br>
<br>
I guess this is a plea for applying
something like the oslotest<br>
framework to client libraries so they get
backward-compat jobs run<br>
against unit tests of all
dependant/consuming software... branchless<br>
tempest already alleviates some of this, but
not the case of changes<br>
in a library which will break
unit/functional tests of another<br>
project.<br>
</blockquote>
<div><br>
</div>
<div>We actually do have some tests for
backwards compatibility, and they all
passed. Presumably because both heat and
horizon have poor integration test.</div>
<div><br>
</div>
<div> We ran </div>
<div><br>
</div>
<div>
<ul
style="color:rgb(0,0,0);font-family:sans-serif">
<li style="list-style-type:none"><span
style="display:inline-block;min-width:20em;padding-bottom:2pt;width:auto!important"><a
moz-do-not-send="true"
href="http://logs.openstack.org/66/94166/3/check/check-tempest-dsvm-full-havana/8e09faa"
style="text-decoration:none;color:rgb(6,84,172)" target="_blank">check-tempest-dsvm-full-havana</a></span> <span><span
style="color:rgb(0,127,0)">SUCCESS</span> in
40m 47s (non-voting)</span></li>
<li style="list-style-type:none"><span
style="display:inline-block;min-width:20em;padding-bottom:2pt;width:auto!important"><a
moz-do-not-send="true"
href="http://logs.openstack.org/66/94166/3/check/check-tempest-dsvm-neutron-havana/b4ad019"
style="text-decoration:none;color:rgb(6,84,172)" target="_blank">check-tempest-dsvm-neutron-havana</a></span> <span><span
style="color:rgb(0,127,0)">SUCCESS</span> in
36m 17s (non-voting)</span></li>
<li style="list-style-type:none"><span
style="display:inline-block;min-width:20em;padding-bottom:2pt;width:auto!important"><a
moz-do-not-send="true"
href="http://logs.openstack.org/66/94166/3/check/check-tempest-dsvm-full-icehouse/c0c62e5"
style="text-decoration:none;color:rgb(6,84,172)" target="_blank">check-tempest-dsvm-full-icehouse</a></span> <span><span
style="color:rgb(0,127,0)">SUCCESS</span> in
53m 05s</span></li>
<li style="list-style-type:none"><span
style="display:inline-block;min-width:20em;padding-bottom:2pt;width:auto!important"><a
moz-do-not-send="true"
href="http://logs.openstack.org/66/94166/3/check/check-tempest-dsvm-neutron-icehouse/a54aedb"
style="color:rgb(6,84,172)"
target="_blank">check-tempest-dsvm-neutron-icehouse</a></span> <span><span
style="color:rgb(0,127,0)">SUCCESS</span> in
57m 28s</span></li>
</ul>
</div>
<div><br>
</div>
<div>on the offending patches (<a
moz-do-not-send="true"
href="https://review.openstack.org/#/c/94166/"
target="_blank">https://review.openstack.org/#/c/94166/</a>)</div>
<div> </div>
<div><br>
</div>
<div>Infra patch that added these tests: <a
moz-do-not-send="true"
href="https://review.openstack.org/#/c/80698/"
target="_blank">https://review.openstack.org/#/c/80698/</a><br>
</div>
<div><br>
</div>
<br>
</div>
</div>
</div>
</blockquote>
</div>
Heat-proper would have continued working fine with
novaclient 2.18.0. The regression was with raising
novaclient exceptions, which is only required in our
unit tests. I saw this break coming and switched to
raising via from_response <a moz-do-not-send="true"
href="https://review.openstack.org/#/c/97977/22/heat/tests/v1_1/fakes.py"
target="_blank">https://review.openstack.org/#/c/97977/22/heat/tests/v1_1/fakes.py</a><br>
<br>
Unit tests tend to deal with more internals of client
libraries just for mocking purposes, and there have been
multiple breaks in unit tests for heat and horizon when
client libraries make internal changes.<br>
<br>
This could be avoided if the client gate jobs run the
unit tests for the projects which consume them.</div>
</blockquote>
<div> </div>
<div>That may work but isn't this exactly what integration
testing is for? <br>
</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
If you mean tempest then no, this is different.<br>
<br>
Client projects have done a good job of keeping their public library
APIs stable. An exception type is public API, but the constructor
for raising that type arguably is more of a gray area since only the
client library should be raising its own exceptions.<br>
<br>
However heat and horizon unit tests need to raise client exceptions
to test their own error condition handling, so exception
constructors could be considered public API, but only for unit test
mocking in other projects.<br>
<br>
This problem couldn't have been caught in an integration test
because nothing outside the unit tests directly raises a client
exception.<br>
<br>
There have been other breakages where internal client library
changes have broken the mocking in our unit tests (I recall a
neutronclient internal refactor).<br>
<br>
In many cases the cause may be inappropriate mocking in the unit
tests, but that is cold comfort when the gates break when a client
library is released.<br>
<br>
Maybe we can just start with adding heat and horizon to the check
jobs of the clients they consume, but the following should also be
considered:<br>
grep "python-.*client" */requirements.txt<br>
<br>
This could give client libraries more confidence that internal
changes don't break anything, and allows them to fix mocking in
other projects before their changes land.<br>
<br>
</body>
</html>