add tests for plain API, fixed some small bugs

7 files changed, 69 insertions, 79 deletions

diff --git a/migrate/versioning/api.py b/migrate/versioning/api.py
index 6e0da11..65a1ddc 100644
--- a/migrate/versioning/api.py
+++ b/migrate/versioning/api.py
@@ -1,12 +1,17 @@
 """
    This module provides an external API to the versioning system.
 
-   .. versionchanged:: 0.4.5 
+   .. versionchanged:: 0.6.0
+   :func:`migrate.versioning.api.test` and schema diff functions \
+   changed order of positional arguments so all accept `url` and `repository`\
+   as first arguments.
+
+   .. versionchanged:: 0.5.4 
     ``--preview_sql`` displays source file when using SQL scripts.
     If Python script is used, it runs the action with mocked engine and
     returns captured SQL statements.
 
-   .. versionchanged:: 0.4.5
+   .. versionchanged:: 0.5.4
     Deprecated ``--echo`` parameter in favour of new
     :func:`migrate.versioning.util.construct_engine` behavior.
 """
@@ -74,6 +79,7 @@ def help(cmd=None, **opts):
         ret = ret.replace('%prog', sys.argv[0])
     return ret
 
+
 @catch_known_errors
 def create(repository, name, **opts):
     """%prog create REPOSITORY_PATH NAME [--table=TABLE]
@@ -84,7 +90,7 @@ def create(repository, name, **opts):
     'migrate_version'.  This table is created in all version-controlled
     databases.
     """
-    repo_path = Repository.create(repository, name, **opts)
+    Repository.create(repository, name, **opts)
 
 
 @catch_known_errors
@@ -192,8 +198,8 @@ def downgrade(url, repository, version, **opts):
         "Try 'upgrade' instead."
     return _migrate(url, repository, version, upgrade=False, err=err, **opts)
     
-def test(repository, url, **opts):
-    """%prog test REPOSITORY_PATH URL [VERSION]
+def test(url, repository, **opts):
+    """%prog test URL REPOSITORY_PATH [VERSION]
 
     Performs the upgrade and downgrade option on the given
     database. This is not a real test and may leave the database in a
@@ -267,8 +273,8 @@ def manage(file, **opts):
     return Repository.create_manage_file(file, **opts)
 
 
-def compare_model_to_db(url, model, repository, **opts):
-    """%prog compare_model_to_db URL MODEL REPOSITORY_PATH
+def compare_model_to_db(url, repository, model, **opts):
+    """%prog compare_model_to_db URL REPOSITORY_PATH MODEL
 
     Compare the current model (assumed to be a module level variable
     of type sqlalchemy.MetaData) against the current database.
@@ -276,7 +282,7 @@ def compare_model_to_db(url, model, repository, **opts):
     NOTE: This is EXPERIMENTAL.
     """  # TODO: get rid of EXPERIMENTAL label
     engine = construct_engine(url, **opts)
-    print ControlledSchema.compare_model_to_db(engine, model, repository)
+    return ControlledSchema.compare_model_to_db(engine, model, repository)
 
 
 def create_model(url, repository, **opts):
@@ -288,13 +294,12 @@ def create_model(url, repository, **opts):
     """  # TODO: get rid of EXPERIMENTAL label
     engine = construct_engine(url, **opts)
     declarative = opts.get('declarative', False)
-    print ControlledSchema.create_model(engine, repository, declarative)
+    return ControlledSchema.create_model(engine, repository, declarative)
 
 
-# TODO: get rid of this? if we don't add back path param
 @catch_known_errors
-def make_update_script_for_model(url, oldmodel, model, repository, **opts):
-    """%prog make_update_script_for_model URL OLDMODEL MODEL REPOSITORY_PATH
+def make_update_script_for_model(url, repository, oldmodel, model, **opts):
+    """%prog make_update_script_for_model URL REPOSITORY_PATH OLDMODEL MODEL
 
     Create a script changing the old Python model to the new (current)
     Python model, sending to stdout.
@@ -302,12 +307,12 @@ def make_update_script_for_model(url, oldmodel, model, repository, **opts):
     NOTE: This is EXPERIMENTAL.
     """  # TODO: get rid of EXPERIMENTAL label
     engine = construct_engine(url, **opts)
-    print PythonScript.make_update_script_for_model(
+    return PythonScript.make_update_script_for_model(
         engine, oldmodel, model, repository, **opts)
 
 
-def update_db_from_model(url, model, repository, **opts):
-    """%prog update_db_from_model URL MODEL REPOSITORY_PATH
+def update_db_from_model(url, repository, model, **opts):
+    """%prog update_db_from_model URL REPOSITORY_PATH MODEL
 
     Modify the database to match the structure of the current Python
     model. This also sets the db_version number to the latest in the
@@ -337,15 +342,14 @@ def _migrate(url, repository, version, upgrade, err, **opts):
                 print change.source()
 
         elif opts.get('preview_py'):
+            if not isinstance(change, PythonScript):
+                raise exceptions.UsageError("Python source can be only displayed"
+                    " for python migration files")
             source_ver = max(ver, nextver)
             module = schema.repository.version(source_ver).script().module
             funcname = upgrade and "upgrade" or "downgrade"
             func = getattr(module, funcname)
-            if isinstance(change, PythonScript):
-                print inspect.getsource(func)
-            else:
-                raise UsageError("Python source can be only displayed"
-                    " for python migration files")
+            print inspect.getsource(func)
         else:
             schema.runchange(ver, change, changeset.step)
             print 'done'
diff --git a/migrate/versioning/schema.py b/migrate/versioning/schema.py
index 754288f..e98779a 100644
--- a/migrate/versioning/schema.py
+++ b/migrate/versioning/schema.py
@@ -1,6 +1,8 @@
 """
    Database schema version management.
 """
+import sys
+
 from sqlalchemy import (Table, Column, MetaData, String, Text, Integer,
     create_engine)
 from sqlalchemy.sql import and_
@@ -32,22 +34,17 @@ class ControlledSchema(object):
     def load(self):
         """Load controlled schema version info from DB"""
         tname = self.repository.version_table
-        if not hasattr(self, 'table') or self.table is None:
-            try:
-                self.table = Table(tname, self.meta, autoload=True)
-            except (sa_exceptions.NoSuchTableError,
-                    AssertionError):
-                # assertionerror is raised if no table is found in oracle db
-                raise exceptions.DatabaseNotControlledError(tname)
-
-        # TODO?: verify that the table is correct (# cols, etc.)
-        result = self.engine.execute(self.table.select(
-                    self.table.c.repository_id == str(self.repository.id)))
-
         try:
+            if not hasattr(self, 'table') or self.table is None:
+                    self.table = Table(tname, self.meta, autoload=True)
+
+            result = self.engine.execute(self.table.select(
+                self.table.c.repository_id == str(self.repository.id)))
+
             data = list(result)[0]
-        except IndexError:
-            raise exceptions.DatabaseNotControlledError(tname)
+        except Exception:
+            cls, exc, tb = sys.exc_info()
+            raise exceptions.DatabaseNotControlledError, exc.message, tb
 
         self.version = data['version']
         return data
diff --git a/migrate/versioning/script/py.py b/migrate/versioning/script/py.py
index 15f68b0..3eaec29 100644
--- a/migrate/versioning/script/py.py
+++ b/migrate/versioning/script/py.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
+import warnings
 import shutil
 from StringIO import StringIO
 
@@ -11,6 +12,7 @@ from migrate.versioning.template import template
 from migrate.versioning.script import base
 from migrate.versioning.util import import_path, load_model, construct_engine
 
+__all__ = ['PythonScript']
 
 class PythonScript(base.BaseScript):
     """Base for Python scripts"""
@@ -88,16 +90,11 @@ class PythonScript(base.BaseScript):
         
         :param path: Script location
         :type path: string
-
         :raises: :exc:`InvalidScriptError <migrate.versioning.exceptions.InvalidScriptError>`
         :returns: Python module
         """
         # Try to import and get the upgrade() func
-        try:
-            module = import_path(path)
-        except:
-            # If the script itself has errors, that's not our problem
-            raise
+        module = import_path(path)
         try:
             assert callable(module.upgrade)
         except Exception, e:
@@ -134,13 +131,15 @@ class PythonScript(base.BaseScript):
             op = 'downgrade'
         else:
             raise exceptions.ScriptError("%d is not a valid step" % step)
+
         funcname = base.operations[op]
-        
-        func = self._func(funcname)
+        script_func = self._func(funcname)
+
         try:
-            func(engine)
+            script_func(engine)
         except TypeError:
-            print "upgrade/downgrade functions must accept engine parameter (since ver 0.5.5)"
+            warnings.warn("upgrade/downgrade functions must accept engine"
+                " parameter (since version > 0.5.4)")
             raise
 
     @property
@@ -148,7 +147,7 @@ class PythonScript(base.BaseScript):
         """Calls :meth:`migrate.versioning.script.py.verify_module`
         and returns it.
         """
-        if not hasattr(self, '_module'):
+        if not getattr(self, '_module', None):
             self._module = self.verify_module(self.path)
         return self._module
 
diff --git a/migrate/versioning/script/sql.py b/migrate/versioning/script/sql.py
index 851fdf2..97dc505 100644
--- a/migrate/versioning/script/sql.py
+++ b/migrate/versioning/script/sql.py
@@ -7,8 +7,16 @@ from migrate.versioning.script import base
 class SqlScript(base.BaseScript):
     """A file containing plain SQL statements."""
 
+    @classmethod
+    def create(cls, path, **opts):
+        """Create an empty migration script at specified path
+        
+        :returns: :class:`SqlScript instance <migrate.versioning.script.sql.SqlScript>`"""
+        cls.require_notfound(path)
+        open(path, "w").close()
+
     # TODO: why is step parameter even here?
-    def run(self, engine, step=None):
+    def run(self, engine, step=None, executemany=True):
         """Runs SQL script through raw dbapi execute call"""
         text = self.source()
         # Don't rely on SA's autocommit here
@@ -21,7 +29,7 @@ class SqlScript(base.BaseScript):
                 # HACK: SQLite doesn't allow multiple statements through
                 # its execute() method, but it provides executescript() instead
                 dbapi = conn.engine.raw_connection()
-                if getattr(dbapi, 'executescript', None):
+                if executemany and getattr(dbapi, 'executescript', None):
                     dbapi.executescript(text)
                 else:
                     conn.execute(text)
diff --git a/migrate/versioning/shell.py b/migrate/versioning/shell.py
index 3fa1478..82a1627 100644
--- a/migrate/versioning/shell.py
+++ b/migrate/versioning/shell.py
@@ -69,7 +69,6 @@ def main(argv=None, **kwargs):
     parser = PassiveOptionParser(usage=usage)
     parser.add_option("-v", "--verbose", action="store_true", dest="verbose")
     parser.add_option("-d", "--debug", action="store_true", dest="debug")
-    parser.add_option("-f", "--force", action="store_true", dest="force")
     help_commands = ['help', '-h', '--help']
     HELP = False
 
@@ -156,8 +155,6 @@ def main(argv=None, **kwargs):
         if ret is not None:
             print ret
     except (exceptions.UsageError, exceptions.KnownError), e:
-        if e.args[0] is None:
-            parser.print_help()
         parser.error(e.args[0])
 
 if __name__ == "__main__":
diff --git a/migrate/versioning/util/__init__.py b/migrate/versioning/util/__init__.py
index 70370ac..01612b1 100644
--- a/migrate/versioning/util/__init__.py
+++ b/migrate/versioning/util/__init__.py
@@ -81,7 +81,7 @@ def catch_known_errors(f, *a, **kw):
     """
 
     try:
-        f(*a, **kw)
+        return f(*a, **kw)
     except exceptions.PathFoundError, e:
         raise exceptions.KnownError("The path %s already exists" % e.args[0])
 
diff --git a/migrate/versioning/version.py b/migrate/versioning/version.py
index d0842e3..92e9f19 100644
--- a/migrate/versioning/version.py
+++ b/migrate/versioning/version.py
@@ -98,11 +98,7 @@ class Collection(pathed.Pathed):
         filename = '%03d%s.py' % (ver, extra)
         filepath = self._version_path(filename)
 
-        if os.path.exists(filepath):
-            raise Exception('Script already exists: %s' % filepath)
-        else:
-            script.PythonScript.create(filepath)
-
+        script.PythonScript.create(filepath)
         self.versions[ver] = Version(ver, self.path, [filename])
         
     def create_new_sql_version(self, database, **k):
@@ -114,10 +110,7 @@ class Collection(pathed.Pathed):
         for op in ('upgrade', 'downgrade'):
             filename = '%03d_%s_%s.sql' % (ver, database, op)
             filepath = self._version_path(filename)
-            if os.path.exists(filepath):
-                raise Exception('Script already exists: %s' % filepath)
-            else:
-                open(filepath, "w").close()
+            script.SqlScript.create(filepath)
             self.versions[ver].add_script(filepath)
         
     def version(self, vernum=None):
@@ -137,7 +130,14 @@ class Collection(pathed.Pathed):
 
 
 class Version(object):
-    """A single version in a collection """
+    """A single version in a collection
+    :param vernum: Version Number 
+    :param path: Path to script files
+    :param filelist: List of scripts
+    :type vernum: int, VerNum
+    :type path: string
+    :type filelist: list
+    """
 
     def __init__(self, vernum, path, filelist):
         self.version = VerNum(vernum)
@@ -165,22 +165,6 @@ class Version(object):
             "There is no script for %d version" % self.version
         return ret
 
-    # deprecated?
-    @classmethod
-    def create(cls, path):
-        os.mkdir(path)
-        # create the version as a proper Python package
-        initfile = os.path.join(path, "__init__.py")
-        if not os.path.exists(initfile):
-            # just touch the file
-            open(initfile, "w").close()
-        try:
-            ret = cls(path)
-        except:
-            os.rmdir(path)
-            raise
-        return ret
-
     def add_script(self, path):
         """Add script to Collection/Version"""
         if path.endswith(Extensions.py):
@@ -203,10 +187,11 @@ class Version(object):
 
     def _add_script_py(self, path):
         if self.python is not None:
-            raise Exception('You can only have one Python script per version,'
-                ' but you have: %s and %s' % (self.python, path))
+            raise exceptions.ScriptError('You can only have one Python script '
+                'per version, but you have: %s and %s' % (self.python, path))
         self.python = script.PythonScript(path)
 
+
 class Extensions:
     """A namespace for file extensions"""
     py = 'py'