[MERGE] "Branch.open()" should re-use the connection for stacked branches

[Vincent Ladeuil]

>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:

<snip/>

    >> If I remember correctly, raising TestNotApplicable during setUp
    >> is bad because tearDown is not called ? It may not matter that
    >> much here but I think it's better to avoid that idiom.
    >> 
    >> Defining a helper should solve that problem but forces you to
    >> call it in every test.

    jam> I've used the idiom before, and didn't think it was a
    jam> problem.

    jam> Certainly I see this:
    jam>     def _addSkipped(self, test, skip_excinfo):
    jam> ....
    jam>         try:
    jam>             test.tearDown()
    jam>         except KeyboardInterrupt:
    jam>             raise

    jam> So if we get a "skipped" test, we explicitly call
    jam> tearDown. Looking at the rest of the code, it seems that
    jam> unittest.TestCase.run() does:

    jam> try:
    jam>     self.setUp()
    jam> except KeyboardInterrupt:
    jam>     raise
    jam> except:
    jam>     result.addError(self, self._exc_info())
    jam>     return

    jam> And then in bzrlib.tests.ExtendedTestResult we override addError with:

    jam> if isinstance(err[1], TestSkipped):
    jam>     return self._addSkipped(test, err)

Wow ! My case was raising UnavailableFeature which isn't an
instance of TestSkipped...

    jam> And in self._addSkipped() we call tearDown() as I mentioned.

    jam> So I'm pretty sure it is allowed.

Correct.

    jam> I would be fine to hear other opinions if we should
    jam> really not be using TestSkipped/TestNotApplicable during
    jam> setUp().

Or should we make UnavailableFeature inherit from TestSkipped ?

If you look at bzrlib.transport.ftp.UnavailableFTPServer, you'll
see that we do some contortions to work around the problem.

    Vincent