PatchValue (AddCleanup) unsafe with non pointer receiver test

[John Meinel]

https://github.com/juju/testing/pull/92 is a possible "fix" for this. It
just panic() if it notices that the suite that was seen in SetUpSuite() is
different than the one seen in AddSuiteCleanup(), and similarly for SetUp()
and AddCleanup()

A different possibility would be to ignore "s" and only use the original
pointer? Would that be a better fix?

John
=:->


On Thu, Mar 17, 2016 at 8:52 AM, John Meinel <john at arbash-meinel.com> wrote:

> I came across this in the LXD test suite today, which was hard to track
> down, so I figured I'd let everyone know about it.
>
> We have a nice helper in testing.IsolationSuite with "PatchValue()" that
> will change a global for you during the test, and then during TearDown()
> will cleanup the patch it made.
> It turns out that if your test doesn't have a pointer receiver this fails,
> because the "suite" object is a copy, so when PatchValue calls AddCleanup
> to do s.testStack = append(...) the suite object goes away before TearDown
> is called.
>
> You can see this with the attached test suite.
>
> Example:
>
> func (s mySuite) TestFoo(c *gc.C) {
>  // This is unsafe because s.PatchValue ends up modifying s.testStack but
> that attribute only exists
>  // for the life of the TestFoo function
>   s.PatchValue(&globalVar, "newvalue")
> }
>
> I tried adding the attached patch so that we catch places we are using
> AddCleanup unsafely, but it fails a few tests I wasn't expecting, so I'm
> not sure if I'm actually doing the right thing.
>
> John
> =:->
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20160317/6c137ac2/attachment.html>