juju/testing: suite-level Patch never gets restored

[Roger Peppe]

If we call CleanupSuite.PatchValue inside SetUpSuite, the value
never gets restored. This is unintuitive (I've found some examples
in the wild that assume it just works).

It should be straightforward to make it work:

- set a field, say suiteSetup, inside CleanupSuite.SetUpSuite to indicate that we're setting up the suite;
- change TearDownTest to clear s.testStack after running callStack.
- in CleanupSuite.SetUpTest, if we find that suiteSetup is
set and testStack is non-empty, move suiteStack to testStack.
- in CleanupSuite.SetUpTest, clear suiteSetup.

[Jimmie Butler (jimmiebtlr)]

Think I may be misunderstanding the problem here.

It seems like the original value is correctly restored on test tear down.
https://github.com/juju/testing/pull/15

[Andrew Wilkins (axwalk)]

@jimmiebtlr: I think Roger is suggesting that PatchValue should be smarter, and have different behaviour depending on whether it's called from within suite setup or from within an individual test case. Seems like a good idea to me.

[Roger Peppe (rogpeppe)]

Actually that is not just a good idea, it is important that it happens that
way. The places that currently call Patch at suite level assume that the
value stays patched for the duration of the suite. By restoring inside the
test, we end up with the wrong value in all but the first test.

I will comment on the pull request directly.

On 10 Jun 2014 02:20, "Andrew Wilkins" <email address hidden> wrote:

> @jimmiebtlr: I think Roger is suggesting that PatchValue should be
> smarter, and have different behaviour depending on whether it's called
> from within suite setup or from within an individual test case. Seems
> like a good idea to me.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1308101
>
> Title:
> juju/testing: suite-level Patch never gets restored
>
> Status in juju-core:
> Triaged
>
> Bug description:
> If we call CleanupSuite.PatchValue inside SetUpSuite, the value
> never gets restored. This is unintuitive (I've found some examples
> in the wild that assume it just works).
>
> It should be straightforward to make it work:
>
> - set a field, say suiteSetup, inside CleanupSuite.SetUpSuite to
> indicate that we're setting up the suite;
> - change TearDownTest to clear s.testStack after running callStack.
> - in CleanupSuite.SetUpTest, if we find that suiteSetup is
> set and testStack is non-empty, move suiteStack to testStack.
> - in CleanupSuite.SetUpTest, clear suiteSetup.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/juju-core/+bug/1308101/+subscriptions
>

[Anastasia (anastasia-macmood)]

There have been a lot of improvements in the area. We believe this to be fixed now.
If you know otherwise, please file a bug against "juju" with concrete examples.