zuul
/
zuul
Code Issues Proposed changes

Make git repo leak check advisory in TestExecutor 

A while ago, we removed the assertion that there were no leaked
git repos because we were unable to make it race-free (potentially
due to changes in memory management in py3).  We inadvertantly
left it in place for one test: test_executor_shutdown, but recent
changes (additions of more tests, etc) have caused the error to
appear there as well.  Remove the assertion but leave the debug
log as a clue for future errors.  This makes the behavior the same
for all tests.

Also, add in a try/except handler around disabling the garbage
collector.

Change-Id: Ib503ea55fd8ddc3fda1002c7edf1a5334a0ad06f
This commit is contained in:
James E. Blair 2019-06-17 14:15:30 -07:00
parent 217dea0025
commit f8523ef9f5
2 changed files with 9 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
tests/base.py
View File

@ -2951,12 +2951,14 @@ class ZuulTestCase(BaseTestCase):
self.assertEqual({}, self.executor_server.job_workers)
# Make sure that git.Repo objects have been garbage collected.
gc.disable()
gc.collect()
for obj in gc.get_objects():
if isinstance(obj, git.Repo):
self.log.debug("Leaked git repo object: 0x%x %s" %
(id(obj), repr(obj)))
gc.enable()
try:
gc.collect()
for obj in gc.get_objects():
if isinstance(obj, git.Repo):
self.log.debug("Leaked git repo object: 0x%x %s" %
(id(obj), repr(obj)))
finally:
gc.enable()
self.assertEmptyQueues()
self.assertNodepoolState()
self.assertNoGeneratedKeys()

6
tests/unit/test_scheduler.py
View File

@ -5739,19 +5739,15 @@ class TestExecutor(ZuulTestCase):
self.assertEqual({}, self.executor_server.job_workers)
# Make sure that git.Repo objects have been garbage collected.
repos = []
gc.disable()
try:
gc.collect()
for obj in gc.get_objects():
if isinstance(obj, git.Repo):
self.log.debug("Leaked git repo object: %s" % repr(obj))
repos.append(obj)
gc.enable()
except Exception:
finally:
gc.enable()
raise
self.assertEqual(len(repos), 0)
def test_executor_shutdown(self):
"Test that the executor can shut down with jobs running"