[MERGE] compare_trees with all unknowns

Robert Collins robertc at robertcollins.net
Wed Jul 26 23:40:41 BST 2006


On Wed, 2006-07-26 at 15:57 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> 
> ...
> 
> > FYI
> > Heres an incremental patch to my tree_implementations patch that adds
> > trees matching all your test trees - I'll be using these in the
> > Tree.compare() patch.
> > 
> > Cheers,
> > Rob
> > 
> > 
> 
> ...
> 
> v- This is the full permutation (which is nice). However, should it
> really be part of the base level TestTreeImplementations class?
> It seems more specialized to specifically testing 'InterTree.compare()'
> so it should be on a different base class.

Well, we want to be sure that every tree is able to represent these
states and test them for conformance. InterTree tests the InterTreecode
path, which depends on *some* permutations of the Tree Formats - but not
all. So no, this does not belong in the InterTree test suite - we're
testing that implementations of Tree can represent these test trees.


> >  
> > +    def _make_abc_tree(self, tree):
> > +        """setup an abc content tree."""
> > +        files = ['a', 'b/', 'b/c']
> > +        self.build_tree(files, transport=tree.bzrdir.root_transport)
> > +        tree.add(files, ['a-id', 'b-id', 'c-id'])
> 
> ...
> 
> v- I realize you are testing it here, though. So maybe it is okay. If
> you think it is something that a large portion of the Tree tests are
> going to want.

Well, its both something they need, and its tests for them - if you
can't construct these test trees, your tree implementation does not meet
our contract.


> ...
> 
> > 
> > === modified file 'bzrlib/workingtree.py'
> > --- bzrlib/workingtree.py	2006-07-24 03:34:16 +0000
> > +++ bzrlib/workingtree.py	2006-07-25 03:14:21 +0000
> > @@ -459,6 +459,9 @@
> >      def get_file(self, file_id):
> >          return self.get_file_byname(self.id2path(file_id))
> >  
> > +    def get_file_text(self, file_id):
> > +        return self.get_file(file_id).read()
> > +
> >      def get_file_byname(self, filename):
> >          return file(self.abspath(filename), 'rb')
> 
> 
> Is there a need for 'get_file_text()'?
> 
> So a couple small issues. If you have good responses for them, +1 from me.

The ''/'_text'/'_lines' pattern for describing whether we want a file, a
text or a sequence of lines seems very clean to me. my tests needed to
check the contents of files and a string was the most convenient - so I
added that method (which already exists on RevisionTree).

Do we *need* it ? Well, we have this somewhat bad pattern of casting
lots of things into StringIO because our interface does not have this
three-way-split consistently available, so yes, I think this is good to
have on Tree.

Cheers,
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/dad31935/attachment.pgp 


More information about the bazaar mailing list