Merge lp:~cjwatson/storm/postgresfixture into lp:storm

[Colin Watson]

Use postgresfixture to simplify developer setup.

Since postgresfixture provides a cluster listening on a Unix-domain socket, we use a TCP proxy for tests that require a TCP connection string.

This necessitated some minor improvements to test cleanup code to ensure that the temporary database can be dropped cleanly.

[Simon Poirier (simpoir)]

+1 LGTM

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2019-09-17 09:45:08 +0000
+++ .bzrignore	2019-09-29 14:08:50 +0000
@@ -2,6 +2,8 @@
 build
 storm.egg-info
 build-stamp
+db
+.db.lock
 dist
 debian/files
 debian/python-storm

=== modified file 'MANIFEST.in'
--- MANIFEST.in	2019-08-11 17:51:37 +0000
+++ MANIFEST.in	2019-09-29 14:08:50 +0000
@@ -1,3 +1,5 @@
 recursive-include storm *.py *.c *.zcml *.txt
 
 include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test tox.ini
+
+prune db

=== modified file 'NEWS'
--- NEWS	2019-09-27 19:22:54 +0000
+++ NEWS	2019-09-29 14:08:50 +0000
@@ -1,6 +1,12 @@
 0.21.1 (XXXX-XX-XX)
 ===================
 
+Improvements
+------------
+
+- Use the postgresfixture package to set up a temporary cluster for
+  PostgreSQL tests, simplifying developer setup.
+
 Bug fixes
 ---------
 

=== modified file 'README'
--- README	2017-02-08 11:47:02 +0000
+++ README	2019-09-29 14:08:50 +0000
@@ -90,7 +90,6 @@
 
  $ dev/ubuntu-deps
  $ make develop
- $ dev/db-setup
  $ make check
 
 LONG VERSION:

=== removed file 'dev/db-setup'
--- dev/db-setup	2016-03-10 12:04:01 +0000
+++ dev/db-setup	1970-01-01 00:00:00 +0000
@@ -1,45 +0,0 @@
-#!/bin/bash -eu
-
-PGHBA=/etc/postgresql/*/main/pg_hba.conf
-PGCONF=/etc/postgresql/*/main/postgresql.conf
-PGINIT=/etc/init.d/postgresql*
-
-is_number() {
-    test "$1" && printf '%f' "$1" >/dev/null;
-}
-
-# Restart service and examine output to attempt to adjust kernel tunables
-restart_service() {
-    sudo $PGINIT stop
-    set +e
-    output=$(sudo $PGINIT start 2>&1)
-    rc=$?
-    set -e
-    if [ $rc -ne 0 ]; then
-        size=$(echo $output | grep -E 'size=[0-9]+' | sed -r 's/.*size=([0-9]+).*/\1/')
-        is_number $size
-        SYSCTLCONF=/etc/sysctl.d/90-storm.conf
-        sudo sh -c "echo kernel.shmmax = $size > $SYSCTLCONF"
-        sudo sysctl -p $SYSCTLCONF
-        sudo $PGINIT start
-    fi
-}
-
-echo " * Fixing localhost access to postgres, restarting service"
-sudo sed -i.bak -r 's/(host.*127\.0\.0\.1.*)\s+\w+$/\1 trust/' $PGHBA
-sudo sed -i.bak -r 's/(host.*::1\/128.*)\s+\w+$/\1 trust/' $PGHBA
-sudo sed -i.bak -r 's/(host.*::1\/128.*)\s+\w+$/\1 trust/' $PGHBA
-sudo sed -i.bak -r 's/#(max_prepared_transactions.*)= 0/\1 = 200/' $PGCONF
-restart_service
-
-echo " * Create postgres superuser ($USER - will fail if existing)"
-# If this fails, we will get errors later, so don't fail.
-sudo -u postgres createuser --superuser $USER || /bin/true
-
-echo " * Create DB Test Fixtures (will fail if existing)"
-createdb storm_test || /bin/true
-
-echo " * Testing DB Access for user:$USER"
-pg_dump storm_test > /dev/null
-
-echo " * All Done."

=== modified file 'setup.py'
--- setup.py	2019-09-28 13:03:31 +0000
+++ setup.py	2019-09-29 14:08:50 +0000
@@ -24,6 +24,7 @@
     "fixtures >= 0.3.5",
     # pgbouncer (the Python module) is not yet packaged in Ubuntu.
     "pgbouncer >= 0.0.7",
+    "postgresfixture",
     "psycopg2 >= 2.3.0",
     "testresources >= 0.2.4",
     "testtools >= 0.9.8",

=== modified file 'storm/tests/databases/base.py'
--- storm/tests/databases/base.py	2019-09-27 19:22:54 +0000
+++ storm/tests/databases/base.py	2019-09-29 14:08:50 +0000
@@ -700,6 +700,7 @@
         self.create_connection()
 
     def tearDown(self):
+        self.drop_connection()
         self.drop_database()
         self.proxy.close()
         super(DatabaseDisconnectionMixin, self).tearDown()
@@ -708,18 +709,21 @@
         return bool(self.get_uri())
 
     def get_uri(self):
-        """Return URI instance with a defined host and port."""
-        if not self.environment_variable or not self.default_port:
-            raise RuntimeError("Define at least %s.environment_variable and "
-                               "%s.default_port" % (type(self).__name__,
-                                                    type(self).__name__))
+        """Return URI instance with a defined host (and port, for TCP)."""
+        if not self.environment_variable:
+            raise RuntimeError(
+                "Define at least %s.environment_variable" %
+                type(self).__name__)
         uri_str = os.environ.get(self.host_environment_variable)
         if uri_str:
             uri = URI(uri_str)
             if not uri.host:
                 raise RuntimeError("The URI in %s must include a host." %
                                    self.host_environment_variable)
-            if not uri.port:
+            if not uri.host.startswith("/") and not uri.port:
+                if not self.default_port:
+                    raise RuntimeError(
+                        "Define at least %s.default_port" % type(self).__name)
                 uri.port = self.default_port
             return uri
         else:
@@ -727,11 +731,19 @@
             if uri_str:
                 uri = URI(uri_str)
                 if uri.host:
-                    if not uri.port:
+                    if not uri.host.startswith("/") and not uri.port:
+                        if not self.default_port:
+                            raise RuntimeError(
+                                "Define at least %s.default_port" %
+                                type(self).__name)
                         uri.port = self.default_port
                     return uri
         return None
 
+    def create_proxy(self, uri):
+        """Create a TCP proxy forwarding requests to `uri`."""
+        return ProxyTCPServer((uri.host, uri.port))
+
     def create_database_and_proxy(self):
         """Set up the TCP proxy and database object.
 
@@ -739,7 +751,7 @@
         database object should point at the TCP proxy.
         """
         uri = self.get_uri()
-        self.proxy = ProxyTCPServer((uri.host, uri.port))
+        self.proxy = self.create_proxy(uri)
         uri.host, uri.port = self.proxy.server_address
         self.proxy_uri = uri
         self.database = create_database(uri)
@@ -747,6 +759,9 @@
     def create_connection(self):
         self.connection = self.database.connect()
 
+    def drop_connection(self):
+        self.connection.close()
+
     def drop_database(self):
         pass
 

=== modified file 'storm/tests/databases/postgres.py'
--- storm/tests/databases/postgres.py	2019-09-17 09:35:10 +0000
+++ storm/tests/databases/postgres.py	2019-09-29 14:08:50 +0000
@@ -25,6 +25,7 @@
 import json
 
 import six
+from six.moves.urllib.parse import urlunsplit
 
 from storm.databases.postgres import (
     Postgres, compile, currval, Returning, Case, PostgresTimeoutTracer,
@@ -50,6 +51,7 @@
 from storm.tests.databases.base import (
     DatabaseTest, DatabaseDisconnectionTest, UnsupportedDatabaseTest,
     TwoPhaseCommitTest, TwoPhaseCommitDisconnectionTest)
+from storm.tests.databases.proxy import ProxyTCPServer
 from storm.tests.expr import column1, column2, column3, elem1, table1, TrackContext
 from storm.tests.tracer import TimeoutTracerTestBase
 from storm.tests.helper import TestHelper
@@ -62,6 +64,16 @@
     has_pgbouncer = True
 
 
+def create_proxy_and_uri(uri):
+    """Create a TCP proxy to a Unix-domain database identified by `uri`."""
+    proxy = ProxyTCPServer(os.path.join(uri.host, ".s.PGSQL.5432"))
+    proxy_host, proxy_port = proxy.server_address
+    proxy_uri = URI(urlunsplit(
+        ("postgres", "%s:%s" % (proxy_host, proxy_port), "/storm_test",
+         "", "")))
+    return proxy, proxy_uri
+
+
 def terminate_other_backends(connection):
     """Terminate all connections to the database except the one given."""
     pid_column = "procpid" if connection._database._version < 90200 else "pid"
@@ -737,6 +749,13 @@
     host_environment_variable = "STORM_POSTGRES_HOST_URI"
     default_port = 5432
 
+    def create_proxy(self, uri):
+        """See `DatabaseDisconnectionMixin.create_proxy`."""
+        if uri.host.startswith("/"):
+            return create_proxy_and_uri(uri)[0]
+        else:
+            return super(PostgresDisconnectionTest, self).create_proxy(uri)
+
     def test_rollback_swallows_InterfaceError(self):
         """Test that InterfaceErrors get caught on rollback().
 
@@ -803,6 +822,13 @@
 
     database_uri = os.environ.get("STORM_POSTGRES_HOST_URI")
 
+    def setUp(self):
+        super(PostgresDisconnectionTestWithoutProxyTCPSockets, self).setUp()
+        if self.database.get_uri().host.startswith("/"):
+            proxy, proxy_uri = create_proxy_and_uri(self.database.get_uri())
+            self.addCleanup(proxy.close)
+            self.database = create_database(proxy_uri)
+
 
 class PostgresDisconnectionTestWithPGBouncerBase(object):
     # Connecting via pgbouncer <http://pgfoundry.org/projects/pgbouncer>
@@ -816,6 +842,9 @@
     def setUp(self):
         super(PostgresDisconnectionTestWithPGBouncerBase, self).setUp()
         database_uri = URI(os.environ["STORM_POSTGRES_HOST_URI"])
+        if database_uri.host.startswith("/"):
+            proxy, database_uri = create_proxy_and_uri(database_uri)
+            self.addCleanup(proxy.close)
         database_user = database_uri.username or os.environ['USER']
         database_dsn = make_dsn(database_uri)
         # Create a pgbouncer fixture.
@@ -874,6 +903,10 @@
         self.tracer.get_remaining_time = lambda: self.remaining_time
         self.remaining_time = 10.5
 
+    def tearDown(self):
+        self.connection.close()
+        super(PostgresTimeoutTracerTest, self).tearDown()
+
     def test_set_statement_timeout(self):
         result = self.connection.execute("SHOW statement_timeout")
         self.assertEquals(result.get_one(), ("10500ms",))

=== modified file 'storm/tests/databases/proxy.py'
--- storm/tests/databases/proxy.py	2019-06-07 17:14:33 +0000
+++ storm/tests/databases/proxy.py	2019-09-29 14:08:50 +0000
@@ -22,11 +22,13 @@
 
 from __future__ import print_function
 
+import errno
 import os
 import select
 import socket
 import threading
 
+import six
 from six.moves import socketserver
 
 
@@ -42,7 +44,10 @@
             self, request, client_address, server)
 
     def handle(self):
-        dst = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        if isinstance(self.server.proxy_dest, (bytes, six.text_type)):
+            dst = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        else:
+            dst = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         dst.connect(self.server.proxy_dest)
 
         readers = [self.request, dst]
@@ -56,13 +61,23 @@
 
             if self.request in rlist:
                 chunk = os.read(self.request.fileno(), 1024)
-                dst.send(chunk)
+                try:
+                    dst.send(chunk)
+                except socket.error as e:
+                    if e.errno == errno.EPIPE:
+                        return
+                    raise
                 if chunk == "":
                     readers.remove(self.request)
                     dst.shutdown(socket.SHUT_WR)
 
             if dst in rlist:
-                chunk = os.read(dst.fileno(), 1024)
+                try:
+                    chunk = os.read(dst.fileno(), 1024)
+                except OSError as e:
+                    if e.errno == errno.ECONNRESET:
+                        return
+                    raise
                 self.request.send(chunk)
                 if chunk == "":
                     readers.remove(dst)

=== modified file 'storm/tests/mocker.py'
--- storm/tests/mocker.py	2019-09-17 09:35:10 +0000
+++ storm/tests/mocker.py	2019-09-29 14:08:50 +0000
@@ -149,7 +149,7 @@
             elif os.path.isdir(path):
                 shutil.rmtree(path)
         self.mocker.restore()
-        for func, args, kwargs in self.__cleanup_funcs:
+        for func, args, kwargs in reversed(self.__cleanup_funcs):
             func(*args, **kwargs)
 
     def addCleanup(self, func, *args, **kwargs):

=== modified file 'test'
--- test	2019-08-11 17:51:37 +0000
+++ test	2019-09-29 14:08:50 +0000
@@ -21,6 +21,7 @@
 #
 import glob
 import optparse
+import re
 import unittest
 import sys
 import os
@@ -73,6 +74,43 @@
     return test_with_runner(runner)
 
 
+def with_postgresfixture(runner_func):
+    """If possible, wrap a test runner with code to set up PostgreSQL."""
+    try:
+        from postgresfixture import ClusterFixture
+    except ImportError:
+        return runner_func
+
+    from six.moves.urllib.parse import urlunsplit
+    from storm.uri import escape
+
+    def wrapper():
+        cluster = ClusterFixture("db")
+        cluster.create()
+        # Allow use of prepared transactions, which some tests need.
+        pg_conf_path = os.path.join(cluster.datadir, "postgresql.conf")
+        with open(pg_conf_path) as pg_conf_old:
+            with open(pg_conf_path + ".new", "w") as pg_conf_new:
+                for line in pg_conf_old:
+                    pg_conf_new.write(re.sub(
+                        r"^#(max_prepared_transactions.*)= 0", r"\1= 200",
+                        line))
+                os.fchmod(
+                    pg_conf_new.fileno(),
+                    os.fstat(pg_conf_old.fileno()).st_mode)
+        os.rename(pg_conf_path + ".new", pg_conf_path)
+
+        with cluster:
+            cluster.createdb("storm_test")
+            uri = urlunsplit(
+                ("postgres", escape(cluster.datadir), "/storm_test", "", ""))
+            os.environ["STORM_POSTGRES_URI"] = uri
+            os.environ["STORM_POSTGRES_HOST_URI"] = uri
+            return runner_func()
+
+    return wrapper
+
+
 if __name__ == "__main__":
     runner = os.environ.get("STORM_TEST_RUNNER")
     if not runner:
@@ -80,6 +118,7 @@
     runner_func = globals().get("test_with_%s" % runner.replace(".", "_"))
     if not runner_func:
         sys.exit("Test runner not found: %s" % runner)
+    runner_func = with_postgresfixture(runner_func)
     sys.exit(runner_func())
 
 # vim:ts=4:sw=4:et

=== modified file 'tox.ini'
--- tox.ini	2019-09-17 09:45:08 +0000
+++ tox.ini	2019-09-29 14:08:50 +0000
@@ -8,8 +8,6 @@
     .[test]
 passenv = STORM_TEST_RUNNER USER
 setenv =
-    STORM_POSTGRES_URI = postgres:storm_test
-    STORM_POSTGRES_HOST_URI = postgres://localhost:5432/storm_test
     cextensions: STORM_CEXTENSIONS = 1
     nocextensions: STORM_CEXTENSIONS = 0
 commands =