<div dir="ltr">So I spent a bit more time, and managed to get all of the tests to pass again for me. I did fix the LXC tests that weren't isolated from the backing filesystem. (If you ran the LXC tests on BTRFS it would notice that it can '--snapshot' rather than use AUFS that the suite was testing.)<div>I also found a couple flaky tests that pass when I run them directly:</div><div><a href="https://bugs.launchpad.net/juju-core/+bug/1559400">https://bugs.launchpad.net/juju-core/+bug/1559400</a><br></div><div><a href="https://bugs.launchpad.net/juju-core/+bug/1559402">https://bugs.launchpad.net/juju-core/+bug/1559402</a><br></div><div><br></div><div>I also discovered another insidious failure:</div><div><br></div><div>func (s *MySuite) SetUpTest(c *gc.C) {</div><div> s.PatchValue()</div><div> s.Base.SetUpTest(c)<br>}</div><div><br></div><div>CleanupSuite.SetUpTest() initializes the list of cleanups to empty. Which means that any PatchValues called before SetUpTest would *never* get called. Which was probably the biggest thing leaking patched code.</div><div><br></div><div>But my branch is up and proposed:</div><div> <a href="http://reviews.vapour.ws/r/4240/diff/#">http://reviews.vapour.ws/r/4240/diff/#</a></div><div><br></div><div>The one really big and nice thing about this branch, is that it should mean you won't ever find that:</div><div> go test</div><div>passes but</div><div> go test -check.f OneTest</div><div>fails because of preconditions not being satisfied.</div><div><br></div><div>John</div><div>=:-></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 18, 2016 at 4:59 AM, Anastasia Macmood <span dir="ltr"><<a href="mailto:anastasia.macmood@canonical.com" target="_blank">anastasia.macmood@canonical.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">
John<br>
<br>
Thank you for discovering and working on this!<br>
<br>
This sounds like an awful can of worms and should be addressed in a
dedicated manner.<br>
<br>
I'll add it to the tech debt board and bug squad board with a
reference to your branch.<br>
<br>
I am not sure that anyone on core has a capacity to tackle it right
this second but I am hoping that the bug squad can take it head-on
soon \o/<br>
<br>
Sincerely Yours,<br>
<br>
Anastasia <br><div><div class="h5">
<br>
<div>On 17/03/16 19:43, John Meinel wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>tl; dr: I need some help fixing our tests that are
incorrectly not isolated from the real world.</div>
<div><br>
</div>
So...
<div><br>
</div>
<div>In investigating the bug with PatchValue and non pointer
receivers, we realized that PatchValue calls AddCleanup. Which
means that If you are doing:</div>
<div>func ... SetUpSuite() {</div>
<div> s.PatchValue(&foo, safeFoo)</div>
<div>}</div>
<div><br>
</div>
<div>That PatchValue gets cleaned up in the first
TearDownTest().Which means that the isolation you thought you
were adding to the *Suite* is actually isolated only to the
first test that gets run.</div>
<div><br>
</div>
<div>I have a branch of "juju/testing" that fixes it so
AddCleanup can be called at any time. If it is called before
SetUpTest(), then it adds a cleanup to the Suite stack,
otherwise it adds it to the current Test stack.</div>
<div><br>
</div>
<div>But that breaks about 100 tests that weren't as isolated as
they thought they were.</div>
<div><br>
</div>
<div>It also breaks a few tests that were Patching values before
calling IsolationSuite.SetUpTest/Suite(), eg:</div>
<div>SetUpTest() {<br>
PatchValue()</div>
<div> Base.SetUpTest()<br>
}</div>
<div><br>
</div>
<div>My branch that starts working on this is at:</div>
<div> <a href="mailto:git@github.com:jameinel/juju" target="_blank">git@github.com:jameinel/juju</a> unsafe-cleanups</div>
<div><br>
</div>
<div>One of the big examples of bad tests is "provider/ec2". 52
tests fail because they either try to read from <a href="http://cloud-images.ubunt.com" target="_blank">cloud-images.ubunt.com</a>
directly, or because they try to read from
test:/streams/v1/index.sjson but don't accept the signature on
those files.</div>
<div><br>
</div>
<div><br>
</div>
<div>Is anyone able to help me tackle cleaning up our test
suite?</div>
<div><br>
Thanks,</div>
<div>John</div>
<div>=:-></div>
<div><br>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>