Questions after testtools merge

[Jonathan Lange]

On Fri, Jan 1, 2010 at 3:27 AM, John Arbash Meinel
<john at arbash-meinel.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Vincent Ladeuil wrote:
>>>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:
>>
>> <snip/>
>>
>>     jam> So I'm pretty sure that always calling tearDown *was* something that we
>>     jam> had to explicitly put into our .run() method. And it certainly could
>>     jam> have been lost in the transition.
>>
>> I'd be surprised if that was the case but if it's verified it's indeed a
>> very serious bug.
>>
>> I'm surprised that no test catches that though...
>
> IIRC stock unittest does not call tearDown if setUp does not complete.
>

Correct. testtools follows its lead.

> Looking at the code in testtools.runtest there is _run_core which has:
>
>        if self.exception_caught == self._run_user(self.case._run_setup,
>            self.result):
>            # Don't run the test method if we failed getting here.
>            self.case._runCleanups(self.result)
>            return
>
> So *if* an exception is caught during setup, it runs cleanups, but not
> teardowns.

Yes, this is a deliberate design decision -- we felt that there should
be a way of guaranteeing that a cleanup gets run.

> Because later on we have:
>
>        try:
>            if self.exception_caught == self._run_user(
>                self.case._run_test_method, self.result):
>                failed = True
>        finally:
>            try:
>                if self.exception_caught == self._run_user(
>                    self.case._run_teardown, self.result):
>                    failed = True
>            finally:
>                try:
>                    if not self._run_user(
>                        self.case._runCleanups, self.result):
>                        failed = True
>                finally:
>                    if not failed:
>                        self.result.addSuccess(self.case,
>                            details=self.case.getDetails())
>
> Also, I think we have a bug, looking at addCleanups, it says:
>    def addCleanup(self, function, *arguments, **keywordArguments):
>        """Add a cleanup function to be called before tearDown.
>
>
> However, if I read the above correctly, teardown is clearly called
> *before* cleanups. Which doesn't quite fit with the fact that setUp is
> called before addCleanup, etc.
>

The docstring is definitely wrong. Given that you had to delve so
deeply into the code, it's also inadequate. I'll fix this now.

jml