Merge djmitche/buildbot:bug2442 (PR #1149)

Conflicts:
	master/buildbot/util/__init__.py

+ pyflakes
+ fix docs

master/buildbot/changes/base.py

@@ -14,13 +14,11 @@
 # Copyright Buildbot Team Members
 
 from twisted.internet import defer
-from twisted.internet import reactor
-from twisted.internet import task
 from twisted.python import log
 from zope.interface import implements
 
 from buildbot.interfaces import IChangeSource
-from buildbot.util import misc
+from buildbot.util import poll
 from buildbot.util import service
 
 
@@ -54,64 +52,25 @@ def _unclaimService(self):
 
 class PollingChangeSource(ChangeSource):
 
-    """
-    Utility subclass for ChangeSources that use some kind of periodic polling
-    operation.  Subclasses should define C{poll} and set C{self.pollInterval}.
-    The rest is taken care of.
-
-    Any subclass will be available via the "poller" webhook.
-    """
-
-    pollInterval = 60
-    "time (in seconds) between calls to C{poll}"
-
-    pollAtLaunch = False
-    "determines when the first poll occurs. True = immediately on launch, False = wait for one pollInterval."
-
-    _loop = None
-
     def __init__(self, name=None, pollInterval=60 * 10, pollAtLaunch=False):
         ChangeSource.__init__(self, name)
         self.pollInterval = pollInterval
         self.pollAtLaunch = pollAtLaunch
 
-        self.doPoll = misc.SerializedInvocation(self.doPoll)
-
+    @poll.method
     def doPoll(self):
-        """
-        This is the method that is called by LoopingCall to actually poll.
-        It may also be called by change hooks to request a poll.
-        It is serialiazed - if you call it while a poll is in progress
-        then the 2nd invocation won't start until the 1st has finished.
-        """
         d = defer.maybeDeferred(self.poll)
         d.addErrback(log.err, 'while polling for changes')
         return d
 
     def poll(self):
-        """
-        Perform the polling operation, and return a deferred that will fire
-        when the operation is complete.  Failures will be logged, but the
-        method will be called again after C{pollInterval} seconds.
-        """
-
-    def startLoop(self):
-        self._loop = task.LoopingCall(self.doPoll)
-        self._loop.start(self.pollInterval, now=self.pollAtLaunch)
+        pass
 
-    def stopLoop(self):
-        if self._loop and self._loop.running:
-            self._loop.stop()
-            self._loop = None
+    def force(self):
+        self.doPoll()
 
     def activate(self):
-        # delay starting doing anything until the reactor is running - if
-        # services are still starting up, they may miss an initial flood of
-        # changes
-        if self.pollInterval:
-            reactor.callWhenRunning(self.startLoop)
-        else:
-            reactor.callWhenRunning(self.doPoll)
+        self.doPoll.start(interval=self.pollInterval, now=self.pollAtLaunch)
 
     def deactivate(self):
-        self.stopLoop()
+        return self.doPoll.stop()

master/buildbot/status/web/hooks/poller.py

@@ -48,6 +48,6 @@ def getChanges(req, options=None):
             raise ValueError("Could not find pollers: %s" % ",".join(missing))
 
     for p in pollers:
-        p.doPoll()
+        p.force()
 
     return [], None

master/buildbot/test/unit/test_status_web_hooks_poller.py

@@ -15,6 +15,7 @@
 
 import buildbot.status.web.change_hook as change_hook
 
+from buildbot import util
 from buildbot.changes import base
 from buildbot.changes.manager import ChangeManager
 from buildbot.test.fake.web import FakeRequest
@@ -31,7 +32,8 @@ class Subclass(base.PollingChangeSource):
         def poll(self):
             self.called = True
 
-    def setUpRequest(self, args, options=True):
+    @defer.inlineCallbacks
+    def setUpRequest(self, args, options=True, activate=True):
         self.changeHook = change_hook.ChangeHookResource(dialects={'poller': options})
 
         self.request = FakeRequest(args=args)
@@ -41,24 +43,42 @@ def setUpRequest(self, args, options=True):
         master = self.request.site.buildbot_service.master
         master.change_svc = ChangeManager(master)
 
-        self.changesrc = self.Subclass("example", None)
+        self.changesrc = self.Subclass("example", 21)
         self.changesrc.setServiceParent(master.change_svc)
+        if activate:
+            self.changesrc.activate()
 
-        self.disabledChangesrc = self.Subclass("disabled", None)
-        self.disabledChangesrc.setServiceParent(master.change_svc)
+        self.otherpoller = self.Subclass("otherpoller", 22)
+        self.otherpoller.setServiceParent(master.change_svc)
+        if activate:
+            self.otherpoller.activate()
 
         anotherchangesrc = base.ChangeSource(name='notapoller')
         anotherchangesrc.setName("notapoller")
         anotherchangesrc.setServiceParent(master.change_svc)
 
-        return self.request.test_render(self.changeHook)
+        yield self.request.test_render(self.changeHook)
+        yield util.asyncSleep(0)
+
+    def tearDown(self):
+        return defer.gatherResults([
+            self.changesrc.deactivate(),
+            self.otherpoller.deactivate(),
+        ])
 
     @defer.inlineCallbacks
     def test_no_args(self):
         yield self.setUpRequest({})
         self.assertEqual(self.request.written, "no changes found")
         self.assertEqual(self.changesrc.called, True)
-        self.assertEqual(self.disabledChangesrc.called, True)
+        self.assertEqual(self.otherpoller.called, True)
+
+    @defer.inlineCallbacks
+    def test_not_active(self):
+        yield self.setUpRequest({}, activate=False)
+        self.assertEqual(self.request.written, "no changes found")
+        self.assertEqual(self.changesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_no_poller(self):
@@ -67,7 +87,7 @@ def test_no_poller(self):
         self.assertEqual(self.request.written, expected)
         self.request.setResponseCode.assert_called_with(400, expected)
         self.assertEqual(self.changesrc.called, False)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_invalid_poller(self):
@@ -76,34 +96,34 @@ def test_invalid_poller(self):
         self.assertEqual(self.request.written, expected)
         self.request.setResponseCode.assert_called_with(400, expected)
         self.assertEqual(self.changesrc.called, False)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_trigger_poll(self):
         yield self.setUpRequest({"poller": ["example"]})
         self.assertEqual(self.request.written, "no changes found")
         self.assertEqual(self.changesrc.called, True)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_allowlist_deny(self):
-        yield self.setUpRequest({"poller": ["disabled"]}, options={"allowed": ["example"]})
-        expected = "Could not find pollers: disabled"
+        yield self.setUpRequest({"poller": ["otherpoller"]}, options={"allowed": ["example"]})
+        expected = "Could not find pollers: otherpoller"
         self.assertEqual(self.request.written, expected)
         self.request.setResponseCode.assert_called_with(400, expected)
         self.assertEqual(self.changesrc.called, False)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_allowlist_allow(self):
         yield self.setUpRequest({"poller": ["example"]}, options={"allowed": ["example"]})
         self.assertEqual(self.request.written, "no changes found")
         self.assertEqual(self.changesrc.called, True)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)
 
     @defer.inlineCallbacks
     def test_allowlist_all(self):
         yield self.setUpRequest({}, options={"allowed": ["example"]})
         self.assertEqual(self.request.written, "no changes found")
         self.assertEqual(self.changesrc.called, True)
-        self.assertEqual(self.disabledChangesrc.called, False)
+        self.assertEqual(self.otherpoller.called, False)

master/buildbot/test/unit/test_util_misc.py

@@ -14,13 +14,9 @@
 # Copyright Buildbot Team Members
 
 from buildbot import util
-from buildbot.test.util import compat
 from buildbot.util import misc
-from buildbot.util.eventual import eventually
 from twisted.internet import defer
-from twisted.internet import reactor
 from twisted.internet import task
-from twisted.python import failure
 from twisted.trial import unittest
 
 
@@ -91,62 +87,6 @@ def check_unlocked(_):
         return d
 
 
-class SerializedInvocation(unittest.TestCase):
-
-    def waitForQuiet(self, si):
-        d = defer.Deferred()
-        si._quiet = lambda: d.callback(None)
-        return d
-
-    # tests
-
-    def test_name(self):
-        self.assertEqual(util.SerializedInvocation, misc.SerializedInvocation)
-
-    def testCallFolding(self):
-        events = []
-
-        def testfn():
-            d = defer.Deferred()
-
-            def done():
-                events.append('TM')
-                d.callback(None)
-            eventually(done)
-            return d
-        si = misc.SerializedInvocation(testfn)
-
-        # run three times - the first starts testfn, the second
-        # requires a second run, and the third is folded.
-        d1 = si()
-        d2 = si()
-        d3 = si()
-
-        dq = self.waitForQuiet(si)
-        d = defer.gatherResults([d1, d2, d3, dq])
-
-        def check(_):
-            self.assertEqual(events, ['TM', 'TM'])
-        d.addCallback(check)
-        return d
-
-    @compat.usesFlushLoggedErrors
-    def testException(self):
-        def testfn():
-            d = defer.Deferred()
-            reactor.callLater(0, d.errback,
-                              failure.Failure(RuntimeError("oh noes")))
-            return d
-        si = misc.SerializedInvocation(testfn)
-
-        d = si()
-
-        def check(_):
-            self.assertEqual(len(self.flushLoggedErrors(RuntimeError)), 1)
-        d.addCallback(check)
-        return d
-
-
 class TestCancelAfter(unittest.TestCase):
 
     def setUp(self):