Merge lp:~mandel/ubuntuone-dev-tools/domain-sockets into lp:ubuntuone-dev-tools

[Manuel de la Peña]

- Added support for servers that use domain sockets as their transport (LP: #988257).

- Added support for servers that use domain sockets as their transport (LP: #988257).

You will see that the tests are ran twice, once using tcp sockets and the second one using unix domain sockets. The use of endpoints has forced me to change the API because endpoints already returns the on connection made deferred. I'll be working on updating all the projects to ensure that this change is taken into account.

[Alejandro J. Cura (alecu)]

Nice branch; code looks mostly fine, but here are some fixes I'm requesting:

---

I don't understand this comment:
        # do not point to a dir because the path will be too long

---

A few typos:
TCPNoConnetionTrackingTestCase -> TCPNoConnectionTrackingTestCase
UnixNoConnetionTrackingTestCase -> UnixNoConnectionTrackingTestCase
"Unix domian sockets" -> "Unix domain sockets"
"perse" -> "per se"
"client_endpoint_patter" -> "client_endpoint_pattern"

---

We don't need this docstring copied everywhere; having it only on the parent class is enough:
""" This test class is not testing the server and client perse but testing
    that we can use a PbServerFactory and PbClientFactory in a way that the
    connection will be closed correctly.
"""

---

Why are the UNIX client and server endpoint strings different?

    client_endpoint_patter = 'unix:path=%s'
    server_endpoint_pattern = 'unix:%s'

---

review: Needs Fixing

[Alejandro J. Cura (alecu)]

Looks good

review: Approve

[Brian Curtin (brian.curtin)]

Fine on Windows. Approved.

review: Approve

=== renamed file 'ubuntuone/devtools/testcases/tests/test_txtcpserver.py' => 'ubuntuone/devtools/testcases/tests/test_txsocketserver.py'
--- ubuntuone/devtools/testcases/tests/test_txtcpserver.py	2012-04-13 16:43:08 +0000
+++ ubuntuone/devtools/testcases/tests/test_txsocketserver.py	2012-04-26 14:30:44 +0000
@@ -32,11 +32,14 @@
 from twisted.spread import pb
 from twisted.trial.unittest import TestCase
 
-from ubuntuone.devtools.testcases.txtcpserver import (
+from ubuntuone.devtools.testcases import skipIfOS
+from ubuntuone.devtools.testcases.txsocketserver import (
     client_protocol_factory,
     server_protocol_factory,
-    PbServerTestCase,
+    ServerTestCase,
+    TCPPbServerTestCase,
     TidyTCPServer,
+    TidyUnixServer,
     TCPServerTestCase,
 )
 
@@ -185,8 +188,6 @@
         protocol_instance.connectionMade()
         self.assertIn('connectionMade', self.called,
                                         'Super connectionMade must be called')
-        self.assertTrue(
-               protocol_instance.factory.testserver_on_connection_made.called)
 
     # factory outside init
     def test_connection_made_called(self):
@@ -206,10 +207,10 @@
 # pylint: enable=E1101
 
 
-class TestPlainTwistedTestCase(TCPServerTestCase):
+class TCPPlainTwistedTestCase(ServerTestCase):
     """Test using a server.
 
-    This test class is not testing the server and client perse but testing
+    This test class is not testing the server and client per se but testing
     that we can use a PbServerFactory and PbClientFactory in a way that the
     connection will be closed correctly.
     """
@@ -217,12 +218,15 @@
     @defer.inlineCallbacks
     def setUp(self):
         """Set the diff tests."""
-        yield super(TestPlainTwistedTestCase, self).setUp()
+        yield super(TCPPlainTwistedTestCase, self).setUp()
         self.adder = Adder()
         self.calculator = Calculator(self.adder)
-        self.listen_server(pb.PBServerFactory, self.calculator)
-        self.connect_client(pb.PBClientFactory)
-        yield self.client_connected
+        yield self.listen_server(pb.PBServerFactory, self.calculator)
+        yield self.connect_client(pb.PBClientFactory)
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyTCPServer()
 
     @defer.inlineCallbacks
     def test_addition(self):
@@ -243,7 +247,16 @@
         self.assertTrue(check)
 
 
-class TestNoConnetionTrackingTestCase(TCPServerTestCase):
+@skipIfOS('win32', 'Unix domain sockets not supported on windows.')
+class UnixPlainTwistedTestCase(TCPPlainTwistedTestCase):
+    """Test using a server over domain sockets."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyUnixServer()
+
+
+class TCPNoConnectionTrackingTestCase(TCPServerTestCase):
     """Test using a server.
 
     This test class is not testing the server and the client perse but testing
@@ -255,15 +268,12 @@
     @defer.inlineCallbacks
     def setUp(self):
         """Set the diff tests."""
-        yield super(TestNoConnetionTrackingTestCase, self).setUp()
+        yield super(TCPNoConnectionTrackingTestCase, self).setUp()
         self.adder = Adder()
         self.calculator = Calculator(self.adder)
         # connect client and server
-        self.listen_server(pb.PBServerFactory, self.calculator)
-        self.connect_client(pb.PBClientFactory)
-
-        # ensure we are connected
-        yield self.client_connected
+        yield self.listen_server(pb.PBServerFactory, self.calculator)
+        yield self.connect_client(pb.PBClientFactory)
 
         self.first_number = 1
         self.second_number = 2
@@ -301,7 +311,17 @@
         self.assertTrue(check)
 
 
-class TestPlainPbTestCase(PbServerTestCase):
+@skipIfOS('win32', 'Unix domain sockets not supported on windows.')
+class UnixNoConnectionTrackingTestCase(TCPNoConnectionTrackingTestCase):
+    """Test using a server over domain sockets."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        # do not point to a dir because the path will be too long
+        return TidyUnixServer()
+
+
+class TCPPlainPbTestCase(TCPPbServerTestCase):
     """Test using a server.
 
     This test class is not testing the server and client perse but testing
@@ -312,12 +332,11 @@
     @defer.inlineCallbacks
     def setUp(self):
         """Set the diff tests."""
-        yield super(TestPlainPbTestCase, self).setUp()
+        yield super(TCPPlainPbTestCase, self).setUp()
         self.adder = Adder()
         self.calculator = Calculator(self.adder)
-        self.listen_server(self.calculator)
-        self.connect_client()
-        yield self.client_connected
+        yield self.listen_server(self.calculator)
+        yield self.connect_client()
 
     @defer.inlineCallbacks
     def test_addition(self):
@@ -338,7 +357,16 @@
         self.assertTrue(check)
 
 
-class MultipleServersTestCase(TestCase):
+@skipIfOS('win32', 'Unix domain sockets not supported on windows.')
+class UnixPlainPbTestCase(TCPPlainPbTestCase):
+    """Test using a server over domain sockets."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyUnixServer()
+
+
+class TCPMultipleServersTestCase(TestCase):
     """Ensure that several servers can be ran."""
 
     timeout = 2
@@ -346,24 +374,27 @@
     @defer.inlineCallbacks
     def setUp(self):
         """Set the diff tests."""
-        yield super(MultipleServersTestCase, self).setUp()
-        self.first_tcp_server = TidyTCPServer()
-        self.second_tcp_server = TidyTCPServer()
+        yield super(TCPMultipleServersTestCase, self).setUp()
+        self.first_tcp_server = self.get_server()
+        self.second_tcp_server = self.get_server()
         self.adder = Adder()
         self.calculator = Calculator(self.adder)
         self.echoer = Echoer()
 
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyTCPServer()
+
     @defer.inlineCallbacks
     def test_single_server(self):
         """Test setting a single server."""
         first_number = 1
         second_number = 2
-        self.first_tcp_server.listen_server(pb.PBServerFactory,
+        yield self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                               self.calculator)
         self.addCleanup(self.first_tcp_server.clean_up)
-        calculator_c = self.first_tcp_server.connect_client(pb.PBClientFactory)
-        # ensure we do have connected
-        yield calculator_c.testserver_on_connection_made
+        calculator_c = yield self.first_tcp_server.connect_client(
+                                                           pb.PBClientFactory)
         calculator = yield calculator_c.getRootObject()
         adder = yield calculator.callRemote('get_adder')
         result = yield adder.callRemote('add', first_number, second_number)
@@ -375,20 +406,20 @@
         first_number = 1
         second_number = 2
         # first server
-        self.first_tcp_server.listen_server(pb.PBServerFactory,
+        yield self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                               self.calculator)
         self.addCleanup(self.first_tcp_server.clean_up)
 
         # second server
-        self.second_tcp_server.listen_server(pb.PBServerFactory, self.echoer)
+        yield self.second_tcp_server.listen_server(pb.PBServerFactory,
+                                                   self.echoer)
         self.addCleanup(self.second_tcp_server.clean_up)
 
         # connect the diff clients
-        calculator_c = self.first_tcp_server.connect_client(pb.PBClientFactory)
-        echoer_c = self.second_tcp_server.connect_client(pb.PBClientFactory)
-        # ensure we do have connected
-        yield calculator_c.testserver_on_connection_made
-        yield echoer_c.testserver_on_connection_made
+        calculator_c = yield self.first_tcp_server.connect_client(
+                                                           pb.PBClientFactory)
+        echoer_c = yield self.second_tcp_server.connect_client(
+                                                           pb.PBClientFactory)
 
         calculator = yield calculator_c.getRootObject()
         adder = yield calculator.callRemote('get_adder')
@@ -398,20 +429,31 @@
         echo = yield echoer.callRemote('say', 'hello')
         self.assertEqual(self.echoer.remote_say('hello'), echo)
 
+    @defer.inlineCallbacks
     def test_no_single_client(self):
         """Test setting a single server no client."""
         # start server but do not connect a client
-        self.first_tcp_server.listen_server(pb.PBServerFactory,
+        yield self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                               self.calculator)
         self.addCleanup(self.first_tcp_server.clean_up)
 
+    @defer.inlineCallbacks
     def test_no_multiple_clients(self):
         """Test setting multiple servers no clients."""
         # first server
-        self.first_tcp_server.listen_server(pb.PBServerFactory,
+        yield self.first_tcp_server.listen_server(pb.PBServerFactory,
                                                               self.calculator)
         self.addCleanup(self.first_tcp_server.clean_up)
 
         # second server
         self.second_tcp_server.listen_server(pb.PBServerFactory, self.echoer)
         self.addCleanup(self.second_tcp_server.clean_up)
+
+
+@skipIfOS('win32', 'Unix domain sockets not supported on windows.')
+class UnixMultipleServersTestCase(TCPMultipleServersTestCase):
+    """Ensure that several servers can be ran."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyUnixServer()

=== renamed file 'ubuntuone/devtools/testcases/txtcpserver.py' => 'ubuntuone/devtools/testcases/txsocketserver.py'
--- ubuntuone/devtools/testcases/txtcpserver.py	2012-04-13 16:45:15 +0000
+++ ubuntuone/devtools/testcases/txsocketserver.py	2012-04-26 14:30:44 +0000
@@ -28,7 +28,11 @@
 
 """Base test case for twisted servers."""
 
-from twisted.internet import defer, protocol
+import os
+import shutil
+import tempfile
+
+from twisted.internet import defer, endpoints, protocol
 from twisted.spread import pb
 
 from ubuntuone.devtools.testcases import BaseTestCase
@@ -69,13 +73,6 @@
     class ClientTidyProtocol(cls):
         """A tidy protocol."""
 
-        def connectionMade(self):
-            """Connection made."""
-            if (self.factory.testserver_on_connection_made is not None
-                    and not self.factory.testserver_on_connection_made.called):
-                self.factory.testserver_on_connection_made.callback(self)
-            cls.connectionMade(self)
-
         def connectionLost(self, *a):
             """Connection list."""
             # pylint: disable=W0212
@@ -89,7 +86,7 @@
     return ClientTidyProtocol
 
 
-class TidyTCPServer(object):
+class TidySocketServer(object):
     """Ensure that twisted servers are correctly managed in tests.
 
     Closing a twisted server is a complicated matter. In order to do so you
@@ -103,7 +100,6 @@
     the reactor is left clean by following the pattern described at
     http://mumak.net/stuff/twisted-disconnect.html
     """
-
     def __init__(self):
         """Create a new instance."""
         self.listener = None
@@ -112,6 +108,15 @@
         self.connector = None
         self.client_factory = None
 
+    def get_server_endpoint(self):
+        """Return the server endpoint description."""
+        raise NotImplementedError('To be implemented by child classes.')
+
+    def get_client_endpoint(self):
+        """Return the client endpoint description."""
+        raise NotImplementedError('To be implemented by child classes.')
+
+    @defer.inlineCallbacks
     def listen_server(self, server_class, *args, **kwargs):
         """Start a server in a random port."""
         from twisted.internet import reactor
@@ -120,9 +125,12 @@
         self.server_factory.testserver_on_connection_lost = defer.Deferred()
         self.server_factory.protocol = server_protocol_factory(
                                                  self.server_factory.protocol)
-        self.listener = reactor.listenTCP(0, self.server_factory)
-        return self.server_factory
+        endpoint = endpoints.serverFromString(reactor,
+                                              self.get_server_endpoint())
+        self.listener = yield endpoint.listen(self.server_factory)
+        defer.returnValue(self.server_factory)
 
+    @defer.inlineCallbacks
     def connect_client(self, client_class, *args, **kwargs):
         """Conect a client to a given server."""
         from twisted.internet import reactor
@@ -131,18 +139,17 @@
             raise ValueError('Server Factory was not provided.')
         if self.listener is None:
             raise ValueError('%s has not started listening.',
-                                                          self.server_factory)
+                             self.server_factory)
 
         self.client_factory = client_class(*args, **kwargs)
         self.client_factory._disconnecting = False
         self.client_factory.protocol = client_protocol_factory(
                                                  self.client_factory.protocol)
-        self.client_factory.testserver_on_connection_made = defer.Deferred()
         self.client_factory.testserver_on_connection_lost = defer.Deferred()
-        self.connector = reactor.connectTCP('localhost',
-                                        self.listener.getHost().port,
-                                        self.client_factory)
-        return self.client_factory
+        endpoint = endpoints.clientFromString(reactor,
+                                                    self.get_client_endpoint())
+        self.connector = yield endpoint.connect(self.client_factory)
+        defer.returnValue(self.client_factory)
 
     def clean_up(self):
         """Action to be performed for clean up."""
@@ -154,8 +161,8 @@
             # clean client and server
             self.server_factory._disconnecting = True
             self.client_factory._disconnecting = True
+            self.connector.transport.loseConnection()
             d = defer.maybeDeferred(self.listener.stopListening)
-            self.connector.disconnect()
             return defer.gatherResults([d,
                 self.client_factory.testserver_on_connection_lost,
                 self.server_factory.testserver_on_connection_lost])
@@ -165,14 +172,66 @@
             return defer.maybeDeferred(self.listener.stopListening)
 
 
-class TCPServerTestCase(BaseTestCase):
-    """Test that uses a single twisted server."""
+class TidyTCPServer(TidySocketServer):
+    """A tidy tcp domain sockets server."""
+
+    client_endpoint_pattern = 'tcp:host=127.0.0.1:port=%s'
+    server_endpoint_pattern = 'tcp:0:interface=127.0.0.1'
+
+    def get_server_endpoint(self):
+        """Return the server endpoint description."""
+        return self.server_endpoint_pattern
+
+    def get_client_endpoint(self):
+        """Return the client endpoint description."""
+        if self.server_factory is None:
+            raise ValueError('Server Factory was not provided.')
+        if self.listener is None:
+            raise ValueError('%s has not started listening.',
+                                                          self.server_factory)
+        return self.client_endpoint_pattern % self.listener.getHost().port
+
+
+class TidyUnixServer(TidySocketServer):
+    """A tidy unix domain sockets server."""
+
+    client_endpoint_pattern = 'unix:path=%s'
+    server_endpoint_pattern = 'unix:%s'
+
+    def __init__(self):
+        """Create a new instance."""
+        super(TidyUnixServer, self).__init__()
+        self.temp_dir = tempfile.mkdtemp()
+        self.path = os.path.join(self.temp_dir, 'tidy_unix_server')
+
+    def get_server_endpoint(self):
+        """Return the server endpoint description."""
+        return self.server_endpoint_pattern % self.path
+
+    def get_client_endpoint(self):
+        """Return the client endpoint description."""
+        return self.client_endpoint_pattern % self.path
+
+    def clean_up(self):
+        """Action to be performed for clean up."""
+        result = super(TidyUnixServer, self).clean_up()
+        # remove the dir once we are disconnected
+        result.addCallback(lambda _: shutil.rmtree(self.temp_dir))
+        return result
+
+
+class ServerTestCase(BaseTestCase):
+    """Base test case for tidy servers."""
 
     @defer.inlineCallbacks
     def setUp(self):
         """Set the diff tests."""
-        yield super(TCPServerTestCase, self).setUp()
-        self.server_runner = TidyTCPServer()
+        yield super(ServerTestCase, self).setUp()
+
+        try:
+            self.server_runner = self.get_server()
+        except NotImplementedError:
+            self.server_runner = None
 
         self.server_factory = None
         self.client_factory = None
@@ -183,28 +242,32 @@
         self.connector = None
         self.addCleanup(self.tear_down_server_client)
 
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        raise NotImplementedError('To be implemented by child classes.')
+
+    @defer.inlineCallbacks
     def listen_server(self, server_class, *args, **kwargs):
         """Listen a server.
 
         The method takes the server class and the arguments that should be
         passed to the server constructor.
         """
-        self.server_factory = self.server_runner.listen_server(server_class,
-                *args, **kwargs)
+        self.server_factory = yield self.server_runner.listen_server(
+                                                server_class, *args, **kwargs)
         self.server_disconnected = \
                 self.server_factory.testserver_on_connection_lost
         self.listener = self.server_runner.listener
 
+    @defer.inlineCallbacks
     def connect_client(self, client_class, *args, **kwargs):
         """Connect the client.
 
         The method takes the client factory  class and the arguments that
         should be passed to the client constructor.
         """
-        self.client_factory = self.server_runner.connect_client(client_class,
-                                                              *args, **kwargs)
-        self.client_connected = \
-                self.client_factory.testserver_on_connection_made
+        self.client_factory = yield self.server_runner.connect_client(
+                                                client_class, *args, **kwargs)
         self.client_disconnected = \
                 self.client_factory.testserver_on_connection_lost
         self.connector = self.server_runner.connector
@@ -215,15 +278,53 @@
             return self.server_runner.clean_up()
 
 
-class PbServerTestCase(TCPServerTestCase):
+class TCPServerTestCase(ServerTestCase):
+    """Test that uses a single twisted server."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyTCPServer()
+
+
+class UnixServerTestCase(ServerTestCase):
+    """Test that uses a single twisted server."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyUnixServer()
+
+
+class PbServerTestCase(ServerTestCase):
     """Test a pb server."""
 
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        raise NotImplementedError('To be implemented by child classes.')
+
+    @defer.inlineCallbacks
     def listen_server(self, *args, **kwargs):
         """Listen a pb server."""
-        super(PbServerTestCase, self).listen_server(pb.PBServerFactory,
+        yield super(PbServerTestCase, self).listen_server(pb.PBServerFactory,
                                                               *args, **kwargs)
 
+    @defer.inlineCallbacks
     def connect_client(self, *args, **kwargs):
         """Connect a pb client."""
-        super(PbServerTestCase, self).connect_client(pb.PBClientFactory,
+        yield super(PbServerTestCase, self).connect_client(pb.PBClientFactory,
                                                               *args, **kwargs)
+
+
+class TCPPbServerTestCase(PbServerTestCase):
+    """Test a pb server over TCP."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyTCPServer()
+
+
+class UnixPbServerTestCase(PbServerTestCase):
+    """Test a pb server over Unix domain sockets."""
+
+    def get_server(self):
+        """Return the server to be used to run the tests."""
+        return TidyUnixServer()

=== modified file 'ubuntuone/devtools/testing/txwebserver.py'
--- ubuntuone/devtools/testing/txwebserver.py	2012-04-17 15:53:14 +0000
+++ ubuntuone/devtools/testing/txwebserver.py	2012-04-26 14:30:44 +0000
@@ -32,7 +32,7 @@
 from twisted.protocols.policies import WrappingFactory
 from twisted.web import server
 
-from ubuntuone.devtools.testcases.txtcpserver import server_protocol_factory
+from ubuntuone.devtools.testcases.txsocketserver import server_protocol_factory
 
 # no init method +  twisted common warnings
 # pylint: disable=W0232, C0103, E1101