Very nearly there with PostgreSQL support for testing the beta2 migration.

- Improve migration logging
- Disable pre_reset() and post_reset() on migrations, which might need to be
  re-enabled for SQLite support.
- Be sure to record the migration version in PostgreSQL.
- Parameterize the Error that will occur.
- Better sample data for PostgreSQL and SQLite, which have different formats.

8 files changed, 205 insertions, 62 deletions

diff --git a/src/mailman/database/base.py b/src/mailman/database/base.py
index f674c9a16..af035914b 100644
--- a/src/mailman/database/base.py
+++ b/src/mailman/database/base.py
@@ -172,9 +172,12 @@ class StormBaseDatabase:
             parts = module_fn.split('_')
             if len(parts) < 2:
                 continue
-            version = parts[1]
-            if len(version.strip()) == 0 or version in versions:
-                # This one is already loaded.
+            version = parts[1].strip()
+            if len(version) == 0:
+                # Not a schema migration file.
+                continue
+            if version in versions:
+                log.debug('already migrated to %s', version)
                 continue
             if until is not None and version > until:
                 # We're done.
@@ -184,6 +187,7 @@ class StormBaseDatabase:
             upgrade = getattr(sys.modules[module_path], 'upgrade', None)
             if upgrade is None:
                 continue
+            log.debug('migrating db to %s: %s', version, module_path)
             upgrade(self, self.store, version, module_path)
 
     def load_sql(self, store, sql):
diff --git a/src/mailman/database/postgresql.py b/src/mailman/database/postgresql.py
index 14855c3f3..879594459 100644
--- a/src/mailman/database/postgresql.py
+++ b/src/mailman/database/postgresql.py
@@ -25,6 +25,8 @@ __all__ = [
     ]
 
 
+import psycopg2
+
 from operator import attrgetter
 from urlparse import urlsplit, urlunsplit
 
@@ -39,8 +41,9 @@ class _TemporaryDB:
         self.database = database
 
     def cleanup(self):
-        self.database.execute('ROLLBACK TO SAVEPOINT testing;')
-        self.database.execute('DROP DATABASE mmtest;')
+        self.database.store.execute('ABORT;')
+        self.database.store.close()
+        config.db.store.execute('DROP DATABASE mmtest;')
 
 
 
@@ -48,6 +51,7 @@ class PostgreSQLDatabase(StormBaseDatabase):
     """Database class for PostgreSQL."""
 
     TAG = 'postgres'
+    Error = psycopg2.ProgrammingError
 
     def _database_exists(self, store):
         """See `BaseDatabase`."""
@@ -85,9 +89,13 @@ class PostgreSQLDatabase(StormBaseDatabase):
         new_parts = list(parts)
         new_parts[2] = '/mmtest'
         url = urlunsplit(new_parts)
+        # Use the existing database connection to create a new testing
+        # database.  Create a savepoint, which will make it easy to reset
+        # after the test.
+        config.db.store.execute('ABORT;')
+        config.db.store.execute('CREATE DATABASE mmtest;')
+        # Now create a new, temporary database.
         database = PostgreSQLDatabase()
-        database.store.execute('SAVEPOINT testing;')
-        database.store.execute('CREATE DATABASE mmtest;')
         with configuration('database', url=url):
             database.initialize()
         return _TemporaryDB(database)
diff --git a/src/mailman/database/schema/mm_00000000000000_base.py b/src/mailman/database/schema/mm_00000000000000_base.py
index ef448d620..f98b30a8d 100644
--- a/src/mailman/database/schema/mm_00000000000000_base.py
+++ b/src/mailman/database/schema/mm_00000000000000_base.py
@@ -38,11 +38,11 @@ def upgrade(database, store, version, module_path):
 
 
 
-def pre_reset(store):
-    global _helper
-    from mailman.testing.database import ResetHelper
-    _helper = ResetHelper(VERSION, store)
+## def pre_reset(store):
+##     global _helper
+##     from mailman.testing.database import ResetHelper
+##     _helper = ResetHelper(VERSION, store)
 
 
-def post_reset(store):
-    _helper.restore(store)
+## def post_reset(store):
+##     _helper.restore(store)
diff --git a/src/mailman/database/schema/mm_20120407000000.py b/src/mailman/database/schema/mm_20120407000000.py
index ad0515a5d..017141683 100644
--- a/src/mailman/database/schema/mm_20120407000000.py
+++ b/src/mailman/database/schema/mm_20120407000000.py
@@ -125,14 +125,16 @@ def upgrade_postgres(database, store, version, module_path):
     # Now drop the old columns.
     store.execute('ALTER TABLE mailinglist DROP COLUMN archive;')
     store.execute('ALTER TABLE mailinglist DROP COLUMN archive_private;')
+    # Record the migration in the version table.
+    database.load_schema(store, version, None, module_path)
 
 
 
-def pre_reset(store):
-    global _helper
-    from mailman.testing.database import ResetHelper
-    _helper = ResetHelper(VERSION, store)
+## def pre_reset(store):
+##     global _helper
+##     from mailman.testing.database import ResetHelper
+##     _helper = ResetHelper(VERSION, store)
 
 
-def post_reset(store):
-    _helper.restore(store)
+## def post_reset(store):
+##     _helper.restore(store)
diff --git a/src/mailman/database/sqlite.py b/src/mailman/database/sqlite.py
index 4e5df55a5..39e7e8113 100644
--- a/src/mailman/database/sqlite.py
+++ b/src/mailman/database/sqlite.py
@@ -27,6 +27,7 @@ __all__ = [
 
 import os
 import shutil
+import sqlite3
 import tempfile
 
 from urlparse import urlparse
@@ -49,6 +50,7 @@ class SQLiteDatabase(StormBaseDatabase):
     """Database class for SQLite."""
 
     TAG = 'sqlite'
+    Error = sqlite3.OperationalError
 
     def _database_exists(self, store):
         """See `BaseDatabase`."""
diff --git a/src/mailman/database/tests/data/migration_postgres_1.sql b/src/mailman/database/tests/data/migration_postgres_1.sql
new file mode 100644
index 000000000..f144367d0
--- /dev/null
+++ b/src/mailman/database/tests/data/migration_postgres_1.sql
@@ -0,0 +1,133 @@
+INSERT INTO "acceptablealias" VALUES(1,'foo@example.com',1);
+INSERT INTO "acceptablealias" VALUES(2,'bar@example.com',1);
+
+INSERT INTO "address" VALUES(
+    1,'anne@example.com',NULL,'Anne Person',
+    '2012-04-19 00:52:24.826432','2012-04-19 00:49:42.373769',1,2);
+INSERT INTO "address" VALUES(
+    2,'bart@example.com',NULL,'Bart Person',
+    '2012-04-19 00:53:25.878800','2012-04-19 00:49:52.882050',2,4);
+
+INSERT INTO "domain" VALUES(
+    1,'example.com','http://example.com',NULL,'postmaster@example.com');
+
+INSERT INTO "mailinglist" VALUES(
+    -- id,list_name,mail_host,include_list_post_header,include_rfc2369_headers
+    1,'test','example.com',True,True,
+    -- created_at,admin_member_chunksize,next_request_id,next_digest_number
+    '2012-04-19 00:46:13.173844',30,1,1,
+    -- digest_last_sent_at,volume,last_post_at,accept_these_nonmembers
+    NULL,1,NULL,E'\\x80025D71012E',
+    -- acceptable_aliases_id,admin_immed_notify,admin_notify_mchanges
+    NULL,True,False,
+    -- administrivia,advertised,anonymous_list,archive,archive_private
+    True,True,False,True,False,
+    -- archive_volume_frequency
+    1,
+    --autorespond_owner,autoresponse_owner_text
+    0,'',
+    -- autorespond_postings,autoresponse_postings_text
+    0,'',
+    -- autorespond_requests,authoresponse_requests_text
+    0,'',
+    -- autoresponse_grace_period
+    '90 days, 0:00:00',
+    -- forward_unrecognized_bounces_to,process_bounces
+    1,True,
+    -- bounce_info_stale_after,bounce_matching_headers
+    '7 days, 0:00:00','
+# Lines that *start* with a ''#'' are comments.
+to: friend@public.com
+message-id: relay.comanche.denmark.eu
+from: list@listme.com
+from: .*@uplinkpro.com
+',
+    -- bounce_notify_owner_on_disable,bounce_notify_owner_on_removal
+    True,True,
+    -- bounce_score_threshold,bounce_you_are_disabled_warnings
+    5,3,
+    -- bounce_you_are_disabled_warnings_interval
+    '7 days, 0:00:00',
+    -- filter_action,filter_content,collapse_alternatives
+    2,False,True,
+    -- convert_html_to_plaintext,default_member_action,default_nonmember_action
+    False,4,0,
+    -- description
+    '',
+    -- digest_footer_uri
+    'mailman:///$listname/$language/footer-generic.txt',
+    -- digest_header_uri
+    NULL,
+    -- digest_is_default,digest_send_periodic,digest_size_threshold
+    False,True,30.0,
+    -- digest_volume_frequency,digestable,discard_these_nonmembers
+    1,True,E'\\x80025D71012E',
+    -- emergency,encode_ascii_prefixes,first_strip_reply_to
+    False,False,False,
+    -- footer_uri
+    'mailman:///$listname/$language/footer-generic.txt',
+    -- forward_auto_discards,gateway_to_mail,gateway_to_news
+    True,False,FAlse,
+    -- generic_nonmember_action,goodby_message_uri
+    1,'',
+    -- header_matches,header_uri,hold_these_nonmembers,info,linked_newsgroup
+    E'\\x80025D71012E',NULL,E'\\x80025D71012E','','',
+    -- max_days_to_hold,max_message_size,max_num_recipients
+    0,40,10,
+    -- member_moderation_notice,mime_is_default_digest,moderator_password
+    '',False,NULL,
+    -- new_member_options,news_moderation,news_prefix_subject_too
+    256,0,True,
+    -- nntp_host,nondigestable,nonmember_rejection_notice,obscure_addresses
+    '',True,'',True,
+    -- owner_chain,owner_pipeline,personalize,post_id
+    'default-owner-chain','default-owner-pipeline',0,1,
+    -- posting_chain,posting_pipeline,preferred_language,private_roster
+    'default-posting-chain','default-posting-pipeline','en',True,
+    -- display_name,reject_these_nonmembers
+    'Test',E'\\x80025D71012E',
+    -- reply_goes_to_list,reply_to_address
+    0,'',
+    -- require_explicit_destination,respond_to_post_requests    
+    True,True,
+    -- scrub_nondigest,send_goodbye_message,send_reminders,send_welcome_message
+    False,True,True,True,
+    -- subject_prefix,subscribe_auto_approval
+    '[Test] ',E'\\x80025D71012E',
+    -- subscribe_policy,topics,topics_bodylines_limit,topics_enabled
+    1,E'\\x80025D71012E',5,False,
+    -- unsubscribe_policy,welcome_message_uri
+    0,'mailman:///welcome.txt');
+
+INSERT INTO "member" VALUES(
+    1,'d1243f4d-e604-4f6b-af52-98d0a7bce0f1',1,'test@example.com',4,NULL,5,1);
+INSERT INTO "member" VALUES(
+    2,'dccc3851-fdfb-4afa-90cf-bdcbf80ad0fd',2,'test@example.com',3,NULL,6,1);
+INSERT INTO "member" VALUES(
+    3,'479be431-45f2-473d-bc3c-7eac614030ac',3,'test@example.com',3,NULL,7,2);
+INSERT INTO "member" VALUES(
+    4,'e2dc604c-d93a-4b91-b5a8-749e3caade36',1,'test@example.com',4,NULL,8,2);
+
+INSERT INTO "preferences" VALUES(1,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(2,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(3,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(4,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(5,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(6,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(7,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+INSERT INTO "preferences" VALUES(8,NULL,NULL,NULL,NULL,NULL,NULL,NULL);
+
+INSERT INTO "user" VALUES(
+    1,'Anne Person',NULL,'0adf3caa-6f26-46f8-a11d-5256c8148592',
+    '2012-04-19 00:49:42.370493',1,1);
+INSERT INTO "user" VALUES(
+    2,'Bart Person',NULL,'63f5d1a2-e533-4055-afe4-475dec3b1163',
+    '2012-04-19 00:49:52.868746',2,3);
+
+INSERT INTO "uid" VALUES(1,'8bf9a615-f23e-4980-b7d1-90ac0203c66f');
+INSERT INTO "uid" VALUES(2,'0adf3caa-6f26-46f8-a11d-5256c8148592');
+INSERT INTO "uid" VALUES(3,'63f5d1a2-e533-4055-afe4-475dec3b1163');
+INSERT INTO "uid" VALUES(4,'d1243f4d-e604-4f6b-af52-98d0a7bce0f1');
+INSERT INTO "uid" VALUES(5,'dccc3851-fdfb-4afa-90cf-bdcbf80ad0fd');
+INSERT INTO "uid" VALUES(6,'479be431-45f2-473d-bc3c-7eac614030ac');
+INSERT INTO "uid" VALUES(7,'e2dc604c-d93a-4b91-b5a8-749e3caade36');
diff --git a/src/mailman/database/tests/data/migration_test_1.sql b/src/mailman/database/tests/data/migration_sqlite_1.sql
index 101feea77..c8810b130 100644
--- a/src/mailman/database/tests/data/migration_test_1.sql
+++ b/src/mailman/database/tests/data/migration_sqlite_1.sql
@@ -8,10 +8,8 @@ INSERT INTO "address" VALUES(
     2,'bart@example.com',NULL,'Bart Person',
     '2012-04-19 00:53:25.878800','2012-04-19 00:49:52.882050',2,4);
 
--- ConfigLayer.testSetUp() will already initialize the domain.
---
--- INSERT INTO "domain" VALUES(
---     1,'example.com','http://example.com',NULL,'postmaster@example.com');
+INSERT INTO "domain" VALUES(
+    1,'example.com','http://example.com',NULL,'postmaster@example.com');
 
 INSERT INTO "mailinglist" VALUES(
     -- id,list_name,mail_host,include_list_post_header,include_rfc2369_headers
diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py
index 0e6edae47..90f2a9dfe 100644
--- a/src/mailman/database/tests/test_migrations.py
+++ b/src/mailman/database/tests/test_migrations.py
@@ -27,10 +27,6 @@ __all__ = [
     ]
 
 
-import os
-import shutil
-import sqlite3
-import tempfile
 import unittest
 
 from pkg_resources import resource_string
@@ -41,7 +37,7 @@ from mailman.interfaces.domain import IDomainManager
 from mailman.interfaces.archiver import ArchivePolicy
 from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.mailinglist import IAcceptableAliasSet
-from mailman.testing.helpers import configuration, temporary_db
+from mailman.testing.helpers import temporary_db
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.modules import call_name
 
@@ -73,7 +69,7 @@ class TestMigration20120407Schema(unittest.TestCase):
     def tearDown(self):
         self._temporary.cleanup()
 
-    def test_sqlite_base(self):
+    def test_pre_upgrade_columns_base(self):
         # Test that before the migration, the old table columns are present
         # and the new database columns are not.
         #
@@ -82,9 +78,12 @@ class TestMigration20120407Schema(unittest.TestCase):
         # Verify that the database has not yet been migrated.
         for missing in ('archive_policy',
                         'nntp_prefix_subject_too'):
-            self.assertRaises(sqlite3.OperationalError,
+            self.assertRaises(self._database.Error,
                               self._database.store.execute,
                               'select {0} from mailinglist;'.format(missing))
+            # Avoid PostgreSQL complaint: InternalError: current transaction
+            # is aborted, commands ignored until end of transaction block.
+            self._database.store.execute('ABORT;')
         for present in ('archive',
                         'archive_private',
                         'archive_volume_frequency',
@@ -96,7 +95,7 @@ class TestMigration20120407Schema(unittest.TestCase):
             self._database.store.execute(
                 'select {0} from mailinglist;'.format(present))
 
-    def test_sqlite_migration(self):
+    def test_post_upgrade_columns_migration(self):
         # Test that after the migration, the old table columns are missing
         # and the new database columns are present.
         #
@@ -116,9 +115,12 @@ class TestMigration20120407Schema(unittest.TestCase):
                         'news_moderation',
                         'news_prefix_subject_too',
                         'nntp_host'):
-            self.assertRaises(sqlite3.OperationalError,
+            self.assertRaises(self._database.Error,
                               self._database.store.execute,
                               'select {0} from mailinglist;'.format(missing))
+            # Avoid PostgreSQL complaint: InternalError: current transaction
+            # is aborted, commands ignored until end of transaction block.
+            self._database.store.execute('ABORT;')
 
 
 
@@ -148,7 +150,8 @@ class TestMigration20120407Data(unittest.TestCase):
         self._database.load_migrations('20120406999999')
         # Load the previous schema's sample data.
         sample_data = resource_string(
-            'mailman.database.tests.data', 'migration_test_1.sql')
+            'mailman.database.tests.data',
+            'migration_{0}_1.sql'.format(database_class.TAG))
         self._database.load_sql(self._database.store, sample_data)
         # Update to the current migration we're testing.
         self._database.load_migrations('20120407000000')
@@ -230,7 +233,8 @@ class TestMigration20120407ArchiveData(unittest.TestCase):
         self._database.load_migrations('20120406999999')
         # Load the previous schema's sample data.
         sample_data = resource_string(
-            'mailman.database.tests.data', 'migration_test_1.sql')
+            'mailman.database.tests.data',
+            'migration_{0}_1.sql'.format(database_class.TAG))
         self._database.load_sql(self._database.store, sample_data)
 
     def _upgrade(self):
@@ -244,13 +248,11 @@ class TestMigration20120407ArchiveData(unittest.TestCase):
         # Test that the new archive_policy value is updated correctly.  In the
         # case of old column archive=0, the archive_private column is
         # ignored.  This test sets it to 0 to ensure it's ignored.
-        with configuration('database', url=self._url):
-            # Set the old archive values.
-            self._database.store.execute(
-                'UPDATE mailinglist SET archive = 0, archive_private = 0 '
-                'WHERE id = 1;')
-            # Complete the migration
-            self._upgrade()
+        self._database.store.execute(
+            'UPDATE mailinglist SET archive = False, archive_private = False '
+            'WHERE id = 1;')
+        # Complete the migration
+        self._upgrade()
         with temporary_db(self._database):
             mlist = getUtility(IListManager).get('test@example.com')
             self.assertEqual(mlist.archive_policy, ArchivePolicy.never)
@@ -259,13 +261,11 @@ class TestMigration20120407ArchiveData(unittest.TestCase):
         # Test that the new archive_policy value is updated correctly.  In the
         # case of old column archive=0, the archive_private column is
         # ignored.  This test sets it to 1 to ensure it's ignored.
-        with configuration('database', url=self._url):
-            # Set the old archive values.
-            self._database.store.execute(
-                'UPDATE mailinglist SET archive = 0, archive_private = 1 '
-                'WHERE id = 1;')
-            # Complete the migration
-            self._upgrade()
+        self._database.store.execute(
+            'UPDATE mailinglist SET archive = False, archive_private = True '
+            'WHERE id = 1;')
+        # Complete the migration
+        self._upgrade()
         with temporary_db(self._database):
             mlist = getUtility(IListManager).get('test@example.com')
             self.assertEqual(mlist.archive_policy, ArchivePolicy.never)
@@ -273,13 +273,11 @@ class TestMigration20120407ArchiveData(unittest.TestCase):
     def test_archive_policy_private(self):
         # Test that the new archive_policy value is updated correctly for
         # private archives.
-        with configuration('database', url=self._url):
-            # Set the old archive values.
-            self._database.store.execute(
-                'UPDATE mailinglist SET archive = 1, archive_private = 1 '
-                'WHERE id = 1;')
-            # Complete the migration
-            self._upgrade()
+        self._database.store.execute(
+            'UPDATE mailinglist SET archive = True, archive_private = True '
+            'WHERE id = 1;')
+        # Complete the migration
+        self._upgrade()
         with temporary_db(self._database):
             mlist = getUtility(IListManager).get('test@example.com')
             self.assertEqual(mlist.archive_policy, ArchivePolicy.private)
@@ -287,13 +285,11 @@ class TestMigration20120407ArchiveData(unittest.TestCase):
     def test_archive_policy_public(self):
         # Test that the new archive_policy value is updated correctly for
         # public archives.
-        with configuration('database', url=self._url):
-            # Set the old archive values.
-            self._database.store.execute(
-                'UPDATE mailinglist SET archive = 1, archive_private = 0 '
-                'WHERE id = 1;')
-            # Complete the migration
-            self._upgrade()
+        self._database.store.execute(
+            'UPDATE mailinglist SET archive = True, archive_private = False '
+            'WHERE id = 1;')
+        # Complete the migration
+        self._upgrade()
         with temporary_db(self._database):
             mlist = getUtility(IListManager).get('test@example.com')
             self.assertEqual(mlist.archive_policy, ArchivePolicy.public)