← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/python-pgbouncer/tox into lp:python-pgbouncer

 

Colin Watson has proposed merging lp:~cjwatson/python-pgbouncer/tox into lp:python-pgbouncer.

Commit message:
Use postgresfixture to test against multiple PostgreSQL versions, and switch to tox.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1002315 in Python PGBouncer: "Use postgresfixture for testing."
  https://bugs.launchpad.net/python-pgbouncer/+bug/1002315
  Bug #1036876 in Python PGBouncer: "pgbouncer/tests.py demands PostgreSQL 8.4"
  https://bugs.launchpad.net/python-pgbouncer/+bug/1036876

For more details, see:
https://code.launchpad.net/~cjwatson/python-pgbouncer/tox/+merge/368549

Regarding postgresfixture, I sacrificed the clever inter-test resource management as part of this work.  It might be possible to make it work again, but the test suite isn't all that slow, and this at least seems pretty robust.

Regarding tox, I generally can't get buildout working these days, not least because of bad interactions with pbr, and so have been progressively converting packages to tox as I touch them.  This also makes it easier to set up test matrixes that test against things like multiple Python versions.

These should really be two separate MPs, but I couldn't get tox working without doing something about the PostgreSQL test setup, and I couldn't get tests running at all without switching to tox, so they ended up a bit intertwined.  Hopefully this isn't too hard to follow.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/python-pgbouncer/tox into lp:python-pgbouncer.
=== modified file '.bzrignore'
--- .bzrignore	2011-10-25 20:03:51 +0000
+++ .bzrignore	2019-06-07 15:00:45 +0000
@@ -1,10 +1,5 @@
-./eggs/*
-./.installed.cfg
-./develop-eggs
-./bin
+./.tox
+./MANIFEST
 ./pgbouncer.egg-info
-./parts
-./eggs
-./download-cache
 TAGS
 tags

=== modified file 'NEWS.txt'
--- NEWS.txt	2012-08-20 16:39:41 +0000
+++ NEWS.txt	2019-06-07 15:00:45 +0000
@@ -1,3 +1,9 @@
+0.0.9
+=====
+
+- Use postgresfixture to test against multiple PostgreSQL versions.
+- Switch to tox.
+
 0.0.8 (2012-08-20)
 ==================
 

=== modified file 'README'
--- README	2011-10-25 20:03:29 +0000
+++ README	2019-06-07 15:00:45 +0000
@@ -28,22 +28,22 @@
 * pgbouncer
 
 * python-fixtures (https://launchpad.net/python-fixtures or
-  http://pypi.python.org/pypi/fixtures)
+  https://pypi.org/project/fixtures)
 
-* testtools (http://pypi.python.org/pypi/testtools)
+* testtools (https://pypi.org/project/testtools)
 
 Testing Dependencies
 ====================
 
 In addition to the above, the tests also depend on:
 
-* psycopg2 (http://pypi.python.org/pypi/psycopg2)
-
-* subunit (http://pypi.python.org/pypi/python-subunit) (optional)
-
-* testresources (http://pypi.python.org/pypi/testresources)
-
-* van.pg (http://pypi.python.org/pypi/van.pg)
+* postgresfixture (https://pypi.org/project/postgresfixture)
+
+* psycopg2 (https://pypi.org/project/psycopg2)
+
+* subunit (https://pypi.org/project/python-subunit) (optional)
+
+* testscenarios (https://pypi.org/project/testscenarios)
 
 Usage
 =====
@@ -77,14 +77,7 @@
 ===========
 
 Upstream development takes place at https://launchpad.net/python-pgbouncer.
-To setup a working area for development, if the dependencies are not
-immediately available, you can use ./bootstrap.py to create bin/buildout, then
-bin/py to get a python interpreter with the dependencies available.
-
-To run the tests, run either:
-
- $ bin/py pgbouncer/tests.py
-
-or:
-
- $ bin/py -m testtools.run pgbouncer.tests.test_suite
+
+To run the tests, run:
+
+ $ tox

=== removed file 'bootstrap.py'
--- bootstrap.py	2011-07-18 03:31:27 +0000
+++ bootstrap.py	1970-01-01 00:00:00 +0000
@@ -1,259 +0,0 @@
-#!/usr/bin/env python
-##############################################################################
-#
-# Copyright (c) 2006 Zope Foundation and Contributors.
-# All Rights Reserved.
-#
-# This software is subject to the provisions of the Zope Public License,
-# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
-# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
-# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
-# FOR A PARTICULAR PURPOSE.
-#
-##############################################################################
-"""Bootstrap a buildout-based project
-
-Simply run this script in a directory containing a buildout.cfg.
-The script accepts buildout command-line options, so you can
-use the -c option to specify an alternate configuration file.
-"""
-
-import os, shutil, sys, tempfile, textwrap, urllib, urllib2, subprocess
-from optparse import OptionParser
-
-if sys.platform == 'win32':
-    def quote(c):
-        if ' ' in c:
-            return '"%s"' % c # work around spawn lamosity on windows
-        else:
-            return c
-else:
-    quote = str
-
-# See zc.buildout.easy_install._has_broken_dash_S for motivation and comments.
-stdout, stderr = subprocess.Popen(
-    [sys.executable, '-Sc',
-     'try:\n'
-     '    import ConfigParser\n'
-     'except ImportError:\n'
-     '    print 1\n'
-     'else:\n'
-     '    print 0\n'],
-    stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()
-has_broken_dash_S = bool(int(stdout.strip()))
-
-# In order to be more robust in the face of system Pythons, we want to
-# run without site-packages loaded.  This is somewhat tricky, in
-# particular because Python 2.6's distutils imports site, so starting
-# with the -S flag is not sufficient.  However, we'll start with that:
-if not has_broken_dash_S and 'site' in sys.modules:
-    # We will restart with python -S.
-    args = sys.argv[:]
-    args[0:0] = [sys.executable, '-S']
-    args = map(quote, args)
-    os.execv(sys.executable, args)
-# Now we are running with -S.  We'll get the clean sys.path, import site
-# because distutils will do it later, and then reset the path and clean
-# out any namespace packages from site-packages that might have been
-# loaded by .pth files.
-clean_path = sys.path[:]
-import site
-sys.path[:] = clean_path
-for k, v in sys.modules.items():
-    if (hasattr(v, '__path__') and
-        len(v.__path__)==1 and
-        not os.path.exists(os.path.join(v.__path__[0],'__init__.py'))):
-        # This is a namespace package.  Remove it.
-        sys.modules.pop(k)
-
-is_jython = sys.platform.startswith('java')
-
-setuptools_source = 'http://peak.telecommunity.com/dist/ez_setup.py'
-distribute_source = 'http://python-distribute.org/distribute_setup.py'
-
-# parsing arguments
-def normalize_to_url(option, opt_str, value, parser):
-    if value:
-        if '://' not in value: # It doesn't smell like a URL.
-            value = 'file://%s' % (
-                urllib.pathname2url(
-                    os.path.abspath(os.path.expanduser(value))),)
-        if opt_str == '--download-base' and not value.endswith('/'):
-            # Download base needs a trailing slash to make the world happy.
-            value += '/'
-    else:
-        value = None
-    name = opt_str[2:].replace('-', '_')
-    setattr(parser.values, name, value)
-
-usage = '''\
-[DESIRED PYTHON FOR BUILDOUT] bootstrap.py [options]
-
-Bootstraps a buildout-based project.
-
-Simply run this script in a directory containing a buildout.cfg, using the
-Python that you want bin/buildout to use.
-
-Note that by using --setup-source and --download-base to point to
-local resources, you can keep this script from going over the network.
-'''
-
-parser = OptionParser(usage=usage)
-parser.add_option("-v", "--version", dest="version",
-                          help="use a specific zc.buildout version")
-parser.add_option("-d", "--distribute",
-                   action="store_true", dest="use_distribute", default=False,
-                   help="Use Distribute rather than Setuptools.")
-parser.add_option("--setup-source", action="callback", dest="setup_source",
-                  callback=normalize_to_url, nargs=1, type="string",
-                  help=("Specify a URL or file location for the setup file. "
-                        "If you use Setuptools, this will default to " +
-                        setuptools_source + "; if you use Distribute, this "
-                        "will default to " + distribute_source +"."))
-parser.add_option("--download-base", action="callback", dest="download_base",
-                  callback=normalize_to_url, nargs=1, type="string",
-                  help=("Specify a URL or directory for downloading "
-                        "zc.buildout and either Setuptools or Distribute. "
-                        "Defaults to PyPI."))
-parser.add_option("--eggs",
-                  help=("Specify a directory for storing eggs.  Defaults to "
-                        "a temporary directory that is deleted when the "
-                        "bootstrap script completes."))
-parser.add_option("-t", "--accept-buildout-test-releases",
-                  dest='accept_buildout_test_releases',
-                  action="store_true", default=False,
-                  help=("Normally, if you do not specify a --version, the "
-                        "bootstrap script and buildout gets the newest "
-                        "*final* versions of zc.buildout and its recipes and "
-                        "extensions for you.  If you use this flag, "
-                        "bootstrap and buildout will get the newest releases "
-                        "even if they are alphas or betas."))
-parser.add_option("-c", None, action="store", dest="config_file",
-                   help=("Specify the path to the buildout configuration "
-                         "file to be used."))
-
-options, args = parser.parse_args()
-
-# if -c was provided, we push it back into args for buildout's main function
-if options.config_file is not None:
-    args += ['-c', options.config_file]
-
-if options.eggs:
-    eggs_dir = os.path.abspath(os.path.expanduser(options.eggs))
-else:
-    eggs_dir = tempfile.mkdtemp()
-
-if options.setup_source is None:
-    if options.use_distribute:
-        options.setup_source = distribute_source
-    else:
-        options.setup_source = setuptools_source
-
-if options.accept_buildout_test_releases:
-    args.append('buildout:accept-buildout-test-releases=true')
-args.append('bootstrap')
-
-try:
-    import pkg_resources
-    import setuptools # A flag.  Sometimes pkg_resources is installed alone.
-    if not hasattr(pkg_resources, '_distribute'):
-        raise ImportError
-except ImportError:
-    ez_code = urllib2.urlopen(
-        options.setup_source).read().replace('\r\n', '\n')
-    ez = {}
-    exec ez_code in ez
-    setup_args = dict(to_dir=eggs_dir, download_delay=0)
-    if options.download_base:
-        setup_args['download_base'] = options.download_base
-    if options.use_distribute:
-        setup_args['no_fake'] = True
-    ez['use_setuptools'](**setup_args)
-    reload(sys.modules['pkg_resources'])
-    import pkg_resources
-    # This does not (always?) update the default working set.  We will
-    # do it.
-    for path in sys.path:
-        if path not in pkg_resources.working_set.entries:
-            pkg_resources.working_set.add_entry(path)
-
-cmd = [quote(sys.executable),
-       '-c',
-       quote('from setuptools.command.easy_install import main; main()'),
-       '-mqNxd',
-       quote(eggs_dir)]
-
-if not has_broken_dash_S:
-    cmd.insert(1, '-S')
-
-find_links = options.download_base
-if not find_links:
-    find_links = os.environ.get('bootstrap-testing-find-links')
-if find_links:
-    cmd.extend(['-f', quote(find_links)])
-
-if options.use_distribute:
-    setup_requirement = 'distribute'
-else:
-    setup_requirement = 'setuptools'
-ws = pkg_resources.working_set
-setup_requirement_path = ws.find(
-    pkg_resources.Requirement.parse(setup_requirement)).location
-env = dict(
-    os.environ,
-    PYTHONPATH=setup_requirement_path)
-
-requirement = 'zc.buildout'
-version = options.version
-if version is None and not options.accept_buildout_test_releases:
-    # Figure out the most recent final version of zc.buildout.
-    import setuptools.package_index
-    _final_parts = '*final-', '*final'
-    def _final_version(parsed_version):
-        for part in parsed_version:
-            if (part[:1] == '*') and (part not in _final_parts):
-                return False
-        return True
-    index = setuptools.package_index.PackageIndex(
-        search_path=[setup_requirement_path])
-    if find_links:
-        index.add_find_links((find_links,))
-    req = pkg_resources.Requirement.parse(requirement)
-    if index.obtain(req) is not None:
-        best = []
-        bestv = None
-        for dist in index[req.project_name]:
-            distv = dist.parsed_version
-            if _final_version(distv):
-                if bestv is None or distv > bestv:
-                    best = [dist]
-                    bestv = distv
-                elif distv == bestv:
-                    best.append(dist)
-        if best:
-            best.sort()
-            version = best[-1].version
-if version:
-    requirement = '=='.join((requirement, version))
-cmd.append(requirement)
-
-if is_jython:
-    import subprocess
-    exitcode = subprocess.Popen(cmd, env=env).wait()
-else: # Windows prefers this, apparently; otherwise we would prefer subprocess
-    exitcode = os.spawnle(*([os.P_WAIT, sys.executable] + cmd + [env]))
-if exitcode != 0:
-    sys.stdout.flush()
-    sys.stderr.flush()
-    print ("An error occurred when trying to install zc.buildout. "
-           "Look above this message for any errors that "
-           "were output by easy_install.")
-    sys.exit(exitcode)
-
-ws.add_entry(eggs_dir)
-ws.require(requirement)
-import zc.buildout.buildout
-zc.buildout.buildout.main(args)
-if not options.eggs: # clean up temporary egg directory
-    shutil.rmtree(eggs_dir)

=== removed file 'buildout.cfg'
--- buildout.cfg	2012-08-20 16:27:22 +0000
+++ buildout.cfg	1970-01-01 00:00:00 +0000
@@ -1,35 +0,0 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-[buildout]
-parts =
-    scripts
-unzip = true
-eggs-directory = eggs
-download-cache = download-cache
-relative-paths = true
-
-# Disable this option temporarily if you want buildout to find software
-# dependencies *other* than those in our download-cache.  Once you have the
-# desired software, reenable this option (and check in the new software to
-# lp:lp-source-dependencies if this is going to be reviewed/merged/deployed.)
-install-from-cache = true
-
-# This also will need to be temporarily disabled or changed for package
-# upgrades.  Newly-added packages should also add their desired version number
-# to versions.cfg.
-extends = versions.cfg
-
-allow-picked-versions = false
-
-prefer-final = true
-
-develop = .
-
-[scripts]
-recipe = z3c.recipe.scripts
-eggs = pgbouncer [test]
-include-site-packages = true
-allowed-eggs-from-site-packages = 
-    subunit
-interpreter = py

=== modified file 'pgbouncer/tests.py'
--- pgbouncer/tests.py	2011-10-28 11:45:32 +0000
+++ pgbouncer/tests.py	2019-06-07 15:00:45 +0000
@@ -14,51 +14,42 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
-import unittest
+from contextlib import closing
 
 import fixtures
+from postgresfixture import ClusterFixture
+from postgresfixture.cluster import PG_VERSIONS
 import psycopg2
-from van.pg import DatabaseManager
-import testresources
+import testscenarios
 import testtools
 
 from pgbouncer.fixture import PGBouncerFixture
 
-# A 'just-works' workaround for Ubuntu not exposing initdb to the main PATH.
-os.environ['PATH'] = os.environ['PATH'] + ':/usr/lib/postgresql/8.4/bin'
-
-
-def test_suite():
-    loader = testresources.TestLoader()
-    return loader.loadTestsFromName(__name__)
-
-
-class ResourcedTestCase(testtools.TestCase, testresources.ResourcedTestCase):
-    """Mix together testtools and testresources."""
-
-
-def setup_user(db):
-    conn = psycopg2.connect(host=db.host, database=db.database)
-    conn.cursor().execute('CREATE USER user1')
-    conn.commit()
-    conn.close()
-
-
-class TestFixture(ResourcedTestCase):
-
-    resources = [('db', DatabaseManager(initialize_sql=setup_user))]
+
+class TestFixture(testscenarios.WithScenarios, testtools.TestCase):
+
+    scenarios = sorted(
+        (version, {'version': version}) for version in PG_VERSIONS)
 
     def setUp(self):
         super(TestFixture, self).setUp()
+        datadir = self.useFixture(fixtures.TempDir()).path
+        self.dbname = 'test_pgbouncer'
+        self.cluster = self.useFixture(
+            ClusterFixture(datadir, version=self.version))
+        self.cluster.createdb(self.dbname)
+        with closing(self.cluster.connect()) as conn:
+            with closing(conn.cursor()) as cur:
+                cur.execute('DROP USER IF EXISTS user1')
+                cur.execute('CREATE USER user1')
         self.bouncer = PGBouncerFixture()
-        self.bouncer.databases[self.db.database] = 'host=' + self.db.host
+        self.bouncer.databases[self.dbname] = 'host=' + datadir
         self.bouncer.users['user1'] = ''
 
     def connect(self, host=None):
         return psycopg2.connect(
             host=(self.bouncer.host if host is None else host),
-            port=self.bouncer.port, database=self.db.database,
+            port=self.bouncer.port, database=self.dbname,
             user='user1')
 
     def test_dynamic_port_allocation(self):
@@ -99,8 +90,3 @@
         bouncer_pid = self.bouncer.process.pid
         self.bouncer.start()
         self.assertEqual(bouncer_pid, self.bouncer.process.pid)
-
-
-if __name__ == "__main__":
-    loader = testresources.TestLoader()
-    unittest.main(testLoader=loader)

=== modified file 'setup.py'
--- setup.py	2012-08-20 16:39:41 +0000
+++ setup.py	2019-06-07 15:00:45 +0000
@@ -14,12 +14,13 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-  
+
 
 from distutils.core import setup
 import os.path
 
-description = file(os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
+with open(os.path.join(os.path.dirname(__file__), 'README')) as f:
+    description = f.read()
 
 setup(name="pgbouncer",
       version="0.0.8",
@@ -43,8 +44,9 @@
           ],
       extras_require = dict(
           test=[
+              'postgresfixture',
+              'testscenarios',
               'testtools',
-              'van.pg',
               ]
           ),
       )

=== added file 'tox.ini'
--- tox.ini	1970-01-01 00:00:00 +0000
+++ tox.ini	2019-06-07 15:00:45 +0000
@@ -0,0 +1,8 @@
+[tox]
+envlist =
+    py27
+
+[testenv]
+deps =
+    .[test]
+commands = python -m testtools.run discover -s pgbouncer {posargs}

=== removed file 'versions.cfg'
--- versions.cfg	2012-08-20 16:27:22 +0000
+++ versions.cfg	1970-01-01 00:00:00 +0000
@@ -1,15 +0,0 @@
-[buildout]
-versions = versions
-
-[versions]
-fixtures = 0.3.6
-psycopg2 = 2.4.5
-pytz = 2012d
-setuptools = 0.6c11
-testresources = 0.2.4
-testtools = 0.9.11
-van.pg = 1.4
-z3c.recipe.filetemplate = 2.1.0
-z3c.recipe.scripts = 1.0.1
-zc.buildout = 1.5.1
-zc.recipe.egg = 1.3.2