From 3c3b3fc10b08bc549bbc449e750f833dbaf10493 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Tue, 17 Sep 2013 16:28:20 +0200 Subject: [PATCH] [1.5.x] Final attempt to solve sporadic test failures. tearDownClass is not called if setUpClass throws an exception, in our case this means that LiveServerTestCase leaks LiveServerThread sockets if the test happens to be skipped later on, and AdminSeleniumWebDriverTestCase doesn't close it's already open browser window. To prevent this leakage we catch errors where needed and manually call _tearDownClassInternal. _tearDownClassInternal should be written as defensively as possible since it is not allowed to make any assumptions on how far setUpClass got. This patch should fix the sporadic "Address already in use"-errors on jenkins and also the "This code isn't under transaction management"-error for sqlite (also just on jenkins). After discussion with koniiiik, jezdez, kmtracey, tos9, lifeless, nedbat and voidspace it was decided that this is the safest approach (thanks to everyone for their comments and help). Manually calling tearDownClass was shut down cause we don't know how our users override our classes. This is a private and very specialized API on purpose and should not be used without a strong reason! This patch partially reverts the earlier attempts to fix those issues, namely: 2fa0dd73b18f55d0fdd1c1d54b1d18031bfcf1ed and 3c5775d36f7e431d9691829a78580873111cb714 Final note: If this patch breaks in a later version of Django, please be very careful on how you fix it, you might not see test failures locally. That said, this patch hopefully doesn't produce even more failures. Backport of 73a610d2a81bc3bf2d3834786b2458bc85953ed0 from master. --- django/contrib/admin/tests.py | 5 +++-- django/test/testcases.py | 8 +++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/django/contrib/admin/tests.py b/django/contrib/admin/tests.py index 78e68e79c2..f529faefce 100644 --- a/django/contrib/admin/tests.py +++ b/django/contrib/admin/tests.py @@ -17,13 +17,14 @@ class AdminSeleniumWebDriverTestCase(LiveServerTestCase): except Exception as e: raise SkipTest('Selenium webdriver "%s" not installed or not ' 'operational: %s' % (cls.webdriver_class, str(e))) + # This has to be last to ensure that resources are cleaned up properly! super(AdminSeleniumWebDriverTestCase, cls).setUpClass() @classmethod - def tearDownClass(cls): + def _tearDownClassInternal(cls): if hasattr(cls, 'selenium'): cls.selenium.quit() - super(AdminSeleniumWebDriverTestCase, cls).tearDownClass() + super(AdminSeleniumWebDriverTestCase, cls)._tearDownClassInternal() def wait_until(self, callback, timeout=10): """ diff --git a/django/test/testcases.py b/django/test/testcases.py index e556d18225..0941fa09d1 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -1157,12 +1157,15 @@ class LiveServerTestCase(TransactionTestCase): # Wait for the live server to be ready cls.server_thread.is_ready.wait() if cls.server_thread.error: + # Clean up behind ourselves, since tearDownClass won't get called in + # case of errors. + cls._tearDownClassInternal() raise cls.server_thread.error super(LiveServerTestCase, cls).setUpClass() @classmethod - def tearDownClass(cls): + def _tearDownClassInternal(cls): # There may not be a 'server_thread' attribute if setUpClass() for some # reasons has raised an exception. if hasattr(cls, 'server_thread'): @@ -1175,4 +1178,7 @@ class LiveServerTestCase(TransactionTestCase): and conn.settings_dict['NAME'] == ':memory:'): conn.allow_thread_sharing = False + @classmethod + def tearDownClass(cls): + cls._tearDownClassInternal() super(LiveServerTestCase, cls).tearDownClass()