Merge lp:~mandel/ubuntuone-dev-tools/mocked-webserver into lp:ubuntuone-dev-tools

[Manuel de la Peña]

- Added a mock Web server which ensures its resources are correctly finalized, for use in tests (LP: #972366).

- Added a mock Webserver than can be used in tests and that ensures that its resources are correctly closed (LP: #972366).

[dobey (dobey)]

107 + def test_get_iri(self):
108 + """Test getting the iri from the server."""
109 + expected = u"http://127.0.0.1:%d/"
110 + self.assert_url(expected)
111 +
112 + def test_get_uri(self):
113 + """Test getting the uri from the server."""
114 + expected = "http://127.0.0.1:%d/"
115 + self.assert_url(expected)

128 + def test_get_ssl_iri(self):
129 + """Test getting the ssl iri."""
130 + self.assertEqual(None, self.server.get_ssl_uri())
131 +
132 + def test_get_ssl_uri(self):
133 + """Test getting the ssl uri."""
134 + self.assertEqual(None, self.server.get_ssl_uri())

274 + def get_iri(self):
275 + """Build the iri for this mock server."""
276 + url = u"http://127.0.0.1:%d/"
277 + return url % self.get_port()
278 +
279 + def get_uri(self):
280 + """Build the uri for this mock server."""
281 + url = "http://127.0.0.1:%d/"
282 + return url % self.get_port()
283 +
284 + def get_ssl_iri(self):
285 + """Build the ssl iri for this mock server."""
286 + if self.ssl_settings:
287 + url = u"https://127.0.0.1:%d/"
288 + return url % self.get_ssl_port()
289 +
290 + def get_ssl_uri(self):
291 + """Build the ssl iri for this mock server."""
292 + if self.ssl_settings:
293 + url = "https://127.0.0.1:%d/"
294 + return url % self.get_ssl_port()

Why all this duplicated code?

review: Needs Fixing

[Alejandro J. Cura (alecu)]

Nice branch. This is a very good second step in order to reduce the amount of boilerplate we have everywhere in our tests.

The only bit I don't like is the many "if self.ssl_settings" scattered around.
What about splitting the WebServer class into HTTPS and HTTP classes?

review: Needs Fixing

[Manuel de la Peña (mandel)]

> 107 + def test_get_iri(self):
> 108 + """Test getting the iri from the server."""
> 109 + expected = u"http://127.0.0.1:%d/"
> 110 + self.assert_url(expected)
> 111 +
> 112 + def test_get_uri(self):
> 113 + """Test getting the uri from the server."""
> 114 + expected = "http://127.0.0.1:%d/"
> 115 + self.assert_url(expected)
>
> 128 + def test_get_ssl_iri(self):
> 129 + """Test getting the ssl iri."""
> 130 + self.assertEqual(None, self.server.get_ssl_uri())
> 131 +
> 132 + def test_get_ssl_uri(self):
> 133 + """Test getting the ssl uri."""
> 134 + self.assertEqual(None, self.server.get_ssl_uri())
>
> 274 + def get_iri(self):
> 275 + """Build the iri for this mock server."""
> 276 + url = u"http://127.0.0.1:%d/"
> 277 + return url % self.get_port()
> 278 +
> 279 + def get_uri(self):
> 280 + """Build the uri for this mock server."""
> 281 + url = "http://127.0.0.1:%d/"
> 282 + return url % self.get_port()
> 283 +
> 284 + def get_ssl_iri(self):
> 285 + """Build the ssl iri for this mock server."""
> 286 + if self.ssl_settings:
> 287 + url = u"https://127.0.0.1:%d/"
> 288 + return url % self.get_ssl_port()
> 289 +
> 290 + def get_ssl_uri(self):
> 291 + """Build the ssl iri for this mock server."""
> 292 + if self.ssl_settings:
> 293 + url = "https://127.0.0.1:%d/"
> 294 + return url % self.get_ssl_port()
>
> Why all this duplicated code?

Looks like a merge fail from me, let me double check.

[Manuel de la Peña (mandel)]

> Nice branch. This is a very good second step in order to reduce the amount of
> boilerplate we have everywhere in our tests.
>
> The only bit I don't like is the many "if self.ssl_settings" scattered around.
> What about splitting the WebServer class into HTTPS and HTTP classes?

Sure!

[dobey (dobey)]

+SAMPLE_RESOURCE = '{"%s": "%s"}' % (SAMPLE_KEY, SAMPLE_VALUE)

There are a lot of strings being combined with %-notation in here. Please use the new .format() notation for string formatting instead, as %-notation is deprecated. As the new notation uses {0} for example as insertion markers, you probably should use unicode({u'{0}'.format(SAMPLE_KEY): u'{0}'.format(SAMPLE_VALUE)}) for this perhaps.

review: Needs Fixing

[Alejandro J. Cura (alecu)]

> +SAMPLE_RESOURCE = '{"%s": "%s"}' % (SAMPLE_KEY, SAMPLE_VALUE)
>
> There are a lot of strings being combined with %-notation in here. Please use
> the new .format() notation for string formatting instead, as %-notation is
> deprecated. As the new notation uses {0} for example as insertion markers, you
> probably should use unicode({u'{0}'.format(SAMPLE_KEY):
> u'{0}'.format(SAMPLE_VALUE)}) for this perhaps.

The SAMPLE_RESOURCE is just a json-like sequence of bytes used for testing, so the fix requested is wrong.
It should be something like:
+SAMPLE_RESOURCE = '{{"{0}": "{1}"}}'.format(SAMPLE_KEY, SAMPLE_VALUE)

Also, and for the record, the string interpolation operator % is not deprecated; not even in python 3.3:
"there are no current plans to deprecate printf-style formatting."
http://docs.python.org/dev/library/stdtypes.html#old-string-formatting-operations

[Alejandro J. Cura (alecu)]

+1

review: Approve

[dobey (dobey)]

Can we have the scheme, host, and port all passed around separately? Passing around "http://foo:{0}" is ugly, and exposes the string structure in the API. Would be better to just pass in scheme and host in these functions, and have the internal code properly construct the URL.

[dobey (dobey)]

Also, as this doesn't provide any testcases, I think it's better to be under testing/ instead of testcases/.

[dobey (dobey)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~mandel/ubuntuone-dev-tools/mocked-webserver into lp:ubuntuone-dev-tools failed. Below is the output from the failed tests.

Traceback (most recent call last):
  File "bin/u1trial", line 337, in <module>
    main()
  File "bin/u1trial", line 317, in main
    suite = trial_runner.get_suite(config)
  File "bin/u1trial", line 196, in get_suite
    config['ignore-paths']))
  File "bin/u1trial", line 180, in _collect_tests
    module_suite = self._load_unittest(filepath)
  File "bin/u1trial", line 120, in _load_unittest
    module = __import__(modpath, None, None, [""])
ImportError: No module named tests.test_txwebserver

[lp:~mandel/ubuntuone-dev-tools/mocked-webserver]

99. By Manuel de la Peña on 2012-04-17

Added missing __init__.py

=== modified file 'ubuntuone/devtools/testcases/txtcpserver.py'
--- ubuntuone/devtools/testcases/txtcpserver.py	2012-04-13 16:39:56 +0000
+++ ubuntuone/devtools/testcases/txtcpserver.py	2012-04-17 19:35:27 +0000
@@ -114,7 +114,6 @@
 
     def listen_server(self, server_class, *args, **kwargs):
         """Start a server in a random port."""
-
         from twisted.internet import reactor
         self.server_factory = server_class(*args, **kwargs)
         self.server_factory._disconnecting = False

=== added directory 'ubuntuone/devtools/testing/tests'
=== added file 'ubuntuone/devtools/testing/tests/__init__.py'
--- ubuntuone/devtools/testing/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/devtools/testing/tests/__init__.py	2012-04-17 19:35:27 +0000
@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# In addition, as a special exception, the copyright holders give
+# permission to link the code of portions of this program with the
+# OpenSSL library under certain conditions as described in each
+# individual source file, and distribute linked combinations
+# including the two.
+# You must obey the GNU General Public License in all respects
+# for all of the code used other than OpenSSL.  If you modify
+# file(s) with this exception, you may extend this exception to your
+# version of the file(s), but you are not obligated to do so.  If you
+# do not wish to do so, delete this exception statement from your
+# version.  If you delete this exception statement from all source
+# files in the program, then also delete it here.
+"""Tests for the testing module."""

=== added file 'ubuntuone/devtools/testing/tests/test_txwebserver.py'
--- ubuntuone/devtools/testing/tests/test_txwebserver.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/devtools/testing/tests/test_txwebserver.py	2012-04-17 19:35:27 +0000
@@ -0,0 +1,172 @@
+# -*- coding: utf-8 -*-
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# In addition, as a special exception, the copyright holders give
+# permission to link the code of portions of this program with the
+# OpenSSL library under certain conditions as described in each
+# individual source file, and distribute linked combinations
+# including the two.
+# You must obey the GNU General Public License in all respects
+# for all of the code used other than OpenSSL.  If you modify
+# file(s) with this exception, you may extend this exception to your
+# version of the file(s), but you are not obligated to do so.  If you
+# do not wish to do so, delete this exception statement from your
+# version.  If you delete this exception statement from all source
+# files in the program, then also delete it here.
+
+"""Test the twisted webserver."""
+
+
+from twisted.internet import defer
+from twisted.trial.unittest import TestCase
+from twisted.web import client, resource
+
+from ubuntuone.devtools.testing.txwebserver import HTTPWebServer
+
+
+SAMPLE_KEY = "result"
+SAMPLE_VALUE = "sample result"
+SAMPLE_RESOURCE = '{{"{0}": "{1}"}}'.format(SAMPLE_KEY, SAMPLE_VALUE)
+SIMPLERESOURCE = "simpleresource"
+OTHER_SIMPLERESOURCE = "othersimpleresource"
+THROWERROR = "throwerror"
+UNAUTHORIZED = "unauthorized"
+
+# no init
+# pylint: disable=W0232
+
+
+class SimpleResource(resource.Resource):
+    """A simple web resource."""
+
+    # pylint: disable=C0103
+    def render_GET(self, request):
+        """Make a bit of html out of these resource's content."""
+        return SAMPLE_RESOURCE
+    # pylint: enable=C0103
+
+
+class WebServerTestCase(TestCase):
+    """Test the web server that will allow to have connections."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the different tests."""
+        yield super(WebServerTestCase, self).setUp()
+        # create the root to be used by the webserver
+        root = resource.Resource()
+        root.putChild(SIMPLERESOURCE, SimpleResource())
+        root.putChild(OTHER_SIMPLERESOURCE, SimpleResource())
+        root.putChild(THROWERROR, resource.NoResource())
+
+        unauthorized_resource = resource.ErrorPage(resource.http.UNAUTHORIZED,
+                                                "Unauthorized", "Unauthorized")
+        root.putChild(UNAUTHORIZED, unauthorized_resource)
+        self.server = HTTPWebServer(root)
+        self.server.start()
+        self.uri = "http://127.0.0.1:{port}/".format(
+                                                   port=self.server.get_port())
+        self.addCleanup(self.server.stop)
+
+    @defer.inlineCallbacks
+    def test_single_request(self):
+        """Test performing a single request to get the data."""
+        url = self.uri + SIMPLERESOURCE
+        result = yield client.getPage(url)
+        self.assertEqual(SAMPLE_RESOURCE, result)
+
+    @defer.inlineCallbacks
+    def test_multiple_requests(self):
+        """Test performing multiple requests."""
+        simple_url = self.uri + SIMPLERESOURCE
+        other_simple_url = self.uri + OTHER_SIMPLERESOURCE
+        simple_result = yield client.getPage(simple_url)
+        other_result = yield client.getPage(other_simple_url)
+        self.assertEqual(SAMPLE_RESOURCE, simple_result)
+        self.assertEqual(SAMPLE_RESOURCE, other_result)
+
+    def assert_url(self, expected):
+        """Assert the url."""
+        port = self.server.get_port()
+        self.assertEqual(expected.format(port=port), self.server.get_iri())
+
+    def test_get_iri(self):
+        """Test getting the iri from the server."""
+        expected = u"http://127.0.0.1:{port}/"
+        self.assert_url(expected)
+
+    def test_get_port(self):
+        """Test getting the port."""
+        # pylint: disable=W0212
+        port = self.server.port.getHost().port
+        # pylint: enable=W0212
+        self.assertEqual(port, self.server.get_port())
+
+
+class MultipleWebServersTestCase(TestCase):
+    """Test with multiple webservers running."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the diff tests."""
+        yield super(MultipleWebServersTestCase, self).setUp()
+        self.root = resource.Resource()
+        self.root.putChild(SIMPLERESOURCE, SimpleResource())
+        self.root.putChild(OTHER_SIMPLERESOURCE, SimpleResource())
+        self.root.putChild(THROWERROR, resource.NoResource())
+
+        unauthorized_resource = resource.ErrorPage(resource.http.UNAUTHORIZED,
+                                                "Unauthorized", "Unauthorized")
+        self.root.putChild(UNAUTHORIZED, unauthorized_resource)
+
+    def get_uri(self, server):
+        """Return the uri for the server."""
+        url = "http://127.0.0.1:{port}/"
+        return url.format(port=server.get_port())
+
+    @defer.inlineCallbacks
+    def test_single_request(self):
+        """Test performing a single request to get the data."""
+        first_server = HTTPWebServer(self.root)
+        first_server.start()
+        self.addCleanup(first_server.stop)
+
+        second_server = HTTPWebServer(self.root)
+        second_server.start()
+        self.addCleanup(second_server.stop)
+
+        for server in [first_server, second_server]:
+            url = self.get_uri(server) + SIMPLERESOURCE
+            result = yield client.getPage(url)
+            self.assertEqual(SAMPLE_RESOURCE, result)
+
+    @defer.inlineCallbacks
+    def test_multiple_requests(self):
+        """Test performing multiple requests."""
+        first_server = HTTPWebServer(self.root)
+        first_server.start()
+        self.addCleanup(first_server.stop)
+
+        second_server = HTTPWebServer(self.root)
+        second_server.start()
+        self.addCleanup(second_server.stop)
+
+        for server in [first_server, second_server]:
+            simple_url = self.get_uri(server) + SIMPLERESOURCE
+            other_simple_url = self.get_uri(server) + OTHER_SIMPLERESOURCE
+            simple_result = yield client.getPage(simple_url)
+            other_result = yield client.getPage(other_simple_url)
+            self.assertEqual(SAMPLE_RESOURCE, simple_result)
+            self.assertEqual(SAMPLE_RESOURCE, other_result)

=== added file 'ubuntuone/devtools/testing/txwebserver.py'
--- ubuntuone/devtools/testing/txwebserver.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/devtools/testing/txwebserver.py	2012-04-17 19:35:27 +0000
@@ -0,0 +1,121 @@
+# -*- coding: utf-8 -*-
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# In addition, as a special exception, the copyright holders give
+# permission to link the code of portions of this program with the
+# OpenSSL library under certain conditions as described in each
+# individual source file, and distribute linked combinations
+# including the two.
+# You must obey the GNU General Public License in all respects
+# for all of the code used other than OpenSSL.  If you modify
+# file(s) with this exception, you may extend this exception to your
+# version of the file(s), but you are not obligated to do so.  If you
+# do not wish to do so, delete this exception statement from your
+# version.  If you delete this exception statement from all source
+# files in the program, then also delete it here.
+
+"""A tx based web server."""
+
+from twisted.internet import defer, reactor, ssl
+from twisted.protocols.policies import WrappingFactory
+from twisted.web import server
+
+from ubuntuone.devtools.testcases.txtcpserver import server_protocol_factory
+
+# no init method +  twisted common warnings
+# pylint: disable=W0232, C0103, E1101
+
+
+class BaseWebServer(object):
+    """Webserver used to perform requests in tests."""
+
+    def __init__(self, root_resource, scheme):
+        """Create and start the instance.
+
+        The ssl_settings parameter contains a dictionary with the key and cert
+        that will be used to perform ssl connections. The root_resource
+        contains the resource with all its childre.
+        """
+        self.root = root_resource
+        self.scheme = scheme
+        self.port = None
+        # use an http.HTTPFactory that was modified to ensure that we have
+        # clean close connections
+        self.site = server.Site(self.root, timeout=None)
+        self.wrapper = WrappingFactory(self.site)
+        self.wrapper.testserver_on_connection_lost = defer.Deferred()
+        self.wrapper.protocol = server_protocol_factory(self.wrapper.protocol)
+        self.wrapper._disconnecting = False
+
+    def listen(self, site):
+        """Listen a port to allow the tests."""
+        raise NotImplementedError('Base abstract class.')
+
+    def get_iri(self):
+        """Build the iri for this mock server."""
+        return u"{scheme}://127.0.0.1:{port}/".format(scheme=self.scheme,
+                                                   port=self.get_port())
+
+    def get_port(self):
+        """Return the port where we are listening."""
+        return self.port.getHost().port
+
+    def start(self):
+        """Start the service."""
+        self.port = self.listen(self.wrapper)
+
+    def stop(self):
+        """Shut it down."""
+        if self.port:
+            self.wrapper._disconnecting = True
+            connected = self.wrapper.protocols.keys()
+            if connected:
+                for con in connected:
+                    con.transport.loseConnection()
+            else:
+                self.wrapper.testserver_on_connection_lost = \
+                                                            defer.succeed(None)
+            d = defer.maybeDeferred(self.port.stopListening)
+            return defer.gatherResults([d,
+                                  self.wrapper.testserver_on_connection_lost])
+        return defer.succeed(None)
+
+
+class HTTPWebServer(BaseWebServer):
+    """A Webserver that listens to http connections."""
+
+    def __init__(self, root_resource):
+        """Create  a new instance."""
+        super(HTTPWebServer, self).__init__(root_resource, 'http')
+
+    def listen(self, site):
+        """Listen a port to allow the tests."""
+        return reactor.listenTCP(0, site)
+
+
+class HTTPSWebServer(BaseWebServer):
+    """A WebServer that listens to https connections."""
+
+    def __init__(self, root_resource, ssl_settings=None):
+        """Create  a new instance."""
+        super(HTTPSWebServer, self).__init__(root_resource, 'https')
+        self.ssl_settings = ssl_settings
+
+    def listen(self, site):
+        """Listen a port to allow the tests."""
+        ssl_context = ssl.DefaultOpenSSLContextFactory(
+                self.ssl_settings['key'], self.ssl_settings['cert'])
+
+        return reactor.listenSSL(0, site, ssl_context)