Rev 4661: Refactor creation and shutdown of test servers to use a common helper, in http://bazaar.launchpad.net/~lifeless/bzr/test-speed
Robert Collins
robertc at robertcollins.net
Thu Aug 27 23:22:13 BST 2009
At http://bazaar.launchpad.net/~lifeless/bzr/test-speed
------------------------------------------------------------
revno: 4661
revision-id: robertc at robertcollins.net-20090827221735-dck1jb1uhktda57h
parent: robertc at robertcollins.net-20090827214833-nlq6gi269u05hnf6
committer: Robert Collins <robertc at robertcollins.net>
branch nick: test-speed
timestamp: Fri 2009-08-28 08:17:35 +1000
message:
Refactor creation and shutdown of test servers to use a common helper,
reducing duplicated code, fixing a number of small race conditions where
servers may not get shut down and making tests that use such servers a
little easier to read.
=== modified file 'NEWS'
--- a/NEWS 2009-08-27 13:20:24 +0000
+++ b/NEWS 2009-08-27 22:17:35 +0000
@@ -110,6 +110,10 @@
to be parameterised. This is not expected to break external use of test
parameterisation, and is substantially faster. (Robert Collins)
+* When manually creating transport servers in test cases, a new helper
+ ``TestCase.start_server`` that registers a cleanup and starts the server
+ should be used. (Robert Collins)
+
bzr 1.18
########
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2009-08-26 23:30:48 +0000
+++ b/bzrlib/tests/__init__.py 2009-08-27 22:17:35 +0000
@@ -928,6 +928,25 @@
def _lock_broken(self, result):
self._lock_actions.append(('broken', result))
+ def start_server(self, transport_server, backing_server=None):
+ """Start transport_server for this test.
+
+ This starts the server, registers a cleanup for it and permits the
+ server's urls to be used.
+ """
+ if backing_server is None:
+ transport_server.setUp()
+ else:
+ try:
+ transport_server.setUp(backing_server)
+ except TypeError, e:
+ # This should never happen; the try:Except here is to assist
+ # developers having to update code rather than seeing an
+ # uninformative TypeError.
+ raise Exception, "Old server API in use: %s, %s" % (
+ transport_server, e)
+ self.addCleanup(transport_server.tearDown)
+
def _ndiff_strings(self, a, b):
"""Return ndiff between two strings containing lines.
@@ -2067,13 +2086,12 @@
if self.__readonly_server is None:
if self.transport_readonly_server is None:
# readonly decorator requested
- # bring up the server
self.__readonly_server = ReadonlyServer()
- self.__readonly_server.setUp(self.get_vfs_only_server())
else:
+ # explicit readonly transport.
self.__readonly_server = self.create_transport_readonly_server()
- self.__readonly_server.setUp(self.get_vfs_only_server())
- self.addCleanup(self.__readonly_server.tearDown)
+ self.start_server(self.__readonly_server,
+ self.get_vfs_only_server())
return self.__readonly_server
def get_readonly_url(self, relpath=None):
@@ -2098,8 +2116,7 @@
"""
if self.__vfs_server is None:
self.__vfs_server = MemoryServer()
- self.__vfs_server.setUp()
- self.addCleanup(self.__vfs_server.tearDown)
+ self.start_server(self.__vfs_server)
return self.__vfs_server
def get_server(self):
@@ -2112,19 +2129,13 @@
then the self.get_vfs_server is returned.
"""
if self.__server is None:
- if self.transport_server is None or self.transport_server is self.vfs_transport_factory:
- return self.get_vfs_only_server()
+ if (self.transport_server is None or self.transport_server is
+ self.vfs_transport_factory):
+ self.__server = self.get_vfs_only_server()
else:
# bring up a decorated means of access to the vfs only server.
self.__server = self.transport_server()
- try:
- self.__server.setUp(self.get_vfs_only_server())
- except TypeError, e:
- # This should never happen; the try:Except here is to assist
- # developers having to update code rather than seeing an
- # uninformative TypeError.
- raise Exception, "Old server API in use: %s, %s" % (self.__server, e)
- self.addCleanup(self.__server.tearDown)
+ self.start_server(self.__server, self.get_vfs_only_server())
return self.__server
def _adjust_url(self, base, relpath):
@@ -2263,9 +2274,8 @@
def make_smart_server(self, path):
smart_server = server.SmartTCPServer_for_testing()
- smart_server.setUp(self.get_server())
+ self.start_server(smart_server, self.get_server())
remote_transport = get_transport(smart_server.get_url()).clone(path)
- self.addCleanup(smart_server.tearDown)
return remote_transport
def make_branch_and_memory_tree(self, relpath, format=None):
@@ -2472,8 +2482,7 @@
"""
if self.__vfs_server is None:
self.__vfs_server = self.vfs_transport_factory()
- self.__vfs_server.setUp()
- self.addCleanup(self.__vfs_server.tearDown)
+ self.start_server(self.__vfs_server)
return self.__vfs_server
def make_branch_and_tree(self, relpath, format=None):
=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py 2009-08-20 04:09:58 +0000
+++ b/bzrlib/tests/blackbox/test_push.py 2009-08-27 22:17:35 +0000
@@ -576,9 +576,7 @@
def setUp(self):
tests.TestCaseWithTransport.setUp(self)
self.memory_server = RedirectingMemoryServer()
- self.memory_server.setUp()
- self.addCleanup(self.memory_server.tearDown)
-
+ self.start_server(self.memory_server)
# Make the branch and tree that we'll be pushing.
t = self.make_branch_and_tree('tree')
self.build_tree(['tree/file'])
=== modified file 'bzrlib/tests/blackbox/test_serve.py'
--- a/bzrlib/tests/blackbox/test_serve.py 2009-07-20 11:27:05 +0000
+++ b/bzrlib/tests/blackbox/test_serve.py 2009-08-27 22:17:35 +0000
@@ -209,8 +209,7 @@
ssh_server = SFTPServer(StubSSHServer)
# XXX: We *don't* want to override the default SSH vendor, so we set
# _vendor to what _get_ssh_vendor returns.
- ssh_server.setUp()
- self.addCleanup(ssh_server.tearDown)
+ self.start_server(ssh_server)
port = ssh_server._listener.port
# Access the branch via a bzr+ssh URL. The BZR_REMOTE_PATH environment
=== modified file 'bzrlib/tests/http_utils.py'
--- a/bzrlib/tests/http_utils.py 2009-05-04 14:48:21 +0000
+++ b/bzrlib/tests/http_utils.py 2009-08-27 22:17:35 +0000
@@ -133,8 +133,7 @@
"""Get the server instance for the secondary transport."""
if self.__secondary_server is None:
self.__secondary_server = self.create_transport_secondary_server()
- self.__secondary_server.setUp()
- self.addCleanup(self.__secondary_server.tearDown)
+ self.start_server(self.__secondary_server)
return self.__secondary_server
=== modified file 'bzrlib/tests/per_branch/test_push.py'
--- a/bzrlib/tests/per_branch/test_push.py 2009-08-14 00:55:42 +0000
+++ b/bzrlib/tests/per_branch/test_push.py 2009-08-27 22:17:35 +0000
@@ -394,8 +394,7 @@
# Create a smart server that publishes whatever the backing VFS server
# does.
self.smart_server = server.SmartTCPServer_for_testing()
- self.smart_server.setUp(self.get_server())
- self.addCleanup(self.smart_server.tearDown)
+ self.start_server(self.smart_server, self.get_server())
# Make two empty branches, 'empty' and 'target'.
self.empty_branch = self.make_branch('empty')
self.make_branch('target')
=== modified file 'bzrlib/tests/per_pack_repository.py'
--- a/bzrlib/tests/per_pack_repository.py 2009-08-14 00:55:42 +0000
+++ b/bzrlib/tests/per_pack_repository.py 2009-08-27 22:17:35 +0000
@@ -271,8 +271,7 @@
# failing to delete obsolete packs is not fatal
format = self.get_format()
server = fakenfs.FakeNFSServer()
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
transport = get_transport(server.get_url())
bzrdir = self.get_format().initialize_on_transport(transport)
repo = bzrdir.create_repository()
@@ -1020,8 +1019,7 @@
# Create a smart server that publishes whatever the backing VFS server
# does.
self.smart_server = server.SmartTCPServer_for_testing()
- self.smart_server.setUp(self.get_server())
- self.addCleanup(self.smart_server.tearDown)
+ self.start_server(self.smart_server, self.get_server())
# Log all HPSS calls into self.hpss_calls.
client._SmartClient.hooks.install_named_hook(
'call', self.capture_hpss_call, None)
=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- a/bzrlib/tests/per_repository/test_repository.py 2009-08-18 22:03:18 +0000
+++ b/bzrlib/tests/per_repository/test_repository.py 2009-08-27 22:17:35 +0000
@@ -823,9 +823,8 @@
be created at the given path."""
repo = self.make_repository(path, shared=shared)
smart_server = server.SmartTCPServer_for_testing()
- smart_server.setUp(self.get_server())
+ self.start_server(smart_server, self.get_server())
remote_transport = get_transport(smart_server.get_url()).clone(path)
- self.addCleanup(smart_server.tearDown)
remote_bzrdir = bzrdir.BzrDir.open_from_transport(remote_transport)
remote_repo = remote_bzrdir.open_repository()
return remote_repo
@@ -897,14 +896,6 @@
local_repo = local_bzrdir.open_repository()
self.assertEqual(remote_backing_repo._format, local_repo._format)
- # XXX: this helper probably belongs on TestCaseWithTransport
- def make_smart_server(self, path):
- smart_server = server.SmartTCPServer_for_testing()
- smart_server.setUp(self.get_server())
- remote_transport = get_transport(smart_server.get_url()).clone(path)
- self.addCleanup(smart_server.tearDown)
- return remote_transport
-
def test_clone_to_hpss(self):
pre_metadir_formats = [RepositoryFormat5(), RepositoryFormat6()]
if self.repository_format in pre_metadir_formats:
=== modified file 'bzrlib/tests/test_bundle.py'
--- a/bzrlib/tests/test_bundle.py 2009-08-04 14:10:09 +0000
+++ b/bzrlib/tests/test_bundle.py 2009-08-27 22:17:35 +0000
@@ -1830,9 +1830,8 @@
"""
from bzrlib.tests.blackbox.test_push import RedirectingMemoryServer
server = RedirectingMemoryServer()
- server.setUp()
+ self.start_server(server)
url = server.get_url() + 'infinite-loop'
- self.addCleanup(server.tearDown)
self.assertRaises(errors.NotABundle, read_mergeable_from_url, url)
def test_smart_server_connection_reset(self):
@@ -1841,8 +1840,7 @@
"""
# Instantiate a server that will provoke a ConnectionReset
sock_server = _DisconnectingTCPServer()
- sock_server.setUp()
- self.addCleanup(sock_server.tearDown)
+ self.start_server(sock_server)
# We don't really care what the url is since the server will close the
# connection without interpreting it
url = sock_server.get_url()
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2009-08-19 16:33:39 +0000
+++ b/bzrlib/tests/test_http.py 2009-08-27 22:17:35 +0000
@@ -304,7 +304,7 @@
server = http_server.HttpServer(BogusRequestHandler)
try:
- self.assertRaises(httplib.UnknownProtocol,server.setUp)
+ self.assertRaises(httplib.UnknownProtocol, server.setUp)
except:
server.tearDown()
self.fail('HTTP Server creation did not raise UnknownProtocol')
@@ -312,7 +312,7 @@
def test_force_invalid_protocol(self):
server = http_server.HttpServer(protocol_version='HTTP/0.1')
try:
- self.assertRaises(httplib.UnknownProtocol,server.setUp)
+ self.assertRaises(httplib.UnknownProtocol, server.setUp)
except:
server.tearDown()
self.fail('HTTP Server creation did not raise UnknownProtocol')
@@ -320,8 +320,10 @@
def test_server_start_and_stop(self):
server = http_server.HttpServer()
server.setUp()
- self.assertTrue(server._http_running)
- server.tearDown()
+ try:
+ self.assertTrue(server._http_running)
+ finally:
+ server.tearDown()
self.assertFalse(server._http_running)
def test_create_http_server_one_zero(self):
@@ -330,8 +332,7 @@
protocol_version = 'HTTP/1.0'
server = http_server.HttpServer(RequestHandlerOneZero)
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
self.assertIsInstance(server._httpd, http_server.TestingHTTPServer)
def test_create_http_server_one_one(self):
@@ -340,8 +341,7 @@
protocol_version = 'HTTP/1.1'
server = http_server.HttpServer(RequestHandlerOneOne)
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
self.assertIsInstance(server._httpd,
http_server.TestingThreadingHTTPServer)
@@ -352,8 +352,7 @@
server = http_server.HttpServer(RequestHandlerOneZero,
protocol_version='HTTP/1.1')
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
self.assertIsInstance(server._httpd,
http_server.TestingThreadingHTTPServer)
@@ -364,8 +363,7 @@
server = http_server.HttpServer(RequestHandlerOneOne,
protocol_version='HTTP/1.0')
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
self.assertIsInstance(server._httpd,
http_server.TestingHTTPServer)
@@ -431,8 +429,8 @@
def test_http_impl_urls(self):
"""There are servers which ask for particular clients to connect"""
server = self._server()
+ server.setUp()
try:
- server.setUp()
url = server.get_url()
self.assertTrue(url.startswith('%s://' % self._qualified_prefix))
finally:
@@ -544,8 +542,7 @@
def test_post_body_is_received(self):
server = RecordingServer(expect_body_tail='end-of-body')
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
scheme = self._qualified_prefix
url = '%s://%s:%s/' % (scheme, server.host, server.port)
http_transport = self._transport(url)
@@ -780,8 +777,7 @@
def test_send_receive_bytes(self):
server = RecordingServer(expect_body_tail='c')
- server.setUp()
- self.addCleanup(server.tearDown)
+ self.start_server(server)
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.connect((server.host, server.port))
sock.sendall('abc')
=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py 2009-08-26 01:18:13 +0000
+++ b/bzrlib/tests/test_remote.py 2009-08-27 22:17:35 +0000
@@ -1945,8 +1945,7 @@
def test_allows_new_revisions(self):
"""get_parent_map's results can be updated by commit."""
smart_server = server.SmartTCPServer_for_testing()
- smart_server.setUp()
- self.addCleanup(smart_server.tearDown)
+ self.start_server(smart_server)
self.make_branch('branch')
branch = Branch.open(smart_server.get_url() + '/branch')
tree = branch.create_checkout('tree', lightweight=True)
@@ -2781,8 +2780,7 @@
stacked_branch.set_stacked_on_url('../base')
# start a server looking at this
smart_server = server.SmartTCPServer_for_testing()
- smart_server.setUp()
- self.addCleanup(smart_server.tearDown)
+ self.start_series(smart_server)
remote_bzrdir = BzrDir.open(smart_server.get_url() + '/stacked')
# can get its branch and repository
remote_branch = remote_bzrdir.open_branch()
@@ -2943,8 +2941,7 @@
# Create a smart server that publishes whatever the backing VFS server
# does.
self.smart_server = server.SmartTCPServer_for_testing()
- self.smart_server.setUp(self.get_server())
- self.addCleanup(self.smart_server.tearDown)
+ self.start_server(self.smart_server, self.get_server())
# Log all HPSS calls into self.hpss_calls.
_SmartClient.hooks.install_named_hook(
'call', self.capture_hpss_call, None)
=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py 2009-08-17 23:15:55 +0000
+++ b/bzrlib/tests/test_smart.py 2009-08-27 22:17:35 +0000
@@ -87,8 +87,7 @@
if self._chroot_server is None:
backing_transport = tests.TestCaseWithTransport.get_transport(self)
self._chroot_server = chroot.ChrootServer(backing_transport)
- self._chroot_server.setUp()
- self.addCleanup(self._chroot_server.tearDown)
+ self.start_server(self._chroot_server)
t = get_transport(self._chroot_server.get_url())
if relpath is not None:
t = t.clone(relpath)
=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_transport.py 2009-08-27 22:17:35 +0000
@@ -363,24 +363,22 @@
def test_abspath(self):
# The abspath is always relative to the chroot_url.
server = ChrootServer(get_transport('memory:///foo/bar/'))
- server.setUp()
+ self.start_server(server)
transport = get_transport(server.get_url())
self.assertEqual(server.get_url(), transport.abspath('/'))
subdir_transport = transport.clone('subdir')
self.assertEqual(server.get_url(), subdir_transport.abspath('/'))
- server.tearDown()
def test_clone(self):
server = ChrootServer(get_transport('memory:///foo/bar/'))
- server.setUp()
+ self.start_server(server)
transport = get_transport(server.get_url())
# relpath from root and root path are the same
relpath_cloned = transport.clone('foo')
abspath_cloned = transport.clone('/foo')
self.assertEqual(server, relpath_cloned.server)
self.assertEqual(server, abspath_cloned.server)
- server.tearDown()
def test_chroot_url_preserves_chroot(self):
"""Calling get_transport on a chroot transport's base should produce a
@@ -393,12 +391,11 @@
new_transport = get_transport(parent_url)
"""
server = ChrootServer(get_transport('memory:///path/subpath'))
- server.setUp()
+ self.start_server(server)
transport = get_transport(server.get_url())
new_transport = get_transport(transport.base)
self.assertEqual(transport.server, new_transport.server)
self.assertEqual(transport.base, new_transport.base)
- server.tearDown()
def test_urljoin_preserves_chroot(self):
"""Using urlutils.join(url, '..') on a chroot URL should not produce a
@@ -410,11 +407,10 @@
new_transport = get_transport(parent_url)
"""
server = ChrootServer(get_transport('memory:///path/'))
- server.setUp()
+ self.start_server(server)
transport = get_transport(server.get_url())
self.assertRaises(
InvalidURLJoin, urlutils.join, transport.base, '..')
- server.tearDown()
class ChrootServerTest(TestCase):
@@ -428,7 +424,10 @@
backing_transport = MemoryTransport()
server = ChrootServer(backing_transport)
server.setUp()
- self.assertTrue(server.scheme in _get_protocol_handlers().keys())
+ try:
+ self.assertTrue(server.scheme in _get_protocol_handlers().keys())
+ finally:
+ server.tearDown()
def test_tearDown(self):
backing_transport = MemoryTransport()
@@ -441,8 +440,10 @@
backing_transport = MemoryTransport()
server = ChrootServer(backing_transport)
server.setUp()
- self.assertEqual('chroot-%d:///' % id(server), server.get_url())
- server.tearDown()
+ try:
+ self.assertEqual('chroot-%d:///' % id(server), server.get_url())
+ finally:
+ server.tearDown()
class ReadonlyDecoratorTransportTest(TestCase):
@@ -460,15 +461,12 @@
import bzrlib.transport.readonly as readonly
# connect to '.' via http which is not listable
server = HttpServer()
- server.setUp()
- try:
- transport = get_transport('readonly+' + server.get_url())
- self.failUnless(isinstance(transport,
- readonly.ReadonlyTransportDecorator))
- self.assertEqual(False, transport.listable())
- self.assertEqual(True, transport.is_readonly())
- finally:
- server.tearDown()
+ self.start_server(server)
+ transport = get_transport('readonly+' + server.get_url())
+ self.failUnless(isinstance(transport,
+ readonly.ReadonlyTransportDecorator))
+ self.assertEqual(False, transport.listable())
+ self.assertEqual(True, transport.is_readonly())
class FakeNFSDecoratorTests(TestCaseInTempDir):
@@ -492,31 +490,24 @@
from bzrlib.tests.http_server import HttpServer
# connect to '.' via http which is not listable
server = HttpServer()
- server.setUp()
- try:
- transport = self.get_nfs_transport(server.get_url())
- self.assertIsInstance(
- transport, bzrlib.transport.fakenfs.FakeNFSTransportDecorator)
- self.assertEqual(False, transport.listable())
- self.assertEqual(True, transport.is_readonly())
- finally:
- server.tearDown()
+ self.start_server(server)
+ transport = self.get_nfs_transport(server.get_url())
+ self.assertIsInstance(
+ transport, bzrlib.transport.fakenfs.FakeNFSTransportDecorator)
+ self.assertEqual(False, transport.listable())
+ self.assertEqual(True, transport.is_readonly())
def test_fakenfs_server_default(self):
# a FakeNFSServer() should bring up a local relpath server for itself
import bzrlib.transport.fakenfs as fakenfs
server = fakenfs.FakeNFSServer()
- server.setUp()
- try:
- # the url should be decorated appropriately
- self.assertStartsWith(server.get_url(), 'fakenfs+')
- # and we should be able to get a transport for it
- transport = get_transport(server.get_url())
- # which must be a FakeNFSTransportDecorator instance.
- self.assertIsInstance(
- transport, fakenfs.FakeNFSTransportDecorator)
- finally:
- server.tearDown()
+ self.start_server(server)
+ # the url should be decorated appropriately
+ self.assertStartsWith(server.get_url(), 'fakenfs+')
+ # and we should be able to get a transport for it
+ transport = get_transport(server.get_url())
+ # which must be a FakeNFSTransportDecorator instance.
+ self.assertIsInstance(transport, fakenfs.FakeNFSTransportDecorator)
def test_fakenfs_rename_semantics(self):
# a FakeNFS transport must mangle the way rename errors occur to
@@ -587,8 +578,7 @@
def setUp(self):
super(TestTransportImplementation, self).setUp()
self._server = self.transport_server()
- self._server.setUp()
- self.addCleanup(self._server.tearDown)
+ self.start_server(self._server)
def get_transport(self, relpath=None):
"""Return a connected transport to the local directory.
More information about the bazaar-commits
mailing list