Merge lp:~jeff-licquia/bzr-bisect/yesno into lp:bzr-bisect

[Jeff Licquia]

This branch fixes bug 1051395. The meaning of "yes" and "no" has always been unclear, largely because the original idea for how they should work hasn't matched how it actually works. So, this patch makes things work the way they're described in the docs.

I've put a more detailed explanation in the bug.

Unmerged revisions

85. By Jeff Licquia on 2012-09-16

Don't hardcode the association between "yes" and "high revision".

Originally, the intent was for "yes" and "no" to mean whatever the user
wanted it to mean, and figure out at runtime whether that made "yes" or
"no" the high rev. This feature was lost at some point (if it ever
existed) and "yes" was assumed to mean "high rev". This was confusing,
because the docs implied you could make "yes" or "no" mean whatever you
wanted.

Now, we figure out from the bisect log which identifier is high, and
stick with that moving forward. Also, there's now a test testing the
"no is high" case.

=== modified file 'cmds.py'
--- cmds.py	2012-07-19 22:22:27 +0000
+++ cmds.py	2012-09-16 15:30:25 +0000
@@ -111,6 +111,7 @@
         self._low_revid = None
         self._middle_revid = None
         self._filename = filename
+        self._high_indicator = None
         self.load()
 
     def _open_for_read(self):
@@ -160,10 +161,12 @@
                     continue
                 if len(matches) > 1:
                     raise RuntimeError("revision %s duplicated" % revision)
-                if matches[0] == "yes":
+                if not self._high_indicator:
+                    self._high_indicator = matches[0]
+                if matches[0] == self._high_indicator:
                     high_revid = revision
                     between_revs = []
-                elif matches[0] == "no":
+                else:
                     low_revid = revision
                     del between_revs[0]
                     break

=== modified file 'tests.py'
--- tests.py	2011-11-24 16:04:11 +0000
+++ tests.py	2012-09-16 15:30:25 +0000
@@ -253,6 +253,38 @@
         self.run_bzr(['bisect', 'no'])
         self.assertRevno(3)
 
+    def testWorkflowReverse(self):
+        """Run through a basic usage scenario, except with senses of
+        'yes' and 'no' reversed.  The plugin should pick up on that
+        and adjust."""
+
+        # Start up the bisection.  When the two ends are set, we should
+        # end up in the middle.
+
+        self.run_bzr(['bisect', 'start'])
+        self.run_bzr(['bisect', 'no'])
+        self.run_bzr(['bisect', 'yes', '-r', '1'])
+        self.assertRevno(3)
+
+        # Mark feature as absent in the middle.  Should move us
+        # halfway back between the current middle and the start.
+
+        self.run_bzr(['bisect', 'no'])
+        self.assertRevno(2)
+
+        # Mark feature as present.  Since this is only one
+        # rev back from the lowest marked revision without the feature,
+        # the process should end, with the current rev set to the
+        # rev following.
+
+        self.run_bzr(['bisect', 'yes'])
+        self.assertRevno(3)
+
+        # Run again.  Since we're done, this should do nothing.
+
+        self.run_bzr(['bisect', 'yes'])
+        self.assertRevno(3)
+
     def testWorkflowSubtree(self):
         """Run through a usage scenario where the offending change
         is in a subtree."""