From abd5011b610bb4a9101a6c177e91f5e6466b9e6c Mon Sep 17 00:00:00 2001
From: warner <warner@lothar.com>
Date: Tue, 29 Apr 2008 21:21:08 +0100
Subject: [PATCH] bonsaipoller: apply fixes from Ben Hearsum, closes #216

---
 ChangeLog                          |  4 ++++
 buildbot/changes/bonsaipoller.py   | 23 +++++++++++++----------
 buildbot/test/test_bonsaipoller.py | 34 ++++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index fc7dc30..3870087 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2008-04-29  Brian Warner  <warner@lothar.com>
 
+	* buildbot/changes/bonsaipoller.py: apply fixes from Ben Hearsum,
+	closes #216.
+	* buildbot/test/test_bonsaipoller.py: same
+
 	* buildbot/status/mail.py (MailNotifier): add mode="passing",
 	thanks to 'matisse' for the patch, closes #169.
 
diff --git a/buildbot/changes/bonsaipoller.py b/buildbot/changes/bonsaipoller.py
index 31cf268..2e319bb 100644
--- a/buildbot/changes/bonsaipoller.py
+++ b/buildbot/changes/bonsaipoller.py
@@ -184,12 +184,7 @@ class BonsaiParser:
         return filename
 
     def _getRevision(self):
-        """Returns the revision of the current <f> node"""
-        rev = self.currentFileNode.getAttribute("rev")
-        if rev == "":
-            raise InvalidResultError("A revision was missing from a file")
-
-        return rev
+        return self.currentFileNode.getAttribute("rev")
 
 
 class BonsaiPoller(base.ChangeSource):
@@ -224,7 +219,8 @@ class BonsaiPoller(base.ChangeSource):
         @param  cvsroot:        The cvsroot of the repository. Usually this is
                                 '/cvsroot'
         @type   pollInterval:   int
-        @param  pollInterval:   The time (in seconds) between queries for changes
+        @param  pollInterval:   The time (in seconds) between queries for
+                                changes
         """
 
         self.bonsaiURL = bonsaiURL
@@ -261,18 +257,25 @@ class BonsaiPoller(base.ChangeSource):
             self.working = True
             d = self._get_changes()
             d.addCallback(self._process_changes)
-            d.addBoth(self._finished)
+            d.addCallbacks(self._finished_ok, self._finished_failure)
         return
 
-    def _finished(self, res):
+    def _finished_ok(self, res):
         assert self.working
         self.working = False
 
-        # check for failure
+        # check for failure -- this is probably never hit but the twisted docs
+        # are not clear enough to be sure. it is being kept "just in case"
         if isinstance(res, failure.Failure):
             log.msg("Bonsai poll failed: %s" % res)
         return res
 
+    def _finished_failure(self, res):
+        log.msg("Bonsai poll failed: %s" % res)
+        assert self.working
+        self.working = False
+        return None # eat the failure
+
     def _make_url(self):
         args = ["treeid=%s" % self.tree, "module=%s" % self.module,
                 "branch=%s" % self.branch, "branchtype=match",
diff --git a/buildbot/test/test_bonsaipoller.py b/buildbot/test/test_bonsaipoller.py
index ccef77e..ca48163 100644
--- a/buildbot/test/test_bonsaipoller.py
+++ b/buildbot/test/test_bonsaipoller.py
@@ -3,6 +3,7 @@
 from twisted.trial import unittest
 from buildbot.changes.bonsaipoller import FileNode, CiNode, BonsaiResult, \
      BonsaiParser, BonsaiPoller, InvalidResultError, EmptyResult
+from buildbot.changes.changes import ChangeMaster
 
 from copy import deepcopy
 import re
@@ -73,9 +74,6 @@ badUnparsedResult = badUnparsedResult.replace("</queryResults>", "")
 invalidDateResult = deepcopy(goodUnparsedResult)
 invalidDateResult = invalidDateResult.replace(str(date1), "foobar")
 
-missingRevisionResult = deepcopy(goodUnparsedResult)
-missingRevisionResult = missingRevisionResult.replace("rev=\""+rev3+"\"", "")
-
 missingFilenameResult = deepcopy(goodUnparsedResult)
 missingFilenameResult = missingFilenameResult.replace(file2, "")
 
@@ -140,9 +138,17 @@ noCheckinMsgRef = [dict(filename="first/file.ext",
                 dict(filename="third/file.ext",
                      revision="1.3")]
 
+class FakeChangeMaster(ChangeMaster):
+    def __init__(self):
+        ChangeMaster.__init__(self)
+
+    def addChange(self, change):
+        pass
+
 class FakeBonsaiPoller(BonsaiPoller):
     def __init__(self):
         BonsaiPoller.__init__(self, "fake url", "fake module", "fake branch")
+        self.parent = FakeChangeMaster()
 
 class TestBonsaiPoller(unittest.TestCase):
     def testFullyFormedResult(self):
@@ -168,13 +174,6 @@ class TestBonsaiPoller(unittest.TestCase):
         except InvalidResultError:
             pass
 
-    def testMissingRevisionResult(self):
-        try:
-            BonsaiParser(missingRevisionResult)
-            self.fail(badResultMsgs["missingRevisionResult"])
-        except InvalidResultError:
-            pass
-
     def testMissingFilenameResult(self):
         try:
             BonsaiParser(missingFilenameResult)
@@ -211,6 +210,21 @@ class TestBonsaiPoller(unittest.TestCase):
         # self.lastChange will not be updated if the change was not submitted
         self.failUnlessEqual(lastChangeBefore, poller.lastChange)
 
+    def testParserWorksAfterInvalidResult(self):
+        """Make sure the BonsaiPoller still works after catching an
+        InvalidResultError"""
+
+        poller = FakeBonsaiPoller()
+
+        lastChangeBefore = poller.lastChange
+        # generate an exception first
+        poller._process_changes(badUnparsedResult)
+        # now give it a valid one...
+        poller._process_changes(goodUnparsedResult)
+        # if poller.lastChange has not been updated then the good result
+        # was not parsed
+        self.failIfEqual(lastChangeBefore, poller.lastChange)
+
     def testMergeEmptyLogMsg(self):
         """Ensure that BonsaiPoller works around the bonsai xml output
         issue when the check-in comment is empty"""
-- 
2.11.4.GIT