[Merge] workingtree.walkdirs to support walking an empty directory

Martin Pool mbp at canonical.com
Thu May 3 07:09:12 BST 2007


Martin Pool has voted +0.
Status is now: Waiting
Comment:
That sounds like a good thing to fix, thanks.

+    added='added'
+    missing='missing'
+    unknown='unknown'
+
+    class DirBlock:
+

I don't understand why DirBlock is inside the test class.  Our general 
practice is to define classes at the top level so it's clear they're not 
coupled to another class and so the indented blocks are less long -- 
even if they're only used from one other class.

A comment on this class would be good - that it gives you an object 
representation of the tuples returned by dirstate.  (Stated like that 
it's more clear that other tests might want to use it.)

-                if current_disk[0][1][top_strip_len:] == '':
+                if (current_disk[0][1][top_strip_len:] == '' and
+                    len(current_disk[1]) > 0):

Sorry I don't understand how this works - can you explain?

+        tree, expected_dirblocks = self.get_tree(self.added, prefix)
+        tree.lock_read()
+        for dirinfo, dirblock in tree.walkdirs(prefix):
+            result.append((dirinfo, list(dirblock)))
+        tree.unlock()
+
+        # check each return value for debugging ease.
+        for pos, item in enumerate(expected_dirblocks):
+            result_pos = []
+            if len(result) > pos:
+                result_pos = result[pos]
+            self.assertEqual(item, result_pos)
+
+        self.assertEqual(len(expected_dirblocks), len(result))
+

It looks like all this is repeated between the two methods and could be 
factored out?

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C418c22640705012222h5a72f900j4d0d7ae2c655a186%40mail.gmail.com%3E



More information about the bazaar mailing list