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

Proposed by Colin Watson on 2019-09-29
Status: Merged
Merged at revision: 536
Proposed branch: lp:~cjwatson/storm/postgresfixture
Merge into: lp:storm
Diff against target: 425 lines (+125/-60)
12 files modified
.bzrignore (+2/-0)
MANIFEST.in (+2/-0)
NEWS (+6/-0)
README (+0/-1)
dev/db-setup (+0/-45)
setup.py (+1/-0)
storm/tests/databases/base.py (+23/-8)
storm/tests/databases/postgres.py (+33/-0)
storm/tests/databases/proxy.py (+18/-3)
storm/tests/mocker.py (+1/-1)
test (+39/-0)
tox.ini (+0/-2)
To merge this branch: bzr merge lp:~cjwatson/storm/postgresfixture
Reviewer Review Type Date Requested Status
Simon Poirier 2019-09-29 Approve on 2019-11-20
Review via email: mp+373379@code.launchpad.net

Commit message

Use postgresfixture to simplify developer setup.

Description of the change

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.

To post a comment you must log in.
Simon Poirier (simpoir) wrote :

+1 LGTM

review: Approve
Colin Watson (cjwatson) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2019-09-17 09:45:08 +0000
3+++ .bzrignore 2019-09-29 14:08:50 +0000
4@@ -2,6 +2,8 @@
5 build
6 storm.egg-info
7 build-stamp
8+db
9+.db.lock
10 dist
11 debian/files
12 debian/python-storm
13
14=== modified file 'MANIFEST.in'
15--- MANIFEST.in 2019-08-11 17:51:37 +0000
16+++ MANIFEST.in 2019-09-29 14:08:50 +0000
17@@ -1,3 +1,5 @@
18 recursive-include storm *.py *.c *.zcml *.txt
19
20 include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test tox.ini
21+
22+prune db
23
24=== modified file 'NEWS'
25--- NEWS 2019-09-27 19:22:54 +0000
26+++ NEWS 2019-09-29 14:08:50 +0000
27@@ -1,6 +1,12 @@
28 0.21.1 (XXXX-XX-XX)
29 ===================
30
31+Improvements
32+------------
33+
34+- Use the postgresfixture package to set up a temporary cluster for
35+ PostgreSQL tests, simplifying developer setup.
36+
37 Bug fixes
38 ---------
39
40
41=== modified file 'README'
42--- README 2017-02-08 11:47:02 +0000
43+++ README 2019-09-29 14:08:50 +0000
44@@ -90,7 +90,6 @@
45
46 $ dev/ubuntu-deps
47 $ make develop
48- $ dev/db-setup
49 $ make check
50
51 LONG VERSION:
52
53=== removed file 'dev/db-setup'
54--- dev/db-setup 2016-03-10 12:04:01 +0000
55+++ dev/db-setup 1970-01-01 00:00:00 +0000
56@@ -1,45 +0,0 @@
57-#!/bin/bash -eu
58-
59-PGHBA=/etc/postgresql/*/main/pg_hba.conf
60-PGCONF=/etc/postgresql/*/main/postgresql.conf
61-PGINIT=/etc/init.d/postgresql*
62-
63-is_number() {
64- test "$1" && printf '%f' "$1" >/dev/null;
65-}
66-
67-# Restart service and examine output to attempt to adjust kernel tunables
68-restart_service() {
69- sudo $PGINIT stop
70- set +e
71- output=$(sudo $PGINIT start 2>&1)
72- rc=$?
73- set -e
74- if [ $rc -ne 0 ]; then
75- size=$(echo $output | grep -E 'size=[0-9]+' | sed -r 's/.*size=([0-9]+).*/\1/')
76- is_number $size
77- SYSCTLCONF=/etc/sysctl.d/90-storm.conf
78- sudo sh -c "echo kernel.shmmax = $size > $SYSCTLCONF"
79- sudo sysctl -p $SYSCTLCONF
80- sudo $PGINIT start
81- fi
82-}
83-
84-echo " * Fixing localhost access to postgres, restarting service"
85-sudo sed -i.bak -r 's/(host.*127\.0\.0\.1.*)\s+\w+$/\1 trust/' $PGHBA
86-sudo sed -i.bak -r 's/(host.*::1\/128.*)\s+\w+$/\1 trust/' $PGHBA
87-sudo sed -i.bak -r 's/(host.*::1\/128.*)\s+\w+$/\1 trust/' $PGHBA
88-sudo sed -i.bak -r 's/#(max_prepared_transactions.*)= 0/\1 = 200/' $PGCONF
89-restart_service
90-
91-echo " * Create postgres superuser ($USER - will fail if existing)"
92-# If this fails, we will get errors later, so don't fail.
93-sudo -u postgres createuser --superuser $USER || /bin/true
94-
95-echo " * Create DB Test Fixtures (will fail if existing)"
96-createdb storm_test || /bin/true
97-
98-echo " * Testing DB Access for user:$USER"
99-pg_dump storm_test > /dev/null
100-
101-echo " * All Done."
102
103=== modified file 'setup.py'
104--- setup.py 2019-09-28 13:03:31 +0000
105+++ setup.py 2019-09-29 14:08:50 +0000
106@@ -24,6 +24,7 @@
107 "fixtures >= 0.3.5",
108 # pgbouncer (the Python module) is not yet packaged in Ubuntu.
109 "pgbouncer >= 0.0.7",
110+ "postgresfixture",
111 "psycopg2 >= 2.3.0",
112 "testresources >= 0.2.4",
113 "testtools >= 0.9.8",
114
115=== modified file 'storm/tests/databases/base.py'
116--- storm/tests/databases/base.py 2019-09-27 19:22:54 +0000
117+++ storm/tests/databases/base.py 2019-09-29 14:08:50 +0000
118@@ -700,6 +700,7 @@
119 self.create_connection()
120
121 def tearDown(self):
122+ self.drop_connection()
123 self.drop_database()
124 self.proxy.close()
125 super(DatabaseDisconnectionMixin, self).tearDown()
126@@ -708,18 +709,21 @@
127 return bool(self.get_uri())
128
129 def get_uri(self):
130- """Return URI instance with a defined host and port."""
131- if not self.environment_variable or not self.default_port:
132- raise RuntimeError("Define at least %s.environment_variable and "
133- "%s.default_port" % (type(self).__name__,
134- type(self).__name__))
135+ """Return URI instance with a defined host (and port, for TCP)."""
136+ if not self.environment_variable:
137+ raise RuntimeError(
138+ "Define at least %s.environment_variable" %
139+ type(self).__name__)
140 uri_str = os.environ.get(self.host_environment_variable)
141 if uri_str:
142 uri = URI(uri_str)
143 if not uri.host:
144 raise RuntimeError("The URI in %s must include a host." %
145 self.host_environment_variable)
146- if not uri.port:
147+ if not uri.host.startswith("/") and not uri.port:
148+ if not self.default_port:
149+ raise RuntimeError(
150+ "Define at least %s.default_port" % type(self).__name)
151 uri.port = self.default_port
152 return uri
153 else:
154@@ -727,11 +731,19 @@
155 if uri_str:
156 uri = URI(uri_str)
157 if uri.host:
158- if not uri.port:
159+ if not uri.host.startswith("/") and not uri.port:
160+ if not self.default_port:
161+ raise RuntimeError(
162+ "Define at least %s.default_port" %
163+ type(self).__name)
164 uri.port = self.default_port
165 return uri
166 return None
167
168+ def create_proxy(self, uri):
169+ """Create a TCP proxy forwarding requests to `uri`."""
170+ return ProxyTCPServer((uri.host, uri.port))
171+
172 def create_database_and_proxy(self):
173 """Set up the TCP proxy and database object.
174
175@@ -739,7 +751,7 @@
176 database object should point at the TCP proxy.
177 """
178 uri = self.get_uri()
179- self.proxy = ProxyTCPServer((uri.host, uri.port))
180+ self.proxy = self.create_proxy(uri)
181 uri.host, uri.port = self.proxy.server_address
182 self.proxy_uri = uri
183 self.database = create_database(uri)
184@@ -747,6 +759,9 @@
185 def create_connection(self):
186 self.connection = self.database.connect()
187
188+ def drop_connection(self):
189+ self.connection.close()
190+
191 def drop_database(self):
192 pass
193
194
195=== modified file 'storm/tests/databases/postgres.py'
196--- storm/tests/databases/postgres.py 2019-09-17 09:35:10 +0000
197+++ storm/tests/databases/postgres.py 2019-09-29 14:08:50 +0000
198@@ -25,6 +25,7 @@
199 import json
200
201 import six
202+from six.moves.urllib.parse import urlunsplit
203
204 from storm.databases.postgres import (
205 Postgres, compile, currval, Returning, Case, PostgresTimeoutTracer,
206@@ -50,6 +51,7 @@
207 from storm.tests.databases.base import (
208 DatabaseTest, DatabaseDisconnectionTest, UnsupportedDatabaseTest,
209 TwoPhaseCommitTest, TwoPhaseCommitDisconnectionTest)
210+from storm.tests.databases.proxy import ProxyTCPServer
211 from storm.tests.expr import column1, column2, column3, elem1, table1, TrackContext
212 from storm.tests.tracer import TimeoutTracerTestBase
213 from storm.tests.helper import TestHelper
214@@ -62,6 +64,16 @@
215 has_pgbouncer = True
216
217
218+def create_proxy_and_uri(uri):
219+ """Create a TCP proxy to a Unix-domain database identified by `uri`."""
220+ proxy = ProxyTCPServer(os.path.join(uri.host, ".s.PGSQL.5432"))
221+ proxy_host, proxy_port = proxy.server_address
222+ proxy_uri = URI(urlunsplit(
223+ ("postgres", "%s:%s" % (proxy_host, proxy_port), "/storm_test",
224+ "", "")))
225+ return proxy, proxy_uri
226+
227+
228 def terminate_other_backends(connection):
229 """Terminate all connections to the database except the one given."""
230 pid_column = "procpid" if connection._database._version < 90200 else "pid"
231@@ -737,6 +749,13 @@
232 host_environment_variable = "STORM_POSTGRES_HOST_URI"
233 default_port = 5432
234
235+ def create_proxy(self, uri):
236+ """See `DatabaseDisconnectionMixin.create_proxy`."""
237+ if uri.host.startswith("/"):
238+ return create_proxy_and_uri(uri)[0]
239+ else:
240+ return super(PostgresDisconnectionTest, self).create_proxy(uri)
241+
242 def test_rollback_swallows_InterfaceError(self):
243 """Test that InterfaceErrors get caught on rollback().
244
245@@ -803,6 +822,13 @@
246
247 database_uri = os.environ.get("STORM_POSTGRES_HOST_URI")
248
249+ def setUp(self):
250+ super(PostgresDisconnectionTestWithoutProxyTCPSockets, self).setUp()
251+ if self.database.get_uri().host.startswith("/"):
252+ proxy, proxy_uri = create_proxy_and_uri(self.database.get_uri())
253+ self.addCleanup(proxy.close)
254+ self.database = create_database(proxy_uri)
255+
256
257 class PostgresDisconnectionTestWithPGBouncerBase(object):
258 # Connecting via pgbouncer <http://pgfoundry.org/projects/pgbouncer>
259@@ -816,6 +842,9 @@
260 def setUp(self):
261 super(PostgresDisconnectionTestWithPGBouncerBase, self).setUp()
262 database_uri = URI(os.environ["STORM_POSTGRES_HOST_URI"])
263+ if database_uri.host.startswith("/"):
264+ proxy, database_uri = create_proxy_and_uri(database_uri)
265+ self.addCleanup(proxy.close)
266 database_user = database_uri.username or os.environ['USER']
267 database_dsn = make_dsn(database_uri)
268 # Create a pgbouncer fixture.
269@@ -874,6 +903,10 @@
270 self.tracer.get_remaining_time = lambda: self.remaining_time
271 self.remaining_time = 10.5
272
273+ def tearDown(self):
274+ self.connection.close()
275+ super(PostgresTimeoutTracerTest, self).tearDown()
276+
277 def test_set_statement_timeout(self):
278 result = self.connection.execute("SHOW statement_timeout")
279 self.assertEquals(result.get_one(), ("10500ms",))
280
281=== modified file 'storm/tests/databases/proxy.py'
282--- storm/tests/databases/proxy.py 2019-06-07 17:14:33 +0000
283+++ storm/tests/databases/proxy.py 2019-09-29 14:08:50 +0000
284@@ -22,11 +22,13 @@
285
286 from __future__ import print_function
287
288+import errno
289 import os
290 import select
291 import socket
292 import threading
293
294+import six
295 from six.moves import socketserver
296
297
298@@ -42,7 +44,10 @@
299 self, request, client_address, server)
300
301 def handle(self):
302- dst = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
303+ if isinstance(self.server.proxy_dest, (bytes, six.text_type)):
304+ dst = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
305+ else:
306+ dst = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
307 dst.connect(self.server.proxy_dest)
308
309 readers = [self.request, dst]
310@@ -56,13 +61,23 @@
311
312 if self.request in rlist:
313 chunk = os.read(self.request.fileno(), 1024)
314- dst.send(chunk)
315+ try:
316+ dst.send(chunk)
317+ except socket.error as e:
318+ if e.errno == errno.EPIPE:
319+ return
320+ raise
321 if chunk == "":
322 readers.remove(self.request)
323 dst.shutdown(socket.SHUT_WR)
324
325 if dst in rlist:
326- chunk = os.read(dst.fileno(), 1024)
327+ try:
328+ chunk = os.read(dst.fileno(), 1024)
329+ except OSError as e:
330+ if e.errno == errno.ECONNRESET:
331+ return
332+ raise
333 self.request.send(chunk)
334 if chunk == "":
335 readers.remove(dst)
336
337=== modified file 'storm/tests/mocker.py'
338--- storm/tests/mocker.py 2019-09-17 09:35:10 +0000
339+++ storm/tests/mocker.py 2019-09-29 14:08:50 +0000
340@@ -149,7 +149,7 @@
341 elif os.path.isdir(path):
342 shutil.rmtree(path)
343 self.mocker.restore()
344- for func, args, kwargs in self.__cleanup_funcs:
345+ for func, args, kwargs in reversed(self.__cleanup_funcs):
346 func(*args, **kwargs)
347
348 def addCleanup(self, func, *args, **kwargs):
349
350=== modified file 'test'
351--- test 2019-08-11 17:51:37 +0000
352+++ test 2019-09-29 14:08:50 +0000
353@@ -21,6 +21,7 @@
354 #
355 import glob
356 import optparse
357+import re
358 import unittest
359 import sys
360 import os
361@@ -73,6 +74,43 @@
362 return test_with_runner(runner)
363
364
365+def with_postgresfixture(runner_func):
366+ """If possible, wrap a test runner with code to set up PostgreSQL."""
367+ try:
368+ from postgresfixture import ClusterFixture
369+ except ImportError:
370+ return runner_func
371+
372+ from six.moves.urllib.parse import urlunsplit
373+ from storm.uri import escape
374+
375+ def wrapper():
376+ cluster = ClusterFixture("db")
377+ cluster.create()
378+ # Allow use of prepared transactions, which some tests need.
379+ pg_conf_path = os.path.join(cluster.datadir, "postgresql.conf")
380+ with open(pg_conf_path) as pg_conf_old:
381+ with open(pg_conf_path + ".new", "w") as pg_conf_new:
382+ for line in pg_conf_old:
383+ pg_conf_new.write(re.sub(
384+ r"^#(max_prepared_transactions.*)= 0", r"\1= 200",
385+ line))
386+ os.fchmod(
387+ pg_conf_new.fileno(),
388+ os.fstat(pg_conf_old.fileno()).st_mode)
389+ os.rename(pg_conf_path + ".new", pg_conf_path)
390+
391+ with cluster:
392+ cluster.createdb("storm_test")
393+ uri = urlunsplit(
394+ ("postgres", escape(cluster.datadir), "/storm_test", "", ""))
395+ os.environ["STORM_POSTGRES_URI"] = uri
396+ os.environ["STORM_POSTGRES_HOST_URI"] = uri
397+ return runner_func()
398+
399+ return wrapper
400+
401+
402 if __name__ == "__main__":
403 runner = os.environ.get("STORM_TEST_RUNNER")
404 if not runner:
405@@ -80,6 +118,7 @@
406 runner_func = globals().get("test_with_%s" % runner.replace(".", "_"))
407 if not runner_func:
408 sys.exit("Test runner not found: %s" % runner)
409+ runner_func = with_postgresfixture(runner_func)
410 sys.exit(runner_func())
411
412 # vim:ts=4:sw=4:et
413
414=== modified file 'tox.ini'
415--- tox.ini 2019-09-17 09:45:08 +0000
416+++ tox.ini 2019-09-29 14:08:50 +0000
417@@ -8,8 +8,6 @@
418 .[test]
419 passenv = STORM_TEST_RUNNER USER
420 setenv =
421- STORM_POSTGRES_URI = postgres:storm_test
422- STORM_POSTGRES_HOST_URI = postgres://localhost:5432/storm_test
423 cextensions: STORM_CEXTENSIONS = 1
424 nocextensions: STORM_CEXTENSIONS = 0
425 commands =

Subscribers

People subscribed via source and target branches

to status/vote changes: