Re: PatchValue (AddCleanup) unsafe with non pointer receiver test

2016-03-19 Thread roger peppe
On 17 March 2016 at 04:52, John Meinel 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,

PatchValue (AddCleanup) unsafe with non pointer receiver test

2016-03-19 Thread John Meinel
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 mad

Re: PatchValue (AddCleanup) unsafe with non pointer receiver test

2016-03-18 Thread 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 u

Re: PatchValue (AddCleanup) unsafe with non pointer receiver test

2016-03-18 Thread John Meinel
So the reason I thought this fix wouldn't work is because I had a failure in BaseSuite.SetUpSuite(). It turns out, the failure was actually valid. SetUpSuite is doing: s.PatchValue(&utils.OutgoingAccessAllowed, false) However, PatchValue calls AddCleanup, and AddCleanup cleans up during TearDownTe