Merge pull request buildbot#8150 from p12tic/test-reactor-explicit-te…

…ar-down

Tear down TestReactorMixin explicitly, not using addCleanup()

master/buildbot/test/integration/test_upgrade.py

@@ -111,11 +111,13 @@ def tearDownUpgradeTest(self):
     # save subclasses the trouble of calling our setUp and tearDown methods
 
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         return self.setUpUpgradeTest()
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.tearDownUpgradeTest()
+        yield self.tearDownUpgradeTest()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def assertModelMatches(self):

master/buildbot/test/integration/worker/test_comm.py

@@ -161,7 +161,7 @@ class TestWorkerComm(unittest.TestCase, TestReactorMixin):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.master = yield fakemaster.make_master(self, wantMq=True, wantData=True, wantDb=True)
 
         # set the worker port to a loopback address with unspecified
@@ -195,6 +195,7 @@ def setUp(self):
         self.server_connection_string = "tcp:0:interface=127.0.0.1"
         self.client_connection_string_tpl = "tcp:host=127.0.0.1:port={port}"
 
+    @defer.inlineCallbacks
     def tearDown(self):
         if self.broker:
             del self.broker
@@ -211,7 +212,8 @@ def tearDown(self):
         if self.buildworker and self.buildworker.detach_d:
             deferreds.append(self.buildworker.detach_d)
 
-        return defer.gatherResults(deferreds)
+        yield defer.gatherResults(deferreds)
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def addWorker(self, **kwargs):

master/buildbot/test/integration/worker/test_workerside.py

@@ -111,7 +111,7 @@ class TestWorkerConnection(unittest.TestCase, TestReactorMixin):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.master = yield fakemaster.make_master(self, wantMq=True, wantData=True, wantDb=True)
         # set the worker port to a loopback address with unspecified
         # port
@@ -154,6 +154,7 @@ def tearDown(self):
         # if the worker is still attached, wait for it to detach, too
         if self.buildworker:
             yield self.buildworker.waitForCompleteShutdown()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def addMasterSideWorker(

master/buildbot/test/reactor.py

@@ -15,6 +15,7 @@
 
 import asyncio
 
+from twisted.internet import defer
 from twisted.internet import threads
 from twisted.python import threadpool
 
@@ -30,7 +31,7 @@ class TestReactorMixin:
     at the end
     """
 
-    def setup_test_reactor(self, use_asyncio=False):
+    def setup_test_reactor(self, use_asyncio=False, auto_tear_down=True):
         self.patch(threadpool, 'ThreadPool', NonThreadPool)
         self.reactor = TestReactor()
         self.reactor.set_test_case(self)
@@ -44,21 +45,32 @@ def deferToThread(f, *args, **kwargs):
 
         self.patch(threads, 'deferToThread', deferToThread)
 
-        # During shutdown sequence we must first stop the reactor and only then
-        # set unset the reactor used for eventually() because any callbacks
-        # that are run during reactor.stop() may use eventually() themselves.
-        self.addCleanup(_setReactor, None)
-        self.addCleanup(self.reactor.assert_no_remaining_calls)
-        self.addCleanup(self.reactor.stop)
-
+        self._reactor_use_asyncio = use_asyncio
         if use_asyncio:
             self.asyncio_loop = AsyncIOLoopWithTwisted(self.reactor)
             asyncio.set_event_loop(self.asyncio_loop)
             self.asyncio_loop.start()
 
-            def stop():
-                self.asyncio_loop.stop()
-                self.asyncio_loop.close()
-                asyncio.set_event_loop(None)
+        if auto_tear_down:
+            self.addCleanup(self.tear_down_test_reactor)
+        self._reactor_tear_down_called = False
+
+    def tear_down_test_reactor(self):
+        if self._reactor_tear_down_called:
+            return
+
+        self._reactor_tear_down_called = True
+
+        if self._reactor_use_asyncio:
+            self.asyncio_loop.stop()
+            self.asyncio_loop.close()
+            asyncio.set_event_loop(None)
+
+        # During shutdown sequence we must first stop the reactor and only then set unset the
+        # reactor used for eventually() because any callbacks that are run during reactor.stop()
+        # may use eventually() themselves.
+        self.reactor.stop()
+        self.reactor.assert_no_remaining_calls()
+        _setReactor(None)
 
-            self.addCleanup(stop)
+        return defer.succeed(None)

master/buildbot/test/unit/changes/test_base.py

@@ -32,11 +32,13 @@ class Subclass(base.ChangeSource):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         yield self.setUpChangeSource()
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.tearDownChangeSource()
+        yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def test_activation(self):
@@ -82,14 +84,16 @@ class Subclass(base.ReconfigurablePollingChangeSource):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
 
         yield self.setUpChangeSource()
 
         yield self.attachChangeSource(self.Subclass(name="DummyCS"))
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.tearDownChangeSource()
+        yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def runClockFor(self, secs):

master/buildbot/test/unit/changes/test_bitbucket.py

@@ -274,7 +274,7 @@ class TestBitbucketPullrequestPoller(
     changesource.ChangeSourceMixin, TestReactorMixin, LoggingMixin, unittest.TestCase
 ):
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setUpLogging()
 
         # create pull requests
@@ -304,8 +304,10 @@ def setUp(self):
 
         return self.setUpChangeSource()
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.tearDownChangeSource()
+        yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     def _fakeGetPage(self, result):
         # Install a fake getPage that puts the requested URL in self.getPage_got_url

master/buildbot/test/unit/changes/test_changes.py

@@ -52,7 +52,7 @@ class Change(unittest.TestCase, TestReactorMixin):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.master = yield fakemaster.make_master(self, wantDb=True)
         self.change23 = changes.Change(**{  # using **dict(..) forces kwargs
             "category": 'devel',
@@ -105,6 +105,10 @@ def setUp(self):
         })
         self.change25.number = 25
 
+    @defer.inlineCallbacks
+    def tearDown(self):
+        yield self.tear_down_test_reactor()
+
     @defer.inlineCallbacks
     def test_fromChdict(self):
         # get a real honest-to-goodness chdict from the fake db

master/buildbot/test/unit/changes/test_gerritchangesource.py

@@ -98,7 +98,7 @@ class TestGerritChangeSource(
     unittest.TestCase,
 ):
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setup_master_run_process()
         self._got_events = []
         return self.setUpChangeSource()
@@ -108,6 +108,7 @@ def tearDown(self):
         if self.master.running:
             yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def create_gerrit(self, host, user, *args, **kwargs):
@@ -1046,14 +1047,15 @@ class TestGerritEventLogPoller(
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         yield self.setUpChangeSource()
         yield self.master.startService()
 
     @defer.inlineCallbacks
     def tearDown(self):
         yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def newChangeSource(self, **kwargs):

master/buildbot/test/unit/changes/test_github.py

@@ -180,7 +180,7 @@ class TestGitHubPullrequestPoller(
 ):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         yield self.setUpChangeSource()
 
         fake_storage_service = FakeSecretStorage()
@@ -197,6 +197,7 @@ def setUp(self):
     def tearDown(self):
         yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def newChangeSource(self, owner, repo, endpoint='https://api.github.com', **kwargs):

master/buildbot/test/unit/changes/test_gitpoller.py

@@ -66,7 +66,7 @@ def createPoller(self):
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setup_master_run_process()
         yield self.setUpChangeSource()
         yield self.master.startService()
@@ -77,6 +77,7 @@ def setUp(self):
     def tearDown(self):
         yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @async_to_deferred
     async def set_last_rev(self, state: dict[str, str]) -> None:
@@ -2433,14 +2434,15 @@ class TestGitPollerConstructor(
 ):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         yield self.setUpChangeSource()
         yield self.master.startService()
 
     @defer.inlineCallbacks
     def tearDown(self):
         yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def test_deprecatedFetchRefspec(self):

master/buildbot/test/unit/changes/test_hgpoller.py

@@ -39,7 +39,7 @@ class TestHgPollerBase(
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setup_master_run_process()
         yield self.setUpChangeSource()
 
@@ -71,6 +71,7 @@ def _isRepositoryReady():
     def tearDown(self):
         yield self.master.stopService()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     @defer.inlineCallbacks
     def check_current_rev(self, wished, branch='default'):

master/buildbot/test/unit/changes/test_mail.py

@@ -29,7 +29,7 @@ class TestMaildirSource(
 ):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.maildir = os.path.abspath("maildir")
 
         yield self.setUpChangeSource()
@@ -56,6 +56,7 @@ def assertMailProcessed(self):
     def tearDown(self):
         yield self.tearDownDirs()
         yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     # tests
 

master/buildbot/test/unit/changes/test_manager.py

@@ -28,15 +28,17 @@
 class TestChangeManager(unittest.TestCase, TestReactorMixin):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.master = yield fakemaster.make_master(self, wantData=True)
         self.cm = manager.ChangeManager()
         self.master.startService()
         yield self.cm.setServiceParent(self.master)
         self.new_config = mock.Mock()
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.master.stopService()
+        yield self.master.stopService()
+        yield self.tear_down_test_reactor()
 
     def make_sources(self, n, klass=base.ChangeSource, **kwargs):
         for i in range(n):

master/buildbot/test/unit/changes/test_p4poller.py

@@ -109,12 +109,14 @@ class TestP4Poller(
 ):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setup_master_run_process()
         yield self.setUpChangeSource()
 
+    @defer.inlineCallbacks
     def tearDown(self):
-        return self.tearDownChangeSource()
+        yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
 
     def add_p4_describe_result(self, number, result):
         self.expect_commands(

master/buildbot/test/unit/changes/test_pb.py

@@ -40,12 +40,17 @@ class TestPBChangeSource(
 
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.setUpPBChangeSource()
         yield self.setUpChangeSource()
 
         self.master.pbmanager = self.pbmanager
 
+    @defer.inlineCallbacks
+    def tearDown(self):
+        yield self.tearDownChangeSource()
+        yield self.tear_down_test_reactor()
+
     def test_registration_no_workerport(self):
         return self._test_registration(None, exp_ConfigErrors=True, user='alice', passwd='sekrit')
 
@@ -212,9 +217,13 @@ def test_reconfigService_default_changed_but_inactive(self):
 class TestChangePerspective(TestReactorMixin, unittest.TestCase):
     @defer.inlineCallbacks
     def setUp(self):
-        self.setup_test_reactor()
+        self.setup_test_reactor(auto_tear_down=False)
         self.master = yield fakemaster.make_master(self, wantDb=True, wantData=True)
 
+        @defer.inlineCallbacks
+        def tearDown(self):
+            yield self.tear_down_test_reactor()
+
     @defer.inlineCallbacks
     def test_addChange_noprefix(self):
         cp = pb.ChangePerspective(self.master, None)