Warning: test suite not actually isolated

[John Meinel]

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.)
I also found a couple flaky tests that pass when I run them directly:
https://bugs.launchpad.net/juju-core/+bug/1559400
https://bugs.launchpad.net/juju-core/+bug/1559402

I also discovered another insidious failure:

func (s *MySuite) SetUpTest(c *gc.C) {
  s.PatchValue()
  s.Base.SetUpTest(c)
}

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.

But my branch is up and proposed:
  http://reviews.vapour.ws/r/4240/diff/#

The one really big and nice thing about this branch, is that it should mean
you won't ever find that:
  go test
passes but
 go test -check.f OneTest
fails because of preconditions not being satisfied.

John
=:->


On Fri, Mar 18, 2016 at 4:59 AM, Anastasia Macmood <
anastasia.macmood at canonical.com> wrote:

> John
>
> Thank you for discovering and working on this!
>
> This sounds like an awful can of worms and should be addressed in a
> dedicated manner.
>
> I'll add it to the tech debt board and bug squad board with a reference to
> your branch.
>
> 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/
>
> Sincerely Yours,
>
> Anastasia
>
> On 17/03/16 19:43, John Meinel wrote:
>
> tl; dr: I need some help fixing our tests that are incorrectly not
> isolated from the real world.
>
> So...
>
> In investigating the bug with PatchValue and non pointer receivers, we
> realized that PatchValue calls AddCleanup. Which means that If you are
> doing:
> func ... SetUpSuite() {
>    s.PatchValue(&foo, safeFoo)
> }
>
> 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.
>
> 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.
>
> But that breaks about 100 tests that weren't as isolated as they thought
> they were.
>
> It also breaks a few tests that were Patching values before calling
> IsolationSuite.SetUpTest/Suite(), eg:
> SetUpTest() {
>   PatchValue()
>   Base.SetUpTest()
> }
>
> My branch that starts working on this is at:
>  git at github.com:jameinel/juju unsafe-cleanups
>
> One of the big examples of bad tests is "provider/ec2". 52 tests fail
> because they either try to read from cloud-images.ubunt.com directly, or
> because they try to read from test:/streams/v1/index.sjson but don't accept
> the signature on those files.
>
>
> Is anyone able to help me tackle cleaning up our test suite?
>
> Thanks,
> John
> =:->
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20160319/79bc6a65/attachment.html>