test_format_initialize_find_open has some isolation problems

[Martin Pool]

I've seen test_format_initialize_find_open fail sometimes when '--first transport' is used with selftest, but it passes when run by itself. So I suspect some test isolation error.

Traceback (most recent call last):
  File "/home/mbp/lib/python/testtools/runtest.py", line 128, in _run_user
    return fn(*args)
  File "/home/mbp/lib/python/testtools/testcase.py", line 369, in _run_test_method
    testMethod()
  File "/home/mbp/bzr/trivial/bzrlib/tests/per_bzrdir/test_bzrdir.py", line 1185, in test_format_initialize_find_open
    bzrdir.BzrDirFormat.find_format(readonly_t))
AssertionError: not equal:
a = RemoteBzrDirFormat(_network_name='Bazaar-NG meta directory, format 1\n')
b = RemoteBzrDirFormat(_network_name=None)

[Martin Pool (mbp)]

I suspect some global state in the format registry is being tweaked by an earlier test.

This is comparing:

a= the bzrdir format for this instance of the parameterized test
b= the actual format observed when opening this transport

so what we see here would be consistent with

1- opening the format not setting the network name
2- the network name being put into the format as a bzrdir instance is initialized; or the network name being set in the parameterization of this test

[Martin Pool (mbp)]

When I run only these tests, in which case it passes, it looks like the network name is not set in either a or b. Thta seems a bit inconsistent with the format remembering its network name when it's initialized on the transport but perhaps we could assume this is the right behaviour because it's the existing behaviour.

[Robert Collins (lifeless)]

On Thu, 2010-01-07 at 03:22 +0000, Martin Pool wrote:
>
> 1- opening the format not setting the network name
> 2- the network name being put into the format as a bzrdir instance is
> initialized; or the network name being set in the parameterization of
> this test

I agree with your hypothesis; I'll note that only Remote*Format have
network names set dynamically - the rest are hard coded; and
Remote*Format shouldn't be in the registry regardless.

-Rob

[Martin Pool (mbp)]

It turns out that, depending on the test ordering, this is sometimes run with the bzrdir_format already having a _network_name set.

=== modified file 'bzrlib/tests/per_bzrdir/test_bzrdir.py'
--- bzrlib/tests/per_bzrdir/test_bzrdir.py 2009-11-18 17:10:41 +0000
+++ bzrlib/tests/per_bzrdir/test_bzrdir.py 2010-01-08 06:26:24 +0000
@@ -1176,6 +1176,13 @@
             # because the default open will not open them and
             # they may not be initializable.
             return
+ # for remote formats, there must be no prior assumption about the
+ # network name to use - it's possible that this may somehow have got
+ # in through an unisolated test though - see
+ # <https://bugs.edge.launchpad.net/bzr/+bug/504102>
+ self.assertEquals(getattr(self.bzrdir_format,
+ '_network_name', None),
+ None)
         # supported formats must be able to init and open
         t = get_transport(self.get_url())
         readonly_t = get_transport(self.get_readonly_url())

[Martin Pool (mbp)]

After some irc discussion we seem to agree that it's a bug that the RemoteBzrDirFormat contains a network_name that's implicitly mutated by calling methods on it. There is a general assumption that format objects are stateless/immutable. I'm not sure how hard that would be to remove.

Perhaps a bit hard because there are other methods on the Format that really are asking about the instance of the bzrdir.

For the moment I'm going to see if we can just avoid the side effect of setting the format network name.