Merge lp:~allenap/postgresfixture/multi-version into lp:~lazr-developers/postgresfixture/trunk

[Gavin Panella]

Work with multiple version of PostgreSQL.

Previously only 9.1 was supported.

Get postgresfixture working with the most up-to-date PostgreSQL installation on a machine, by default, and allow selection of a different one.

[Graham Binns (gmb)]

Sweet. Imagine that, people hard coding dependency versions. You'd never get that at a company like Canonical.

Oh, wait...

review: Approve

[lp:~allenap/postgresfixture/multi-version]

10. By Gavin Panella on 2014-01-17

Use LooseVersion instead of float when comparing PostgreSQL versions.

=== modified file 'postgresfixture/cluster.py'
--- postgresfixture/cluster.py	2012-07-16 20:00:28 +0000
+++ postgresfixture/cluster.py	2014-01-17 14:25:09 +0000
@@ -12,18 +12,22 @@
 __metaclass__ = type
 __all__ = [
     "Cluster",
+    "PG_VERSION_MAX",
+    "PG_VERSIONS",
     ]
 
 from contextlib import (
     closing,
     contextmanager,
     )
+from distutils.version import LooseVersion
 from fcntl import (
     LOCK_EX,
     LOCK_UN,
     lockf,
     )
 from functools import wraps
+from glob import iglob
 from os import (
     devnull,
     environ,
@@ -40,18 +44,32 @@
 import psycopg2
 
 
-PG_VERSION = "9.1"
-PG_BIN = "/usr/lib/postgresql/%s/bin" % PG_VERSION
-
-
-def path_with_pg_bin(exe_path):
-    """Return `exe_path` with `PG_BIN` added."""
+PG_BASE = "/usr/lib/postgresql"
+
+PG_VERSION_BINS = {
+    path.basename(pgdir): path.join(pgdir, "bin")
+    for pgdir in iglob(path.join(PG_BASE, "*"))
+    if path.exists(path.join(pgdir, "bin", "pg_ctl"))
+}
+
+PG_VERSION_MAX = max(PG_VERSION_BINS, key=LooseVersion)
+PG_VERSIONS = sorted(PG_VERSION_BINS, key=LooseVersion)
+
+
+def get_pg_bin(version):
+    """Return the PostgreSQL ``bin`` directory for the given `version`."""
+    return PG_VERSION_BINS[version]
+
+
+def path_with_pg_bin(exe_path, version):
+    """Return `exe_path` with the PostgreSQL ``bin`` directory added."""
     exe_path = [
         elem for elem in exe_path.split(path.pathsep)
         if len(elem) != 0 and not elem.isspace()
         ]
-    if PG_BIN not in exe_path:
-        exe_path.insert(0, PG_BIN)
+    pg_bin = get_pg_bin(version)
+    if pg_bin not in exe_path:
+        exe_path.insert(0, pg_bin)
     return path.pathsep.join(exe_path)
 
 
@@ -67,8 +85,9 @@
 class Cluster:
     """Represents a PostgreSQL cluster, running or not."""
 
-    def __init__(self, datadir):
+    def __init__(self, datadir, version=PG_VERSION_MAX):
         self.datadir = path.abspath(datadir)
+        self.version = version
         self.lockfile = path.join(
             path.dirname(self.datadir),
             ".%s.lock" % path.basename(self.datadir))
@@ -92,7 +111,7 @@
     def execute(self, *command, **options):
         """Execute a command with an environment suitable for this cluster."""
         env = options.pop("env", environ).copy()
-        env["PATH"] = path_with_pg_bin(env.get("PATH", ""))
+        env["PATH"] = path_with_pg_bin(env.get("PATH", ""), self.version)
         env["PGDATA"] = env["PGHOST"] = self.datadir
         check_call(command, env=env, **options)
 
@@ -121,7 +140,7 @@
         with open(devnull, "wb") as null:
             try:
                 self.execute("pg_ctl", "status", stdout=null)
-            except CalledProcessError, error:
+            except CalledProcessError as error:
                 if error.returncode == 1:
                     return False
                 else:

=== modified file 'postgresfixture/clusterfixture.py'
--- postgresfixture/clusterfixture.py	2012-05-22 16:15:44 +0000
+++ postgresfixture/clusterfixture.py	2014-01-17 14:25:09 +0000
@@ -29,7 +29,10 @@
     )
 
 from fixtures import Fixture
-from postgresfixture.cluster import Cluster
+from postgresfixture.cluster import (
+    Cluster,
+    PG_VERSION_MAX,
+    )
 
 
 class ProcessSemaphore:
@@ -49,7 +52,7 @@
     def acquire(self):
         try:
             makedirs(self.lockdir)
-        except OSError, error:
+        except OSError as error:
             if error.errno != EEXIST:
                 raise
         open(self.lockfile, "w").close()
@@ -57,7 +60,7 @@
     def release(self):
         try:
             unlink(self.lockfile)
-        except OSError, error:
+        except OSError as error:
             if error.errno != ENOENT:
                 raise
 
@@ -65,7 +68,7 @@
     def locked(self):
         try:
             rmdir(self.lockdir)
-        except OSError, error:
+        except OSError as error:
             if error.errno == ENOTEMPTY:
                 return True
             elif error.errno == ENOENT:
@@ -82,7 +85,7 @@
                 int(name) if name.isdigit() else name
                 for name in listdir(self.lockdir)
                 ]
-        except OSError, error:
+        except OSError as error:
             if error.errno == ENOENT:
                 return []
             else:
@@ -92,12 +95,12 @@
 class ClusterFixture(Cluster, Fixture):
     """A fixture for a `Cluster`."""
 
-    def __init__(self, datadir, preserve=False):
+    def __init__(self, datadir, preserve=False, version=PG_VERSION_MAX):
         """
         @param preserve: Leave the cluster and its databases behind, even if
             this fixture creates them.
         """
-        super(ClusterFixture, self).__init__(datadir)
+        super(ClusterFixture, self).__init__(datadir, version=version)
         self.preserve = preserve
         self.shares = ProcessSemaphore(
             path.join(self.datadir, "shares"))

=== modified file 'postgresfixture/main.py'
--- postgresfixture/main.py	2012-05-22 21:51:08 +0000
+++ postgresfixture/main.py	2014-01-17 14:25:09 +0000
@@ -26,6 +26,10 @@
 import sys
 from time import sleep
 
+from postgresfixture.cluster import (
+    PG_VERSION_MAX,
+    PG_VERSIONS,
+    )
 from postgresfixture.clusterfixture import ClusterFixture
 
 
@@ -142,6 +146,10 @@
         "preserve the cluster and its databases when exiting, "
         "even if it was necessary to create and start it "
         "(default: %(default)s)"))
+argument_parser.add_argument(
+    "--version", dest="version", choices=PG_VERSIONS,
+    default=PG_VERSION_MAX, help=(
+        "The version of PostgreSQL to use (default: %(default)s)"))
 argument_subparsers = argument_parser.add_subparsers(
     title="actions")
 
@@ -191,9 +199,9 @@
         setup()
         cluster = ClusterFixture(
             datadir=args.datadir,
-            preserve=args.preserve)
+            preserve=args.preserve, version=args.version)
         args.handler(cluster, args)
-    except CalledProcessError, error:
+    except CalledProcessError as error:
         raise SystemExit(error.returncode)
     except KeyboardInterrupt:
         pass

=== modified file 'postgresfixture/tests/test_cluster.py'
--- postgresfixture/tests/test_cluster.py	2012-05-22 22:27:47 +0000
+++ postgresfixture/tests/test_cluster.py	2014-01-17 14:25:09 +0000
@@ -28,11 +28,13 @@
 import postgresfixture.cluster
 from postgresfixture.cluster import (
     Cluster,
+    get_pg_bin,
     path_with_pg_bin,
-    PG_BIN,
+    PG_VERSIONS,
     )
 from postgresfixture.main import repr_pid
 from postgresfixture.testing import TestCase
+from testscenarios import WithScenarios
 from testtools.content import text_content
 from testtools.matchers import (
     DirExists,
@@ -42,13 +44,19 @@
     )
 
 
-class TestFunctions(TestCase):
+class TestFunctions(WithScenarios, TestCase):
+
+    scenarios = sorted(
+        (version, {"version": version})
+        for version in PG_VERSIONS
+    )
 
     def test_path_with_pg_bin(self):
-        self.assertEqual(PG_BIN, path_with_pg_bin(""))
+        pg_bin = get_pg_bin(self.version)
+        self.assertEqual(pg_bin, path_with_pg_bin("", self.version))
         self.assertEqual(
-            PG_BIN + path.pathsep + "/bin:/usr/bin",
-            path_with_pg_bin("/bin:/usr/bin"))
+            pg_bin + path.pathsep + "/bin:/usr/bin",
+            path_with_pg_bin("/bin:/usr/bin", self.version))
 
     def test_repr_pid_not_a_number(self):
         self.assertEqual("alice", repr_pid("alice"))
@@ -62,9 +70,16 @@
         self.assertThat(repr_pid(pid), StartsWith("%d (" % pid))
 
 
-class TestCluster(TestCase):
-
-    make = Cluster
+class TestCluster(WithScenarios, TestCase):
+
+    scenarios = sorted(
+        (version, {"version": version})
+        for version in PG_VERSIONS
+    )
+
+    def make(self, *args, **kwargs):
+        kwargs.setdefault("version", self.version)
+        return Cluster(*args, **kwargs)
 
     def test_init(self):
         # The datadir passed into the Cluster constructor is resolved to an
@@ -133,7 +148,7 @@
         self.assertEqual(cluster.datadir, env.get("PGHOST"))
         self.assertThat(
             env.get("PATH", ""),
-            StartsWith(PG_BIN + path.pathsep))
+            StartsWith(get_pg_bin(self.version) + path.pathsep))
 
     def test_exists(self):
         cluster = self.make(self.make_dir())

=== modified file 'postgresfixture/tests/test_main.py'
--- postgresfixture/tests/test_main.py	2012-05-22 16:15:44 +0000
+++ postgresfixture/tests/test_main.py	2014-01-17 14:25:09 +0000
@@ -19,6 +19,10 @@
 
 from fixtures import EnvironmentVariableFixture
 from postgresfixture import main
+from postgresfixture.cluster import (
+    PG_VERSION_MAX,
+    PG_VERSIONS,
+    )
 from postgresfixture.clusterfixture import ClusterFixture
 from postgresfixture.testing import TestCase
 from testtools.matchers import StartsWith
@@ -35,7 +39,7 @@
     def parse_args(self, *args):
         try:
             return main.argument_parser.parse_args(args)
-        except SystemExit, error:
+        except SystemExit as error:
             self.fail("parse_args%r failed with %r" % (args, error))
 
     def test_run(self):
@@ -174,3 +178,54 @@
             sys.stderr.getvalue(), StartsWith(
                 "%s: cluster is locked by:" % cluster.datadir))
         self.assertTrue(cluster.exists)
+
+
+class TestVersion(TestCase):
+
+    def patch_pg_versions(self, versions):
+        PG_VERSIONS[:] = versions
+
+    def test_uses_supplied_version(self):
+        # Reset PG_VERSIONS after the test has run.
+        self.addCleanup(self.patch_pg_versions, list(PG_VERSIONS))
+        self.patch_pg_versions(["1.1", "2.2", "3.3"])
+
+        # Record calls to our patched handler.
+        handler_calls = []
+
+        def handler(cluster, args):
+            handler_calls.append((cluster, args))
+
+        self.patch(
+            main.get_action("status"), "_defaults",
+            {"handler": handler})
+
+        # Prevent main() from altering terminal settings.
+        self.patch(main, "setup", lambda: None)
+
+        # The version chosen is picked up by the argument parser and
+        # passed into the Cluster constructor.
+        main.main(["--version", "2.2", "status"])
+        self.assertEqual(
+            [("2.2", "2.2")],
+            [(cluster.version, args.version)
+             for (cluster, args) in handler_calls])
+
+    def test_uses_default_version(self):
+        # Record calls to our patched handler.
+        handler_calls = []
+
+        def handler(cluster, args):
+            handler_calls.append((cluster, args))
+
+        self.patch(
+            main.get_action("status"), "_defaults",
+            {"handler": handler})
+
+        # The argument parser has the default version and passes it into
+        # the Cluster constructor.
+        main.main(["status"])
+        self.assertEqual(
+            [(PG_VERSION_MAX, PG_VERSION_MAX)],
+            [(cluster.version, args.version)
+             for (cluster, args) in handler_calls])

=== modified file 'requirements.txt'
--- requirements.txt	2012-05-21 11:13:40 +0000
+++ requirements.txt	2014-01-17 14:25:09 +0000
@@ -1,3 +1,4 @@
 fixtures >= 0.3.8
 psycopg2 >= 2.4.4
 testtools >= 0.9.14
+testscenarios >= 0.4