From f495b95d0ee28141bd067ca64a0176189f5d8293 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Thu, 6 Dec 2012 13:16:14 -0800 Subject: Fix bugs in test_migrations.py This fixes two related issues in nova/tests/test_migrations.py. First, the snake_walk flag was only set to True for the first test run out of TestMigrations. setUp() always set snake_walk to False initially then would only update the value if TEST_DATABASES was empty. TEST_DATABASES was only empty during the setUp() of the first test. Fix this by resetting test_databases before each test. Second, because snake_walk was always False when test_walk_versions performing the snake walk version of the database migrations was never tested and broken. When downgrading from version X to version X -1 of a database version X must be passed to the migration tool as version X contains the downgrade path for that particular version. Set the correct versions when snake walking the DB migrations. Part of blueprint grizzly-testtools Change-Id: I8d4d9ee8efe46b4690dd6cbe95879e88aaeb0b5a --- nova/tests/test_migrations.py | 49 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 25 deletions(-) (limited to 'nova') diff --git a/nova/tests/test_migrations.py b/nova/tests/test_migrations.py index 138bb3477..f0a047b1f 100644 --- a/nova/tests/test_migrations.py +++ b/nova/tests/test_migrations.py @@ -79,7 +79,6 @@ def _have_mysql(): class TestMigrations(test.TestCase): """Test sqlalchemy-migrate migrations""" - TEST_DATABASES = {} DEFAULT_CONFIG_FILE = os.path.join(os.path.dirname(__file__), 'test_migrations.conf') # Test machines can set the NOVA_TEST_MIGRATIONS_CONF variable @@ -94,28 +93,28 @@ class TestMigrations(test.TestCase): super(TestMigrations, self).setUp() self.snake_walk = False + self.test_databases = {} # Load test databases from the config file. Only do this # once. No need to re-run this on each test... LOG.debug('config_path is %s' % TestMigrations.CONFIG_FILE_PATH) - if not TestMigrations.TEST_DATABASES: - if os.path.exists(TestMigrations.CONFIG_FILE_PATH): - cp = ConfigParser.RawConfigParser() - try: - cp.read(TestMigrations.CONFIG_FILE_PATH) - defaults = cp.defaults() - for key, value in defaults.items(): - TestMigrations.TEST_DATABASES[key] = value - self.snake_walk = cp.getboolean('walk_style', 'snake_walk') - except ConfigParser.ParsingError, e: - self.fail("Failed to read test_migrations.conf config " - "file. Got error: %s" % e) - else: - self.fail("Failed to find test_migrations.conf config " - "file.") + if os.path.exists(TestMigrations.CONFIG_FILE_PATH): + cp = ConfigParser.RawConfigParser() + try: + cp.read(TestMigrations.CONFIG_FILE_PATH) + defaults = cp.defaults() + for key, value in defaults.items(): + self.test_databases[key] = value + self.snake_walk = cp.getboolean('walk_style', 'snake_walk') + except ConfigParser.ParsingError, e: + self.fail("Failed to read test_migrations.conf config " + "file. Got error: %s" % e) + else: + self.fail("Failed to find test_migrations.conf config " + "file.") self.engines = {} - for key, value in TestMigrations.TEST_DATABASES.items(): + for key, value in self.test_databases.items(): self.engines[key] = sqlalchemy.create_engine(value) # We start each test case with a completely blank slate. @@ -131,8 +130,8 @@ class TestMigrations(test.TestCase): # remove these from the list so they aren't used in the migration tests if "mysqlcitest" in self.engines: del self.engines["mysqlcitest"] - if "mysqlcitest" in TestMigrations.TEST_DATABASES: - del TestMigrations.TEST_DATABASES["mysqlcitest"] + if "mysqlcitest" in self.test_databases: + del self.test_databases["mysqlcitest"] super(TestMigrations, self).tearDown() def _reset_databases(self): @@ -141,7 +140,7 @@ class TestMigrations(test.TestCase): LOG.debug(output) self.assertEqual(0, status) for key, engine in self.engines.items(): - conn_string = TestMigrations.TEST_DATABASES[key] + conn_string = self.test_databases[key] conn_pieces = urlparse.urlparse(conn_string) if conn_string.startswith('sqlite'): # We can just delete the SQLite database, which is @@ -222,7 +221,7 @@ class TestMigrations(test.TestCase): connect_string = _mysql_get_connect_string() engine = sqlalchemy.create_engine(connect_string) self.engines["mysqlcitest"] = engine - TestMigrations.TEST_DATABASES["mysqlcitest"] = connect_string + self.test_databases["mysqlcitest"] = connect_string # build a fully populated mysql database with all the tables self._reset_databases() @@ -268,19 +267,19 @@ class TestMigrations(test.TestCase): # upgrade -> downgrade -> upgrade self._migrate_up(engine, version) if snake_walk: - self._migrate_down(engine, version - 1) + self._migrate_down(engine, version) self._migrate_up(engine, version) if downgrade: # Now walk it back down to 0 from the latest, testing # the downgrade paths. for version in reversed( - xrange(migration.INIT_VERSION + 1, - TestMigrations.REPOSITORY.latest)): + xrange(migration.INIT_VERSION + 2, + TestMigrations.REPOSITORY.latest + 1)): # downgrade -> upgrade -> downgrade self._migrate_down(engine, version) if snake_walk: - self._migrate_up(engine, version + 1) + self._migrate_up(engine, version) self._migrate_down(engine, version) def _migrate_down(self, engine, version): -- cgit