[RFC] intertree-implementations

[Robert Collins]

On Wed, 2006-07-26 at 15:36 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > This is the first cut at InterTree - I've hooked it up to Tree.compare.
> > From here I plan to expand the tests until all the parameters are
> > present and minimally tested. At that point I intend to deprecate
> > compare_trees as the public interface.
> > 
> > I'm sending this in now for comments and feedback..
> > 
> > Cheers,
> > Rob
> > 
> 
> ...
> 
> 
> > -    def get_tree_no_parents_no_content(self):
> > -        # make a working tree with the right shape
> > -        tree = self.make_branch_and_tree('.')
> > +    def get_tree_no_parents_no_content(self, empty_tree, converter=None):
> > +        """Make a tree with no parents and no contents from empty_tree.
> > +        
> > +        :param empty_tree: A working tree with no content and no parents to
> > +            modify.
> > +        """
> >          # convert that to the final shape
> > -        return self.workingtree_to_test_tree(tree)
> > +        if converter is None:
> > +            converter = self.workingtree_to_test_tree
> > +        return converter(empty_tree)
> 
> This kind of seems like an odd api. Where you have to pass in an empty
> tree, in order to get an empty tree.
> I understand you really want it because you want to use the converter
> parameter.
> 
> >  
> >  
> >  class TreeTestProviderAdapter(WorkingTreeTestProviderAdapter):
> > 
> > === modified file 'bzrlib/tests/tree_implementations/test_test_trees.py'
> > --- bzrlib/tests/tree_implementations/test_test_trees.py	2006-07-21 01:37:47 +0000
> > +++ bzrlib/tests/tree_implementations/test_test_trees.py	2006-07-24 10:15:48 +0000
> > @@ -23,7 +23,8 @@
> >  class TestTreeShapes(TestCaseWithTree):
> >  
> >      def test_empty_tree_no_parents(self):
> > -        tree = self.get_tree_no_parents_no_content()
> > +        tree = self.make_branch_and_tree('.')
> > +        tree = self.get_tree_no_parents_no_content(tree)
> >          self.assertEqual([], tree.get_parent_ids())
> >          self.assertEqual([], tree.conflicts())
> >          self.assertEqual([], list(tree.unknowns()))
> 
> But it leads to a bit of:
> 	Give me an empty tree.
> 	No really, I want an empty tree.
> 
> 
> Otherwise +1 from me.

So there is a little of that smell. However, lets look at what it really
does:
we have two trees that we want to test an operation between. They might
be:
WorkingTreeFormat2, RevisionTree
or
WorkingTreeFormat2, WorkingTreeFormat4
or 
WorkingTreeFormat2, WorkingTreeFormat2

so it was really about finding a good way to make the generative logic
only be written once, but let getting a tree be quite straight forward.
I'm happy to tweak if folk feel there are more obvious ways to do it.

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060727/d471be6c/attachment.pgp