Questions after testtools merge

[John Arbash Meinel]

-----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.

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. 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.

So

1) I'm fairly sure that if an exception is raised in setUp() we no
longer call tearDown.
2) I'm also fairly sure that cleanups are getting called after tearDown.
And that cleanups are *not* being called inside tearDown.

At least, unless we override this again in bzrlib. I think we do
override addCleanup, which means we may be doing things differently.
Though when we get rid of that, we may run in to the 'cleanups called
after teardown' as a potential bug.

> 
>     jam> That said, we've moved to the 'addCleanup()' style of teardown,
>     jam> so we could just switch those failing tests over to the new way
>     jam> of doing things.
> 
> cleanups are called during tearDown so that should not address the
> problem :-/
> 
>         Vincent
> 


John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAks80QgACgkQJdeBCYSNAAPqfQCdG/AhhrSrn+qChCDi3JOB0lON
SckAoJ0W3LKpeddGmXmSVenR9vaZxGG4
=V7L9
-----END PGP SIGNATURE-----