launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19539
[Merge] lp:~wgrant/launchpad/test-no-sequence-reset into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/test-no-sequence-reset into lp:launchpad with lp:~wgrant/launchpad/test-no-sequence-deps-translations as a prerequisite.
Commit message:
Don't reset sequences between tests. It's expensive and few tests needed it.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/test-no-sequence-reset/+merge/273388
Don't reset sequences between tests.
It costs about 50ms per test, and only 41 tests actually depended on known sequence values.
Before: DatabaseLayer.testTearDown 10 calls taking 0.6s.
After: DatabaseLayer.testTearDown 10 calls taking negligible time.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/test-no-sequence-reset into lp:launchpad.
=== modified file 'lib/lp/testing/layers.py'
--- lib/lp/testing/layers.py 2013-08-14 11:15:51 +0000
+++ lib/lp/testing/layers.py 2015-10-05 10:08:14 +0000
@@ -719,20 +719,15 @@
@profiled
def setUp(cls):
cls._is_setup = True
- # Read the sequences we'll need from the test template database.
- reset_sequences_sql = LaunchpadTestSetup(
- dbname='launchpad_ftest_template').generateResetSequencesSQL()
# Allocate a template for this test instance
if os.environ.get('LP_TEST_INSTANCE'):
template_name = '_'.join([LaunchpadTestSetup.template,
os.environ.get('LP_TEST_INSTANCE')])
- cls._db_template_fixture = LaunchpadTestSetup(
- dbname=template_name, reset_sequences_sql=reset_sequences_sql)
+ cls._db_template_fixture = LaunchpadTestSetup(dbname=template_name)
cls._db_template_fixture.setUp()
else:
template_name = LaunchpadTestSetup.template
- cls._db_fixture = LaunchpadTestSetup(template=template_name,
- reset_sequences_sql=reset_sequences_sql)
+ cls._db_fixture = LaunchpadTestSetup(template=template_name)
cls.force_dirty_database()
# Nuke any existing DB (for persistent-test-services) [though they
# prevent this !?]
@@ -747,7 +742,7 @@
# point for addressing that mismatch.
cls._db_fixture.tearDown()
# Bring up the db, so that it is available for other layers.
- cls._ensure_db()
+ cls._db_fixture.setUp()
@classmethod
@profiled
@@ -771,28 +766,8 @@
pass
@classmethod
- def _ensure_db(cls):
- cls._db_fixture.setUp()
- # Ensure that the database is connectable. Because we might have
- # just created it, keep trying for a few seconds incase PostgreSQL
- # is taking its time getting its house in order.
- attempts = 60
- for count in range(0, attempts):
- try:
- cls.connect().close()
- except psycopg2.Error:
- if count == attempts - 1:
- raise
- time.sleep(0.5)
- else:
- break
-
- @classmethod
@profiled
def testTearDown(cls):
- # Ensure that the database is connectable
- cls.connect().close()
-
cls._db_fixture.tearDown()
# Fail tests that forget to uninstall their database policies.
@@ -804,7 +779,7 @@
# Reset/bring up the db - makes it available for either the next test,
# or a subordinate layer which builds on the db. This wastes one setup
# per db layer teardown per run, but thats tolerable.
- cls._ensure_db()
+ cls._db_fixture.setUp()
@classmethod
@profiled
=== modified file 'lib/lp/testing/pgsql.py'
--- lib/lp/testing/pgsql.py 2015-10-01 22:36:23 +0000
+++ lib/lp/testing/pgsql.py 2015-10-05 10:08:14 +0000
@@ -19,10 +19,6 @@
from lp.services.config import config
from lp.services.database import activity_cols
-from lp.services.database.postgresql import (
- generateResetSequencesSQL,
- resetSequences,
- )
class ConnectionWrapper:
@@ -179,7 +175,7 @@
_reset_db = True
def __init__(self, template=None, dbname=dynamic, dbuser=None,
- host=None, port=None, reset_sequences_sql=None):
+ host=None, port=None):
'''Construct the PgTestSetup
Note that dbuser is not used for setting up or tearing down
@@ -221,7 +217,6 @@
self.host = host
if port is not None:
self.port = port
- self.reset_sequences_sql = reset_sequences_sql
def _connectionString(self, dbname, dbuser=None):
connection_parameters = ['dbname=%s' % dbname]
@@ -238,15 +233,6 @@
dbname = self.dbname
return psycopg2.connect(self._connectionString(dbname))
- def generateResetSequencesSQL(self):
- """Return a SQL statement that resets all sequences."""
- con = self.superuser_connection()
- cur = con.cursor()
- try:
- return generateResetSequencesSQL(cur)
- finally:
- con.close()
-
def setUp(self):
'''Create a fresh database (dropping the old if necessary)
@@ -257,18 +243,6 @@
if (self.template, self.dbname) != PgTestSetup._last_db:
PgTestSetup._reset_db = True
if not PgTestSetup._reset_db:
- # The database doesn't need to be reset. We reset the sequences
- # anyway (because they might have been incremented even if
- # nothing was committed), making sure not to disturb the
- # 'committed' flag, and we're done.
- con = self.superuser_connection()
- cur = con.cursor()
- if self.reset_sequences_sql is None:
- resetSequences(cur)
- else:
- cur.execute(self.reset_sequences_sql)
- con.commit()
- con.close()
ConnectionWrapper.committed = False
ConnectionWrapper.dirty = False
return
@@ -366,6 +340,7 @@
l.unlock()
if debug:
sys.stderr.write('released %s\n' % (pid,))
+
ConnectionWrapper.committed = False
ConnectionWrapper.dirty = False
PgTestSetup._last_db = (self.template, self.dbname)
=== modified file 'lib/lp/testing/tests/test_pgsql.py'
--- lib/lp/testing/tests/test_pgsql.py 2015-10-01 22:36:23 +0000
+++ lib/lp/testing/tests/test_pgsql.py 2015-10-05 10:08:14 +0000
@@ -119,82 +119,3 @@
con.commit()
finally:
fixture.tearDown()
-
- def test_sequences(self):
- # Sequences may be affected by connections even if the connection
- # is rolled back. So ensure the database is reset fully, in the
- # cases where we just rollback the changes we also need to reset all
- # the sequences.
-
- # Setup a table that uses a sequence
- fixture = PgTestSetup()
- fixture.setUp()
- try:
- con = fixture.connect()
- cur = con.cursor()
- cur.execute('CREATE TABLE foo (x serial, y integer)')
- con.commit()
- con.close()
- # Fake it so the harness doesn't know a change has been made
- ConnectionWrapper.committed = False
- finally:
- fixture.tearDown()
-
- sequence_values = []
- # Insert a row into it and roll back the changes. Each time, we
- # should end up with the same sequence value
- for i in range(3):
- fixture.setUp()
- try:
- con = fixture.connect()
- cur = con.cursor()
- cur.execute('INSERT INTO foo (y) VALUES (1)')
- cur.execute("SELECT currval('foo_x_seq')")
- sequence_values.append(cur.fetchone()[0])
- con.rollback()
- con.close()
- finally:
- fixture.tearDown()
-
- # Fail if we got a diffent sequence value at some point
- for v in sequence_values:
- self.failUnlessEqual(v, sequence_values[0])
-
- # Repeat the test, but this time with some data already in the
- # table
- fixture.setUp()
- try:
- con = fixture.connect()
- cur = con.cursor()
- cur.execute('INSERT INTO foo (y) VALUES (1)')
- con.commit()
- con.close()
- # Fake it so the harness doesn't know a change has been made
- ConnectionWrapper.committed = False
- finally:
- fixture.tearDown()
-
- sequence_values = []
- # Insert a row into it and roll back the changes. Each time, we
- # should end up with the same sequence value
- for i in range(1, 3):
- fixture.setUp()
- try:
- con = fixture.connect()
- cur = con.cursor()
- cur.execute('INSERT INTO foo (y) VALUES (1)')
- cur.execute("SELECT currval('foo_x_seq')")
- sequence_values.append(cur.fetchone()[0])
- con.rollback()
- con.close()
- finally:
- fixture.tearDown()
-
- # Fail if we got a diffent sequence value at some point
- for v in sequence_values:
- self.failUnlessEqual(v, sequence_values[0])
-
- # The database still exists because, after the last commit,
- # ConnectionWrapper.committed was set to False.
- # Drop the database here to avoid test ordering issues.
- fixture.dropDb()
References
