Merge lp:~gthorslund/bzr-bisect/539937-subtree into lp:bzr-bisect

[Gustaf Thorslund]

Added three test cases. Two of them will still fail due to:
  https://bugs.launchpad.net/bzr-bisect/+bug/539937
marked them as KnownFailure.

[Jelmer Vernooij (jelmer)]

Thanks! It's nice to have some tests for these bugs, that should make them a lot easier to fix.

For consistency, can you change the syntax of the docstrings to use """ rather than " ?

The script tests probably won't work on Windows at the moment. I wonder if this is an issue for bzr-bisect (did it work on Windows before?). Ideally we should make these tests work on Windows as well, though for the moment it is probably sufficient to check sys.platform and raise TestSkipped if sys.platform is "win32".

review: Needs Fixing (code)

[lp:~gthorslund/bzr-bisect/539937-subtree]

81. By Gustaf Thorslund on 2010-11-05

Fixed " -> """ for docstrings. Added some docstrings. Also converted some #-comment into a docstring.

[Jelmer Vernooij (jelmer)]

review: Approve (code)

=== modified file '__init__.py'
--- __init__.py	2010-10-16 17:54:57 +0000
+++ __init__.py	2010-11-05 12:09:47 +0000
@@ -14,7 +14,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-"Support for git-style bisection."
+"""Support for git-style bisection."""
 
 import sys
 import os
@@ -22,7 +22,7 @@
 from bzrlib.commands import Command, register_command
 from bzrlib.errors import BzrCommandError
 from bzrlib.option import Option
-from bzrlib.trace import info
+from bzrlib.trace import note
 
 from meta import *
 
@@ -31,7 +31,7 @@
 
 
 class BisectCurrent(object):
-    "Bisect class for managing the current revision."
+    """Bisect class for managing the current revision."""
 
     def __init__(self, filename = bisect_rev_path):
         self._filename = filename
@@ -45,23 +45,23 @@
             self._revid = self._bzrbranch.last_revision()
 
     def _save(self):
-        "Save the current revision."
+        """Save the current revision."""
 
         revid_file = open(self._filename, "w")
         revid_file.write(self._revid + "\n")
         revid_file.close()
 
     def get_current_revid(self):
-        "Return the current revision id."
+        """Return the current revision id."""
         return self._revid
 
     def get_current_revno(self):
-        "Return the current revision number as a tuple."
+        """Return the current revision number as a tuple."""
         revdict = self._bzrbranch.get_revision_id_to_revno_map()
         return revdict[self.get_current_revid()]
 
     def get_parent_revids(self):
-        "Return the IDs of the current revision's predecessors."
+        """Return the IDs of the current revision's predecessors."""
         repo = self._bzrbranch.repository
         repo.lock_read()
         retval = repo.get_parent_map([self._revid]).get(self._revid, None)
@@ -69,18 +69,18 @@
         return retval
 
     def is_merge_point(self):
-        "Is the current revision a merge point?"
+        """Is the current revision a merge point?"""
         return len(self.get_parent_revids()) > 1
 
     def show_rev_log(self, out = sys.stdout):
-        "Write the current revision's log entry to a file."
+        """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()])
         out.write("On revision %s (%s):\n%s\n" % (revno, rev.revision_id,
                                                   rev.message))
 
     def switch(self, revid):
-        "Switch the current revision to the given revid."
+        """Switch the current revision to the given revid."""
         working = self._bzrdir.open_workingtree()
         if isinstance(revid, int):
             revid = self._bzrbranch.get_rev_id(revid)
@@ -92,7 +92,7 @@
         self._save()
 
     def reset(self):
-        "Revert bisection, setting the working tree to normal."
+        """Revert bisection, setting the working tree to normal."""
         working = self._bzrdir.open_workingtree()
         last_rev = working.branch.last_revision()
         rev_tree = working.branch.repository.revision_tree(last_rev)
@@ -102,7 +102,7 @@
 
 
 class BisectLog(object):
-    "Bisect log file handler."
+    """Bisect log file handler."""
 
     def __init__(self, filename = bisect_info_path):
         self._items = []
@@ -115,27 +115,27 @@
         self.load()
 
     def _open_for_read(self):
-        "Open log file for reading."
+        """Open log file for reading."""
         if self._filename:
             return open(self._filename)
         else:
             return sys.stdin
 
     def _open_for_write(self):
-        "Open log file for writing."
+        """Open log file for writing."""
         if self._filename:
             return open(self._filename, "w")
         else:
             return sys.stdout
 
     def _load_bzr_tree(self):
-        "Load bzr information."
+        """Load bzr information."""
         if not self._bzrdir:
             self._bzrdir = bzrlib.bzrdir.BzrDir.open_containing('.')[0]
             self._bzrbranch = self._bzrdir.open_branch()
 
     def _find_range_and_middle(self, branch_last_rev = None):
-        "Find the current revision range, and the midpoint."
+        """Find the current revision range, and the midpoint."""
         self._load_bzr_tree()
         self._middle_revid = None
 
@@ -193,12 +193,12 @@
         self._low_revid = low_revid
 
     def _switch_wc_to_revno(self, revno, outf):
-        "Move the working tree to the given revno."
+        """Move the working tree to the given revno."""
         self._current.switch(revno)
         self._current.show_rev_log(out=outf)
 
     def _set_status(self, revid, status):
-        "Set the bisect status for the given revid."
+        """Set the bisect status for the given revid."""
         if not self.is_done():
             if status != "done" and revid in [x[0] for x in self._items 
                                               if x[1] in ['yes', 'no']]:
@@ -206,11 +206,11 @@
             self._items.append((revid, status))
 
     def change_file_name(self, filename):
-        "Switch log files."
+        """Switch log files."""
         self._filename = filename
 
     def load(self):
-        "Load the bisection log."
+        """Load the bisection log."""
         self._items = []
         if os.path.exists(self._filename):
             revlog = self._open_for_read()
@@ -219,23 +219,23 @@
                 self._items.append((revid, status))
 
     def save(self):
-        "Save the bisection log."
+        """Save the bisection log."""
         revlog = self._open_for_write()
         for (revid, status) in self._items:
             revlog.write("%s %s\n" % (revid, status))
 
     def is_done(self):
-        "Report whether we've found the right revision."
+        """Report whether we've found the right revision."""
         return len(self._items) > 0 and self._items[-1][1] == "done"
 
     def set_status_from_revspec(self, revspec, status):
-        "Set the bisection status for the revision in revspec."
+        """Set the bisection status for the revision in revspec."""
         self._load_bzr_tree()
         revid = revspec[0].in_history(self._bzrbranch).rev_id
         self._set_status(revid, status)
 
     def set_current(self, status):
-        "Set the current revision to the given bisection status."
+        """Set the current revision to the given bisection status."""
         self._set_status(self._current.get_current_revid(), status)
 
     def is_merge_point(self, revid):
@@ -251,7 +251,7 @@
         return retval
 
     def bisect(self, outf):
-        "Using the current revision's status, do a bisection."
+        """Using the current revision's status, do a bisection."""
         self._find_range_and_middle()
         # If we've found the "final" revision, check for a
         # merge point.
@@ -322,7 +322,7 @@
                      'revision']
 
     def _check(self):
-        "Check preconditions for most operations to work."
+        """Check preconditions for most operations to work."""
         if not os.path.exists(bisect_info_path):
             raise BzrCommandError("No bisection in progress.")
 
@@ -332,7 +332,7 @@
         Returns boolean indicating if bisection is done."""
         bisect_log = BisectLog()
         if bisect_log.is_done():
-            info("No further bisection is possible.\n")
+            note("No further bisection is possible.\n")
             bisect_log._current.show_rev_log(self.outf)
             return True
 
@@ -345,7 +345,7 @@
         return False
 
     def run(self, subcommand, args_list, revision=None, output=None):
-        "Handle the bisect command."
+        """Handle the bisect command."""
 
         log_fn = None
         if subcommand in ('yes', 'no', 'move') and revision:
@@ -384,13 +384,13 @@
                 "Unknown bisect command: " + subcommand)
 
     def reset(self):
-        "Reset the bisect state to no state."
+        """Reset the bisect state to no state."""
         self._check()
         BisectCurrent().reset()
         os.unlink(bisect_info_path)
 
     def start(self):
-        "Reset the bisect state, then prepare for a new bisection."
+        """Reset the bisect state, then prepare for a new bisection."""
         if os.path.exists(bisect_info_path):
             BisectCurrent().reset()
             os.unlink(bisect_info_path)
@@ -400,21 +400,21 @@
         bisect_log.save()
 
     def yes(self, revspec):
-        "Mark that a given revision has the state we're looking for."
+        """Mark that a given revision has the state we're looking for."""
         self._set_state(revspec, "yes")
 
     def no(self, revspec):
-        "Mark that a given revision does not have the state we're looking for."
+        """Mark that a given revision does not have the state we're looking for."""
         self._set_state(revspec, "no")
 
     def move(self, revspec):
-        "Move to a different revision manually."
+        """Move to a different revision manually."""
         current = BisectCurrent()
         current.switch(revspec)
         current.show_rev_log(out=self.outf)
 
     def log(self, filename):
-        "Write the current bisect log to a file."
+        """Write the current bisect log to a file."""
         self._check()
         bisect_log = BisectLog()
         bisect_log.change_file_name(filename)
@@ -434,7 +434,7 @@
 
     def run_bisect(self, script):
         import subprocess
-        info("Starting bisect.")
+        note("Starting bisect.")
         self.start()
         while True:
             try:

=== modified file 'meta.py'
--- meta.py	2009-04-02 15:11:50 +0000
+++ meta.py	2010-11-05 12:09:47 +0000
@@ -14,7 +14,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-# Queryable plugin variables, from a proposal by Robert Collins.
+"""Queryable plugin variables, from a proposal by Robert Collins."""
 
 bzr_plugin_name = 'bisect'
 

=== modified file 'tests.py'
--- tests.py	2010-10-16 17:54:57 +0000
+++ tests.py	2010-11-05 12:09:47 +0000
@@ -17,6 +17,7 @@
 "Test suite for the bzr bisect plugin."
 
 import os
+import stat
 from cStringIO import StringIO
 import shutil
 import bzrlib
@@ -24,13 +25,13 @@
 import bzrlib.tests
 import bzrlib.revisionspec
 import bzrlib.plugins.bisect as bisect
-
+from bzrlib.tests import KnownFailure
 
 class BisectTestCase(bzrlib.tests.TestCaseWithTransport):
-    "Test harness specific to the bisect plugin."
+    """Test harness specific to the bisect plugin."""
 
     def assertRevno(self, rev):
-        "Make sure we're at the right revision."
+        """Make sure we're at the right revision."""
 
         rev_contents = {1: "one", 1.1: "one dot one", 1.2: "one dot two",
                         1.3: "one dot three", 2: "two", 3: "three",
@@ -45,12 +46,15 @@
                                  % (rev, found_rev))
 
     def setUp(self):
-        bzrlib.tests.TestCaseWithTransport.setUp(self)
+        """Set up tests."""
 
         # These tests assume a branch with five revisions, and
         # a branch from version 1 containing three revisions
         # merged at version 2.
 
+        bzrlib.tests.TestCaseWithTransport.setUp(self)
+
+
         self.tree = self.make_branch_and_tree(".")
 
         test_file = open("test_file", "w")
@@ -58,7 +62,12 @@
         test_file.close()
         self.tree.add(self.tree.relpath(os.path.join(os.getcwd(),
                                                      'test_file')))
-        self.tree.commit(message = "add test file")
+        test_file_append = open("test_file_append", "a")
+        test_file_append.write("one\n")
+        test_file_append.close()
+        self.tree.add(self.tree.relpath(os.path.join(os.getcwd(),
+                                                     'test_file_append')))
+        self.tree.commit(message = "add test files")
 
         bzrlib.bzrdir.BzrDir.open(".").sprout("../temp-clone")
         clone_bzrdir = bzrlib.bzrdir.BzrDir.open("../temp-clone")
@@ -67,6 +76,9 @@
             test_file = open("../temp-clone/test_file", "w")
             test_file.write(content)
             test_file.close()
+            test_file_append = open("../temp-clone/test_file_append", "a")
+            test_file_append.write(content + "\n")
+            test_file_append.close()
             clone_tree.commit(message = "make branch test change")
             saved_subtree_revid = clone_tree.branch.last_revision()
 
@@ -74,6 +86,9 @@
         test_file = open("test_file", "w")
         test_file.write("two")
         test_file.close()
+        test_file_append = open("test_file_append", "a")
+        test_file_append.write("two\n")
+        test_file_append.close()
         self.tree.commit(message = "merge external branch")
         shutil.rmtree("../temp-clone")
 
@@ -84,14 +99,17 @@
             test_file = open("test_file", "w")
             test_file.write(content)
             test_file.close()
+            test_file_append = open("test_file_append", "a")
+            test_file_append.write(content + "\n")
+            test_file_append.close()
             self.tree.commit(message = "make test change")
 
 
 class BisectHarnessTests(BisectTestCase):
-    "Tests for the harness itself."
+    """Tests for the harness itself."""
 
     def testLastRev(self):
-        "Test that the last revision is correct."
+        """Test that the last revision is correct."""
         repo = self.tree.branch.repository
         top_revtree = repo.revision_tree(self.tree.last_revision())
         top_revtree.lock_read()
@@ -102,7 +120,7 @@
         assert test_content == "five"
 
     def testSubtreeRev(self):
-        "Test that the last revision in a subtree is correct."
+        """Test that the last revision in a subtree is correct."""
         repo = self.tree.branch.repository
         sub_revtree = repo.revision_tree(self.subtree_rev)
         sub_revtree.lock_read()
@@ -114,42 +132,44 @@
 
 
 class BisectMetaTests(BisectTestCase):
-    "Test the metadata provided by the package."
+    """Test the metadata provided by the package."""
 
     def testVersionPresent(self):
+        """Check if version_info is available for the plugin."""
         assert bisect.version_info
 
     def testBzrVersioning(self):
+        """Check if API is of right version."""
         assert bisect.bzr_minimum_api <= bzrlib.api_minimum_version
         assert bisect.bzr_minimum_api <= bzrlib.version_info[:3]
 
 
 class BisectCurrentUnitTests(BisectTestCase):
-    "Test the BisectCurrent class."
+    """Test the BisectCurrent class."""
 
     def testShowLog(self):
-        "Test that the log can be shown."
+        """Test that the log can be shown."""
         # Not a very good test; just makes sure the code doesn't fail,
         # not that the output makes any sense.
         sio = StringIO()
         bisect.BisectCurrent().show_rev_log(out=sio)
 
     def testShowLogSubtree(self):
-        "Test that a subtree's log can be shown."
+        """Test that a subtree's log can be shown."""
         current = bisect.BisectCurrent()
         current.switch(self.subtree_rev)
         sio = StringIO()
         current.show_rev_log(out=sio)
 
     def testSwitchVersions(self):
-        "Test switching versions."
+        """Test switching versions."""
         current = bisect.BisectCurrent()
         self.assertRevno(5)
         current.switch(4)
         self.assertRevno(4)
 
     def testReset(self):
-        "Test resetting the working tree to a non-bisected state."
+        """Test resetting the working tree to a non-bisected state."""
         current = bisect.BisectCurrent()
         current.switch(4)
         current.reset()
@@ -157,7 +177,7 @@
         assert not os.path.exists(bisect.bisect_rev_path)
 
     def testIsMergePoint(self):
-        "Test merge point detection."
+        """Test merge point detection."""
         current = bisect.BisectCurrent()
         self.assertRevno(5)
         assert not current.is_merge_point()
@@ -166,16 +186,16 @@
 
 
 class BisectLogUnitTests(BisectTestCase):
-    "Test the BisectLog class."
+    """Test the BisectLog class."""
 
     def testCreateBlank(self):
-        "Test creation of new log."
+        """Test creation of new log."""
         bisect_log = bisect.BisectLog()
         bisect_log.save()
         assert os.path.exists(bisect.bisect_info_path)
 
     def testLoad(self):
-        "Test loading a log."
+        """Test loading a log."""
         preloaded_log = open(bisect.bisect_info_path, "w")
         preloaded_log.write("rev1 yes\nrev2 no\nrev3 yes\n")
         preloaded_log.close()
@@ -187,7 +207,7 @@
         assert bisect_log._items[2] == ("rev3", "yes")
 
     def testSave(self):
-        "Test saving the log."
+        """Test saving the log."""
         bisect_log = bisect.BisectLog()
         bisect_log._items = [("rev1", "yes"), ("rev2", "no"), ("rev3", "yes")]
         bisect_log.save()
@@ -197,10 +217,10 @@
 
 
 class BisectFuncTests(BisectTestCase):
-    "Functional tests for the bisect plugin."
+    """Functional tests for the bisect plugin."""
 
     def testWorkflow(self):
-        "Run through a basic usage scenario."
+        """Run through a basic usage scenario."""
 
         # Start up the bisection.  When the two ends are set, we should
         # end up in the middle.
@@ -266,7 +286,7 @@
         self.assertRevno(1.1)
 
     def testMove(self):
-        "Test manually moving to a different revision during the bisection."
+        """Test manually moving to a different revision during the bisection."""
 
         # Set up a bisection in progress.
 
@@ -280,7 +300,7 @@
         self.assertRevno(2)
 
     def testReset(self):
-        "Test resetting the tree."
+        """Test resetting the tree."""
 
         # Set up a bisection in progress.
 
@@ -310,7 +330,7 @@
         self.failUnless(content == "keep me")
 
     def testLog(self):
-        "Test saving the current bisection state, and re-loading it."
+        """Test saving the current bisection state, and re-loading it."""
 
         # Set up a bisection in progress.
 
@@ -337,3 +357,52 @@
 
         self.run_bzr(['bisect', 'no'])
         self.assertRevno(3)
+
+    def testRunScript(self):
+        """Make a test script and run it."""
+        test_script = open("test_script", "w")
+        test_script.write("#!/bin/sh\n"
+                          "grep -q '^four' test_file_append\n")
+        test_script.close()
+        os.chmod("test_script", stat.S_IRWXU)
+        self.run_bzr(['bisect', 'start'])
+        self.run_bzr(['bisect', 'yes'])
+        self.run_bzr(['bisect', 'no', '-r', '1'])
+        self.run_bzr(['bisect', 'run', './test_script'])
+        self.assertRevno(4)
+
+    def testRunScriptMergePoint(self):
+        """Make a test script and run it."""
+        test_script = open("test_script", "w")
+        test_script.write("#!/bin/sh\n"
+                          "grep -q '^two' test_file_append\n")
+        test_script.close()
+        os.chmod("test_script", stat.S_IRWXU)
+        self.run_bzr(['bisect', 'start'])
+        self.run_bzr(['bisect', 'yes'])
+        self.run_bzr(['bisect', 'no', '-r', '1'])
+        self.run_bzr(['bisect', 'run', './test_script'])
+        try:
+            self.assertRevno(2)
+        except AssertionError:
+            raise KnownFailure\
+                ("bisect does not drill down into merge commits: "
+                 "https://bugs.launchpad.net/bzr-bisect/+bug/539937")
+        
+    def testRunScriptSubtree(self):
+        """Make a test script and run it."""
+        test_script = open("test_script", "w")
+        test_script.write("#!/bin/sh\n"
+                          "grep -q '^one dot two' test_file_append\n")
+        test_script.close()
+        os.chmod("test_script", stat.S_IRWXU)
+        self.run_bzr(['bisect', 'start'])
+        self.run_bzr(['bisect', 'yes'])
+        self.run_bzr(['bisect', 'no', '-r', '1'])
+        self.run_bzr(['bisect', 'run', './test_script'])
+        try:
+            self.assertRevno(1.2)
+        except AssertionError:
+            raise KnownFailure\
+                ("bisect does not drill down into merge commits: "
+                 "https://bugs.launchpad.net/bzr-bisect/+bug/539937")