[MERGE] Improvements to test_bzrdir

Andrew Bennetts andrew at canonical.com
Fri Aug 18 06:26:53 BST 2006


Robert Collins wrote:
> On Thu, 2006-08-17 at 16:23 +1000, Martin Pool wrote:
> > On 17 Aug 2006, Robert Collins <robertc at robertcollins.net> wrote:
[...]
> > > Theres a lot of duplicated try:except blocks in there - I suggest a
> > > helper function like 'requireWorkingTreeIfLocal(a_bzrdir)' or some such.
> > 
> > I agree it should be factored out.  Not so sure about that name.  The
> > desired behaviour, I think, is:
> > 
> >  - if the target is not local, the test can't proceed so skip
> >  - if it is local, build a working tree
> > 
> > Perhaps createWorkingTreeOrSkip() then.
> 
> There are two separate cases : one that creates a tree, the other that
> tries to open one that was created I think.
> 
> But I like the name createWorkingTreeOrSkip for thae case of creation.

Done.  I've added skipIfNoWorkingTree and createWorkingTreeOrSkip helpers.  I've
stuck with camelCase as suggested in email and on IRC (so as to be consistent
with unittest's style), although precedent for test helpers in
TestCaseWithTransport seems to be underscored_words instead.

The branch can be found at http://people.ubuntu.com/~andrew/bzr.for.merge, and
I've attached the latest patch.

There's still some fairly repetitive try-excepts that raise TestSkipped in
the test_sprout test methods, but I think they're ok as is.

-Andrew.

-------------- next part --------------
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2006-08-15 13:06:11 +0000
+++ bzrlib/bzrdir.py	2006-08-17 07:02:13 +0000
@@ -615,7 +615,8 @@
                 # XXX FIXME RBC 20060214 need tests for this when the basis
                 # is incomplete
                 result_repo.fetch(basis_repo, revision_id=revision_id)
-            result_repo.fetch(source_repository, revision_id=revision_id)
+            if source_repository is not None:
+                result_repo.fetch(source_repository, revision_id=revision_id)
         if source_branch is not None:
             source_branch.sprout(result, revision_id=revision_id)
         else:

=== modified file 'bzrlib/tests/bzrdir_implementations/test_bzrdir.py'
--- bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2006-08-05 22:33:36 +0000
+++ bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2006-08-18 05:14:02 +0000
@@ -104,6 +104,29 @@
                                          target.get(path).read(),
                                          "text for file %r differs:\n" % path)
 
+    def skipIfNoWorkingTree(self, a_bzrdir):
+        """Raises TestSkipped if a_bzrdir doesn't have a working tree.
+        
+        If the bzrdir does have a workingtree, this is a no-op.
+        """
+        try:
+            a_bzrdir.open_workingtree()
+        except (errors.NotLocalUrl, errors.NoWorkingTree):
+            raise TestSkipped("bzrdir on transport %r has no working tree"
+                              % a_bzrdir.transport)
+
+    def createWorkingTreeOrSkip(self, a_bzrdir):
+        """Create a working tree on a_bzrdir, or raise TestSkipped.
+        
+        A simple wrapper for create_workingtree that translates NotLocalUrl into
+        TestSkipped.  Returns the newly created working tree.
+        """
+        try:
+            return a_bzrdir.create_workingtree()
+        except errors.NotLocalUrl:
+            raise TestSkipped("cannot make working tree with transport %r"
+                              % a_bzrdir.transport)
+
     def test_clone_bzrdir_empty(self):
         dir = self.make_bzrdir('source')
         target = dir.clone(self.get_url('target'))
@@ -332,6 +355,7 @@
         tree.commit('revision 1')
         dir = tree.bzrdir
         target = dir.clone(self.get_url('target'))
+        self.skipIfNoWorkingTree(target)
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/stat-cache',
@@ -348,6 +372,7 @@
         tree.commit('revision 1')
         dir = tree.bzrdir
         target = dir.clone(self.get_url('target'))
+        self.skipIfNoWorkingTree(target)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/stat-cache',
                                      './.bzr/checkout/stat-cache',
@@ -373,8 +398,9 @@
         except errors.IncompatibleFormat:
             # this is ok too, not all formats have to support references.
             return
-        dir.create_workingtree()
+        self.createWorkingTreeOrSkip(dir)
         target = dir.clone(self.get_url('target'))
+        self.skipIfNoWorkingTree(target)
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/stat-cache',
@@ -382,7 +408,6 @@
                                      './.bzr/repository/inventory.knit',
                                      ])
 
-
     def test_clone_bzrdir_tree_revision(self):
         # test for revision limiting, [smoke test, not corner case checks].
         # make a tree with a revision with a last-revision
@@ -396,6 +421,7 @@
         tree.commit('revision 2', rev_id='2', allow_pointless=True)
         dir = tree.bzrdir
         target = dir.clone(self.get_url('target'), revision_id='1')
+        self.skipIfNoWorkingTree(target)
         self.assertEqual('1', target.open_workingtree().last_revision())
 
     def test_clone_bzrdir_incomplete_source_with_basis(self):
@@ -414,12 +440,20 @@
         dir = source.bzrdir
         target = dir.clone(self.get_url('target'), basis=tree.bzrdir)
         self.assertEqual('2', target.open_branch().last_revision())
-        self.assertEqual('2', target.open_workingtree().last_revision())
+        try:
+            self.assertEqual('2', target.open_workingtree().last_revision())
+        except errors.NoWorkingTree:
+            # It should have a working tree if it's able to have one, so if
+            # we're here make sure it really can't have one.
+            self.assertRaises(errors.NotLocalUrl, target.create_workingtree)
         self.assertTrue(target.open_branch().repository.has_revision('2'))
 
     def test_sprout_bzrdir_empty(self):
         dir = self.make_bzrdir('source')
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target = dir.sprout(self.get_url('target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # creates a new repository branch and tree
         target.open_repository()
@@ -433,12 +467,15 @@
             self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'))
+        try:
+            target = dir.sprout(self.get_url('target/child'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertRaises(errors.NoRepositoryPresent, target.open_repository)
         target.open_branch()
         target.open_workingtree()
 
-    def test_sprout_bzrdir_empty_under_shared_repo(self):
+    def test_sprout_bzrdir_empty_under_shared_repo_force_new(self):
         # the force_new_repo parameter should force use of a new repo in an empty
         # bzrdir's sprout logic
         dir = self.make_bzrdir('source')
@@ -446,7 +483,11 @@
             self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'), force_new_repo=True)
+        try:
+            target = dir.sprout(self.get_url('target/child'),
+                                force_new_repo=True)
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         target.open_repository()
         target.open_branch()
         target.open_workingtree()
@@ -460,7 +501,10 @@
         repo = dir.create_repository()
         repo.fetch(tree.branch.repository)
         self.assertTrue(repo.has_revision('1'))
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target = dir.sprout(self.get_url('target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/repository/inventory.knit',
@@ -481,7 +525,10 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'))
+        try:
+            target = dir.sprout(self.get_url('target/child'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertTrue(shared_repo.has_revision('1'))
 
@@ -500,7 +547,10 @@
         tree.bzrdir.open_repository().copy_content_into(shared_repo)
         dir = self.make_bzrdir('shared/source')
         dir.create_branch()
-        target = dir.sprout(self.get_url('shared/target'))
+        try:
+            target = dir.sprout(self.get_url('shared/target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertNotEqual(dir.transport.base, shared_repo.bzrdir.transport.base)
         self.assertTrue(shared_repo.has_revision('1'))
@@ -523,7 +573,10 @@
         self.assertTrue(shared_repo.has_revision('1'))
         dir = self.make_bzrdir('shared/source')
         dir.create_branch()
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target = dir.sprout(self.get_url('target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertNotEqual(dir.transport.base, shared_repo.bzrdir.transport.base)
         branch = target.open_branch()
@@ -546,7 +599,11 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'), force_new_repo=True)
+        try:
+            target = dir.sprout(self.get_url('target/child'),
+                                force_new_repo=True)
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertFalse(shared_repo.has_revision('1'))
 
@@ -565,7 +622,10 @@
         source = self.make_repository('source')
         tree.bzrdir.open_repository().copy_content_into(source)
         dir = source.bzrdir
-        target = dir.sprout(self.get_url('target'), revision_id='2')
+        try:
+            target = dir.sprout(self.get_url('target'), revision_id='2')
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         raise TestSkipped('revision limiting not strict yet')
 
     def test_sprout_bzrdir_branch_and_repo(self):
@@ -577,7 +637,10 @@
         tree.bzrdir.open_repository().copy_content_into(source.repository)
         tree.bzrdir.open_branch().copy_content_into(source)
         dir = source.bzrdir
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target = dir.sprout(self.get_url('target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/stat-cache',
@@ -602,7 +665,10 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'))
+        try:
+            target = dir.sprout(self.get_url('target/child'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertTrue(shared_repo.has_revision('1'))
 
     def test_sprout_bzrdir_branch_and_repo_shared_force_new_repo(self):
@@ -620,7 +686,11 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'), force_new_repo=True)
+        try:
+            target = dir.sprout(self.get_url('target/child'),
+                                force_new_repo=True)
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertFalse(shared_repo.has_revision('1'))
 
@@ -635,7 +705,10 @@
             # this is ok too, not all formats have to support references.
             return
         self.assertRaises(errors.NoRepositoryPresent, dir.open_repository)
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target = dir.sprout(self.get_url('target'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # we want target to have a branch that is in-place.
         self.assertEqual(target, target.open_branch().bzrdir)
@@ -659,7 +732,10 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'))
+        try:
+            target = dir.sprout(self.get_url('target/child'))
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # we want target to have a branch that is in-place.
         self.assertEqual(target, target.open_branch().bzrdir)
@@ -685,7 +761,11 @@
             shared_repo = self.make_repository('target', shared=True)
         except errors.IncompatibleFormat:
             return
-        target = dir.sprout(self.get_url('target/child'), force_new_repo=True)
+        try:
+            target = dir.sprout(self.get_url('target/child'),
+                                force_new_repo=True)
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # we want target to have a branch that is in-place.
         self.assertEqual(target, target.open_branch().bzrdir)
@@ -708,7 +788,12 @@
         tree.bzrdir.open_repository().copy_content_into(source.repository)
         tree.bzrdir.open_branch().copy_content_into(source)
         dir = source.bzrdir
-        target = dir.sprout(self.get_url('target'), revision_id='1')
+        try:
+            target_url = self.get_url('target')
+            target = dir.sprout(target_url, revision_id='1')
+        except errors.NotLocalUrl:
+            raise TestSkipped("sprout cannot make working tree at %r"
+                              % target_url)
         self.assertEqual('1', target.open_branch().last_revision())
         
     def test_sprout_bzrdir_tree_branch_repo(self):
@@ -717,7 +802,12 @@
         tree.add('foo')
         tree.commit('revision 1')
         dir = tree.bzrdir
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target_url = self.get_url('target')
+            target = dir.sprout(target_url)
+        except errors.NotLocalUrl:
+            raise TestSkipped("sprout cannot make working tree at %r"
+                              % target_url)
         self.assertNotEqual(dir.transport.base, target.transport.base)
         self.assertDirectoriesEqual(dir.root_transport, target.root_transport,
                                     ['./.bzr/stat-cache',
@@ -739,10 +829,15 @@
             # this is ok too, not all formats have to support references.
             return
         self.assertRaises(errors.NoRepositoryPresent, dir.open_repository)
-        tree = dir.create_workingtree()
+        tree = self.createWorkingTreeOrSkip(dir)
         tree.bzrdir.root_transport.mkdir('subdir')
         tree.add('subdir')
-        target = dir.sprout(self.get_url('target'))
+        try:
+            target_url = self.get_url('target')
+            target = dir.sprout(target_url)
+        except (errors.NotLocalUrl, errors.NoWorkingTree):
+            raise TestSkipped("sprout cannot make working tree at %r"
+                              % target_url)
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # we want target to have a branch that is in-place.
         self.assertEqual(target, target.open_branch().bzrdir)
@@ -765,12 +860,13 @@
             # this is ok too, not all formats have to support references.
             return
         self.assertRaises(errors.NoRepositoryPresent, dir.open_repository)
-        tree = dir.create_workingtree()
+        tree = self.createWorkingTreeOrSkip(dir)
         self.build_tree(['foo'], transport=dir.root_transport)
         tree.add('foo')
         tree.commit('revision 1', rev_id='1')
         tree.commit('revision 2', rev_id='2', allow_pointless=True)
         target = dir.sprout(self.get_url('target'), revision_id='1')
+        self.skipIfNoWorkingTree(target)
         self.assertNotEqual(dir.transport.base, target.transport.base)
         # we want target to have a branch that is in-place.
         self.assertEqual(target, target.open_branch().bzrdir)
@@ -793,7 +889,12 @@
         tree.commit('revision 1', rev_id='1')
         tree.commit('revision 2', rev_id='2', allow_pointless=True)
         dir = tree.bzrdir
-        target = dir.sprout(self.get_url('target'), revision_id='1')
+        try:
+            target_url = self.get_url('target')
+            target = dir.sprout(target_url, revision_id='1')
+        except errors.NotLocalUrl:
+            raise TestSkipped("sprout cannot make working tree at %r"
+                              % target_url)
         self.assertEqual('1', target.open_workingtree().last_revision())
 
     def test_sprout_bzrdir_incomplete_source_with_basis(self):
@@ -810,7 +911,10 @@
         tree.copy_content_into(source)
         self.assertFalse(source.branch.repository.has_revision('2'))
         dir = source.bzrdir
-        target = dir.sprout(self.get_url('target'), basis=tree.bzrdir)
+        try:
+            target = dir.sprout(self.get_url('target'), basis=tree.bzrdir)
+        except errors.NotLocalUrl:
+            raise TestSkipped('Cannot sprout to remote bzrdirs.')
         self.assertEqual('2', target.open_branch().last_revision())
         self.assertEqual('2', target.open_workingtree().last_revision())
         self.assertTrue(target.open_branch().repository.has_revision('2'))
@@ -994,8 +1098,8 @@
         except errors.IncompatibleFormat:
             found_transport = dir.get_branch_transport(identifiable_format)
         self.assertTrue(isinstance(found_transport, transport.Transport))
-        # and the dir which has been initialized for us must be statable.
-        found_transport.stat('.')
+        # and the dir which has been initialized for us must exist.
+        found_transport.list_dir('.')
 
     def test_get_repository_transport(self):
         dir = self.make_bzrdir('.')
@@ -1015,8 +1119,8 @@
         except errors.IncompatibleFormat:
             found_transport = dir.get_repository_transport(identifiable_format)
         self.assertTrue(isinstance(found_transport, transport.Transport))
-        # and the dir which has been initialized for us must be statable.
-        found_transport.stat('.')
+        # and the dir which has been initialized for us must exist.
+        found_transport.list_dir('.')
 
     def test_get_workingtree_transport(self):
         dir = self.make_bzrdir('.')
@@ -1036,8 +1140,8 @@
         except errors.IncompatibleFormat:
             found_transport = dir.get_workingtree_transport(identifiable_format)
         self.assertTrue(isinstance(found_transport, transport.Transport))
-        # and the dir which has been initialized for us must be statable.
-        found_transport.stat('.')
+        # and the dir which has been initialized for us must exist.
+        found_transport.list_dir('.')
 
     def test_root_transport(self):
         dir = self.make_bzrdir('.')
@@ -1171,12 +1275,12 @@
         dir.needs_format_conversion(None)
 
     def test_upgrade_new_instance(self):
-        """Does an available updater work ?."""
+        """Does an available updater work?"""
         dir = self.make_bzrdir('.')
-        # for now, check is not ready for partial bzrdirs.
+        # for now, upgrade is not ready for partial bzrdirs.
         dir.create_repository()
         dir.create_branch()
-        dir.create_workingtree()
+        self.createWorkingTreeOrSkip(dir)
         if dir.can_convert_format():
             # if its default updatable there must be an updater 
             # (we change the default to match the lastest known format



More information about the bazaar mailing list