Merge lp:~naesten/bzr-bisect/683822-bisect-start-range-argument into lp:bzr-bisect

[Samuel Bronson]

[Samuel Bronson (naesten)]

Unfortunately, it turns out that this doesn't match the convention used by "run" for the common case of finding when a problem was introduced :-(.

[lp:~naesten/bzr-bisect/683822-bisect-start-range-argument]

87. By Samuel Bronson on 2012-09-18

Roll back my test changes; later, I'll add some tests just for this.

88. By Samuel Bronson on 2012-09-18

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

This gives "yes" and "no" equal standing again.

89. By Samuel Bronson on 2012-09-19

Reverse polarity of "start -r" and "run -r".

Now it matches the "run" commands's return-value handling.

90. By Samuel Bronson on 2012-09-19

Add a test for "bzr bisect run -r".

91. By Samuel Bronson on 2012-09-21

Add some debug output (mostly log-only).

[Samuel Bronson (naesten)]

> Unfortunately, it turns out that this doesn't match the convention used by
> "run" for the common case of finding when a problem was introduced :-(.

Okay, as of revision 89, I've matched the polarities here; now, it's the bulk of the tests that don't match ;-).

Unmerged revisions

91. By Samuel Bronson on 2012-09-21

Add some debug output (mostly log-only).

90. By Samuel Bronson on 2012-09-19

Add a test for "bzr bisect run -r".

89. By Samuel Bronson on 2012-09-19

Reverse polarity of "start -r" and "run -r".

Now it matches the "run" commands's return-value handling.

88. By Samuel Bronson on 2012-09-18

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

This gives "yes" and "no" equal standing again.

87. By Samuel Bronson on 2012-09-18

Roll back my test changes; later, I'll add some tests just for this.

86. By Samuel Bronson on 2012-09-16

Implement the "-r revA..revB" syntax for "start" and "run" suggested in bug 683822.

Quite likely this is buggy, and none of the scenareos in the test suite (some of which I converted to this syntax) are complicated enough to show this: they all seem to start by setting the bisect range to cover all revisions on the branch.

More tests would be quite useful!

85. By Samuel Bronson on 2012-09-15

Cleanup option handling preperatory to implementing bug 683822.

=== modified file 'cmds.py'
--- cmds.py	2012-07-19 22:22:27 +0000
+++ cmds.py	2012-09-21 21:50:23 +0000
@@ -23,7 +23,7 @@
 from bzrlib.commands import Command
 from bzrlib.errors import BzrCommandError
 from bzrlib.option import Option
-from bzrlib.trace import note
+from bzrlib.trace import note, mutter
 
 bisect_info_path = ".bzr/bisect"
 bisect_rev_path = ".bzr/bisect_revid"
@@ -75,6 +75,7 @@
         """Write the current revision's log entry to a file."""
         rev = self._bzrbranch.repository.get_revision(self._revid)
         revno = ".".join([str(x) for x in self.get_current_revno()])
+        mutter("On revision %s (%s)", revno, rev.revision_id)
         out.write("On revision %s (%s):\n%s\n" % (revno, rev.revision_id,
                                                   rev.message))
 
@@ -111,6 +112,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 +162,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
@@ -201,6 +205,7 @@
     def _set_status(self, revid, status):
         """Set the bisect status for the given revid."""
         if not self.is_done():
+            note("Marking %s as %s", revid, status)
             if status != "done" and revid in [x[0] for x in self._items
                                               if x[1] in ['yes', 'no']]:
                 raise RuntimeError("attempting to add revid %s twice" % revid)
@@ -253,6 +258,7 @@
 
     def bisect(self, outf):
         """Using the current revision's status, do a bisection."""
+        mutter("Bisecting")
         self._find_range_and_middle()
         # If we've found the "final" revision, check for a
         # merge point.
@@ -297,6 +303,10 @@
         The specified revision (or the current revision, if not given)
         does not have the characteristic we're looking for,
 
+    bzr bisect start -r revA..revB
+        Like: "bzr bisect start; bzr bisect yes revA; bzr bisect no revB".
+        If without revA, skip the "yes"; without revB, skip the "no".
+
     bzr bisect move -r rev
         Switch to a different revision manually.  Use if the bisect
         algorithm chooses a revision that is not suitable.  Try to
@@ -313,12 +323,14 @@
         Replay a previously-saved bisect log, forgetting any bisection
         that might be in progress.
 
-    bzr bisect run <script>
+    bzr bisect run [-r revA:revB] <script>
         Bisect automatically using <script> to determine 'yes' or 'no'.
         <script> should exit with:
            0 for yes
            125 for unknown (like build failed so we could not test)
            anything else for no
+
+        "-r revA:revB" is handled as for "start".
     """
 
     takes_args = ['subcommand', 'args*']
@@ -353,23 +365,47 @@
         """Handle the bisect command."""
 
         log_fn = None
+        # Handle args_list
+        if subcommand == 'replay':
+            if len(args_list) > 1:
+                raise BzrCommandError(
+                    "The 'bisect replay' command only accepts one argument.")
+            elif args_list:
+                log_fn = args_list[0]
+            pass
+        elif subcommand == 'run':
+            if len(args_list) != 1:
+                raise BzrCommandError(
+                    "The 'bisect run' command requires a single argument.")
+            run_script = args_list[0]
+        elif args_list:
+            raise BzrCommandError(
+                "The 'bisect %s' command doesn't take any arguments." %
+                (subcommand))
+
+        # Handle revision
         if subcommand in ('yes', 'no', 'move') and revision:
-            pass
-        elif subcommand in ('replay', ) and args_list and len(args_list) == 1:
-            log_fn = args_list[0]
-        elif subcommand in ('move', ) and not revision:
+            if len(revision) > 1:
+                raise BzrCommandError(
+                    "The 'bisect %s' command doesn't accept revision ranges." %
+                    (subcommand))
+        elif subcommand == 'move' and not revision:
             raise BzrCommandError(
                 "The 'bisect move' command requires a revision.")
-        elif subcommand in ('run', ):
-            run_script = args_list[0]
-        elif args_list or revision:
+        elif subcommand in ('start', 'run') and revision:
+            if len(revision) != 2:
+                raise BzrCommandError(
+                    "The 'bisect %s' command only accepts revision ranges." %
+                    (subcommand))
+        elif revision:
             raise BzrCommandError(
-                "Improper arguments to bisect " + subcommand)
+                "The 'bisect %s' command doesn't accept revspecs." %
+                (subcommand))
 
         # Dispatch.
 
         if subcommand == "start":
-            self.start()
+            self.start(revision)
         elif subcommand == "yes":
             self.yes(revision)
         elif subcommand == "no":
@@ -383,7 +419,7 @@
         elif subcommand == "replay":
             self.replay(log_fn)
         elif subcommand == "run":
-            self.run_bisect(run_script)
+            self.run_bisect(run_script, revision)
         else:
             raise BzrCommandError(
                 "Unknown bisect command: " + subcommand)
@@ -394,7 +430,7 @@
         BisectCurrent().reset()
         os.unlink(bisect_info_path)
 
-    def start(self):
+    def start(self, revspec):
         """Reset the bisect state, then prepare for a new bisection."""
         if os.path.exists(bisect_info_path):
             BisectCurrent().reset()
@@ -402,6 +438,13 @@
 
         bisect_log = BisectLog()
         bisect_log.set_current("start")
+
+        if revspec:
+            revA, revB = revspec
+            if revA: bisect_log.set_status_from_revspec([revA], 'yes')
+            if revB: bisect_log.set_status_from_revspec([revB], 'no')
+            bisect_log.bisect(self.outf)
+
         bisect_log.save()
 
     def yes(self, revspec):
@@ -437,15 +480,16 @@
 
         bisect_log.bisect(self.outf)
 
-    def run_bisect(self, script):
+    def run_bisect(self, script, revspec):
         import subprocess
         note("Starting bisect.")
-        self.start()
+        self.start(revspec)
         while True:
             try:
                 process = subprocess.Popen(script, shell=True)
                 process.wait()
                 retcode = process.returncode
+                mutter("Script returned %d", retcode)
                 if retcode == 0:
                     done = self._set_state(None, 'yes')
                 elif retcode == 125:
@@ -455,4 +499,4 @@
                 if done:
                     break
             except RuntimeError:
-                break
+                raise

=== modified file 'tests.py'
--- tests.py	2011-11-24 16:04:11 +0000
+++ tests.py	2012-09-21 21:50:23 +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."""
@@ -414,3 +446,8 @@
             raise KnownFailure\
                 ("bisect does not drill down into merge commits: "
                  "https://bugs.launchpad.net/bzr-bisect/+bug/539937")
+
+    def testRunRange(self):
+        out, err = self.run_bzr(['bisect', 'run', '-r', '2..5',
+                                 '! grep -q one test_file_append'])
+        self.assertRevno(3)