Skip to content

Commit

Permalink
Merge pull request buildbot#8254 from p12tic/tests-remove-teardown
Browse files Browse the repository at this point in the history
Prefer TestCase.addCleanup() to the same functionality in tearDown()
  • Loading branch information
p12tic authored Dec 8, 2024
2 parents 1606946 + 440f2a1 commit b1b830c
Show file tree
Hide file tree
Showing 246 changed files with 606 additions and 2,243 deletions.
3 changes: 0 additions & 3 deletions master/buildbot/test/integration/test_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ def setUp(self):
self.basedir = os.path.abspath('basedir')
self.filename = os.path.abspath("test.cfg")

def tearDown(self):
self.tearDownDirs()

def test_sample_config(self):
filename = util.sibpath(runner.__file__, 'sample.cfg')
with assertNotProducesWarnings(DeprecatedApiWarning):
Expand Down
5 changes: 1 addition & 4 deletions master/buildbot/test/integration/test_graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,10 @@ def setUp(self):
self.master.allSchedulers = lambda: scheds

yield self.master.startService()
self.addCleanup(self.master.stopService)

yield self.insert_initial_data()

@defer.inlineCallbacks
def tearDown(self):
yield self.master.stopService()

@defer.inlineCallbacks
def insert_initial_data(self):
yield self.master.db.insert_test_data([
Expand Down
15 changes: 8 additions & 7 deletions master/buildbot/test/integration/test_telegram_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,14 @@ def send_message(chat, message, **kwargs):

tb.bot.send_message = send_message

@defer.inlineCallbacks
def tearDown(self):
self.tearDownDirs()
if self.master:
yield self.master.www.stopService()
yield self.master.mq.stopService()
yield self.master.test_shutdown()
@defer.inlineCallbacks
def cleanup():
if self.master:
yield self.master.www.stopService()
yield self.master.mq.stopService()
yield self.master.test_shutdown()

self.addCleanup(cleanup)

@defer.inlineCallbacks
def testWebhook(self):
Expand Down
11 changes: 2 additions & 9 deletions master/buildbot/test/integration/test_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,14 @@ def setUpUpgradeTest(self):
)

self._sql_log_handler = querylog.start_log_queries()

def tearDownUpgradeTest(self):
querylog.stop_log_queries(self._sql_log_handler)
self.addCleanup(lambda: querylog.stop_log_queries(self._sql_log_handler))

# save subclasses the trouble of calling our setUp and tearDown methods

def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
return self.setUpUpgradeTest()

@defer.inlineCallbacks
def tearDown(self):
yield self.tearDownUpgradeTest()
yield self.tear_down_test_reactor()

@defer.inlineCallbacks
def assertModelMatches(self):
def comp(engine):
Expand Down
12 changes: 4 additions & 8 deletions master/buildbot/test/integration/test_www.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,19 @@ def setUp(self):

self.master = master

self.addCleanup(self.master.test_shutdown)
self.addCleanup(self.master.www.stopService)

# build an HTTP agent, using an explicit connection pool if Twisted
# supports it (Twisted 13.0.0 and up)
if hasattr(client, 'HTTPConnectionPool'):
self.pool = client.HTTPConnectionPool(reactor)
self.agent = client.Agent(reactor, pool=self.pool)
self.addCleanup(self.pool.closeCachedConnections)
else:
self.pool = None
self.agent = client.Agent(reactor)

@defer.inlineCallbacks
def tearDown(self):
if self.pool:
yield self.pool.closeCachedConnections()
if self.master:
yield self.master.www.stopService()
yield self.master.test_shutdown()

@defer.inlineCallbacks
def apiGet(self, url, expect200=True):
pg = yield self.agent.request(b'GET', url)
Expand Down
41 changes: 21 additions & 20 deletions master/buildbot/test/integration/worker/test_comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class TestWorkerComm(unittest.TestCase, TestReactorMixin):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.master = yield fakemaster.make_master(self, wantMq=True, wantData=True, wantDb=True)

# set the worker port to a loopback address with unspecified
Expand Down Expand Up @@ -195,25 +195,26 @@ 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
if self.endpoint:
del self.endpoint
deferreds = [
*self._detach_deferreds,
self.pbmanager.stopService(),
self.botmaster.stopService(),
self.workers.stopService(),
]

# if the worker is still attached, wait for it to detach, too
if self.buildworker and self.buildworker.detach_d:
deferreds.append(self.buildworker.detach_d)

yield defer.gatherResults(deferreds, consumeErrors=True)
yield self.tear_down_test_reactor()
@defer.inlineCallbacks
def cleanup():
if self.broker:
del self.broker
if self.endpoint:
del self.endpoint
deferreds = [
*self._detach_deferreds,
self.pbmanager.stopService(),
self.botmaster.stopService(),
self.workers.stopService(),
]

# if the worker is still attached, wait for it to detach, too
if self.buildworker and self.buildworker.detach_d:
deferreds.append(self.buildworker.detach_d)

yield defer.gatherResults(deferreds, consumeErrors=True)

self.addCleanup(cleanup)

@defer.inlineCallbacks
def addWorker(self, **kwargs):
Expand Down
28 changes: 15 additions & 13 deletions master/buildbot/test/integration/worker/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,19 +147,21 @@ def setUp(self):
self.target_port = self.queue.get()
write_to_log(f"got target_port {self.target_port}\n")

def tearDown(self):
write_to_log("tearDown\n")
self.proxy_process.terminate()
self.proxy_process.join()
if self.enable_debug:
print("---- stdout ----")
with open(get_log_path(), encoding='utf-8') as file:
print(file.read())
print("---- ------ ----")
with open(self.queue.get(), encoding='utf-8') as file:
print(file.read())
print("---- ------ ----")
os.unlink(get_log_path())
def cleanup():
write_to_log("cleanup\n")
self.proxy_process.terminate()
self.proxy_process.join()
if self.enable_debug:
print("---- stdout ----")
with open(get_log_path(), encoding='utf-8') as file:
print(file.read())
print("---- ------ ----")
with open(self.queue.get(), encoding='utf-8') as file:
print(file.read())
print("---- ------ ----")
os.unlink(get_log_path())

self.addCleanup(cleanup)

@defer.inlineCallbacks
def setup_master(self, config_dict, startWorker=True):
Expand Down
29 changes: 15 additions & 14 deletions master/buildbot/test/integration/worker/test_workerside.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class TestWorkerConnection(unittest.TestCase, TestReactorMixin):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.master = yield fakemaster.make_master(self, wantMq=True, wantData=True, wantDb=True)
# set the worker port to a loopback address with unspecified
# port
Expand Down Expand Up @@ -142,19 +142,20 @@ def setUp(self):

self.tmpdirs = set()

@defer.inlineCallbacks
def tearDown(self):
for tmp in self.tmpdirs:
if os.path.exists(tmp):
shutil.rmtree(tmp)
yield self.pbmanager.stopService()
yield self.botmaster.stopService()
yield self.workers.stopService()

# 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 cleanup():
for tmp in self.tmpdirs:
if os.path.exists(tmp):
shutil.rmtree(tmp)
yield self.pbmanager.stopService()
yield self.botmaster.stopService()
yield self.workers.stopService()

# if the worker is still attached, wait for it to detach, too
if self.buildworker:
yield self.buildworker.waitForCompleteShutdown()

self.addCleanup(cleanup)

@defer.inlineCallbacks
def addMasterSideWorker(
Expand Down
4 changes: 4 additions & 0 deletions master/buildbot/test/reactor.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from buildbot.test.fake.reactor import TestReactor
from buildbot.util import twisted
from buildbot.util.eventual import _setReactor
from buildbot.warnings import warn_deprecated


class TestReactorMixin:
Expand All @@ -32,6 +33,9 @@ class TestReactorMixin:
"""

def setup_test_reactor(self, use_asyncio=False, auto_tear_down=True):
if not auto_tear_down:
warn_deprecated('4.2.0', 'auto_tear_down=False is deprecated')

self.patch(threadpool, 'ThreadPool', NonThreadPool)
self.patch(twisted, 'ThreadPool', NonThreadPool)
self.reactor = TestReactor()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@ class TestBadRows(TestReactorMixin, unittest.TestCase):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.master = yield fakemaster.make_master(self, wantDb=True)
self.db = self.master.db

@defer.inlineCallbacks
def tearDown(self):
yield self.tear_down_test_reactor()

@defer.inlineCallbacks
def test_bogus_row_no_source(self):
yield self.db.insert_test_data([
Expand Down
8 changes: 6 additions & 2 deletions master/buildbot/test/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,8 +704,12 @@ def setup_test_build_step(
self._exp_result_summaries: list[str] = []
self._exp_build_result_summaries: list[str] = []

def tear_down_test_build_step(self):
pass
def tear_down_test_build_step(self): # pragma: no cover
warn_deprecated(
'4.2.0',
'tear_down_test_build_step() no longer needs to be called, '
+ 'test tear down is run automatically',
)

def _setup_fake_build(self, worker_version, worker_env, build_files):
if worker_version is None:
Expand Down
14 changes: 2 additions & 12 deletions master/buildbot/test/unit/changes/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,9 @@ class Subclass(base.ChangeSource):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
yield self.setUpChangeSource()

@defer.inlineCallbacks
def tearDown(self):
yield self.tearDownChangeSource()
yield self.tear_down_test_reactor()

@defer.inlineCallbacks
def test_activation(self):
cs = self.Subclass(name="DummyCS")
Expand Down Expand Up @@ -84,17 +79,12 @@ class Subclass(base.ReconfigurablePollingChangeSource):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()

yield self.setUpChangeSource()

yield self.attachChangeSource(self.Subclass(name="DummyCS"))

@defer.inlineCallbacks
def tearDown(self):
yield self.tearDownChangeSource()
yield self.tear_down_test_reactor()

@defer.inlineCallbacks
def runClockFor(self, secs):
yield self.reactor.pump([0] + [1.0] * secs)
Expand Down
7 changes: 1 addition & 6 deletions master/buildbot/test/unit/changes/test_bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ class TestBitbucketPullrequestPoller(
changesource.ChangeSourceMixin, TestReactorMixin, LoggingMixin, unittest.TestCase
):
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.setUpLogging()

# create pull requests
Expand Down Expand Up @@ -304,11 +304,6 @@ def setUp(self):

return self.setUpChangeSource()

@defer.inlineCallbacks
def tearDown(self):
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
# and return result
Expand Down
6 changes: 1 addition & 5 deletions master/buildbot/test/unit/changes/test_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Change(unittest.TestCase, TestReactorMixin):

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.master = yield fakemaster.make_master(self, wantDb=True)
self.change23 = changes.Change(**{ # using **dict(..) forces kwargs
"category": 'devel',
Expand Down Expand Up @@ -107,10 +107,6 @@ 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
Expand Down
21 changes: 5 additions & 16 deletions master/buildbot/test/unit/changes/test_gerritchangesource.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,12 @@ class TestGerritChangeSource(
TestReactorMixin,
unittest.TestCase,
):
@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
self.setup_master_run_process()
self._got_events = []
return self.setUpChangeSource()

@defer.inlineCallbacks
def tearDown(self):
if self.master.running:
yield self.stopChangeSource()
yield self.tearDownChangeSource()
yield self.tear_down_test_reactor()
yield self.setUpChangeSource()

@defer.inlineCallbacks
def create_gerrit(self, host, user, *args, **kwargs):
Expand Down Expand Up @@ -1052,15 +1046,10 @@ class TestGerritEventLogPoller(

@defer.inlineCallbacks
def setUp(self):
self.setup_test_reactor(auto_tear_down=False)
self.setup_test_reactor()
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()
self.addCleanup(self.master.stopService)

@defer.inlineCallbacks
def newChangeSource(self, **kwargs):
Expand Down
Loading

0 comments on commit b1b830c

Please sign in to comment.