Finish up cache and fix tests.

Still TODO: Merge with ignore functionality.

tests/functional/test_cache.py

@@ -12,20 +12,25 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import os
+import pprint
 import shutil
 import tempfile
 import unittest
-from unittest import mock
+import sys
 
-from django_migration_linter.utils import get_default_cache_file
 from tests import fixtures
 
-from django_migration_linter import MigrationLinter
-from tests.fixtures import ADD_NOT_NULL_COLUMN_PROJECT
+from django_migration_linter import MigrationLinter, Cache, DEFAULT_CACHE_PATH
+
+if sys.version_info >= (3, 3):
+    import unittest.mock as mock
+else:
+    import mock
+
 
 
 class CacheTest(unittest.TestCase):
-    MIGRATION_FILE = os.path.join(ADD_NOT_NULL_COLUMN_PROJECT, 'test_app', 'migrations', '0001_create_table.py')
+    MIGRATION_FILE = os.path.join(fixtures.ALTER_COLUMN_PROJECT, 'test_app', 'migrations', '0001_initial.py')
     fd, temp_path = tempfile.mkstemp()
 
     def setUp(self):
@@ -36,50 +41,83 @@ class CacheTest(unittest.TestCase):
         os.remove(self.temp_path)
 
     def test_cache_normal(self):
-        if os.path.exists(get_default_cache_file('test_project_add_not_null_column')):
-            os.remove(get_default_cache_file('test_project_add_not_null_column'))
+        cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_add_not_null_column.pickle')
+        if os.path.exists(cache_file):
+            os.remove(cache_file)
         linter = MigrationLinter(fixtures.ADD_NOT_NULL_COLUMN_PROJECT)
-        linter.lint_all_migrations()
-        cache = linter.get_cache()
-        self.assertEqual(cache['hash']['result'], 'OK')
-        self.assertEqual(cache['hashERR']['result'], 'ERR')
-        self.assertDictEqual(cache['hashERR']['errors'], ...)
 
-        with mock.patch(
-                'subprocess.Popen'
-        ) as popen_mock:
+        with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock:
             linter.lint_all_migrations()
-        self.assertEqual(popen_mock.call_count, 2)
+            sql_mock.assert_called()
 
-        with mock.patch(
-                'subprocess.Popen'
-        ) as popen_mock:
+        cache = Cache(
+            fixtures.ADD_NOT_NULL_COLUMN_PROJECT,
+            DEFAULT_CACHE_PATH
+        )
+        cache.load()
+
+        self.assertEqual(cache['3ef74e7f3e53e273e2fc95379248d58d']['result'], 'OK')
+        self.assertEqual(cache['e1e312b6d08ecbe017c25c58fc2be257']['result'], 'ERR')
+        self.assertListEqual(
+            cache['e1e312b6d08ecbe017c25c58fc2be257']['errors'],
+            [{'err_msg': 'RENAMING tables', 'code': 'RENAME_TABLE', 'table': None, 'column': None}]
+        )
+
+        # Start the Linter again -> should use cache now.
+        linter = MigrationLinter(fixtures.ADD_NOT_NULL_COLUMN_PROJECT)
+
+        with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock:
             linter.lint_all_migrations()
+            sql_mock.assert_not_called()
 
-        self.assertEqual(popen_mock.call_count, 0)
-        self.assertFalse(linter.has_errors())
+        self.assertTrue(linter.has_errors)
 
     def test_cache_ignored(self):
         pass
 
     def test_cache_modified(self):
-        if os.path.exists(get_default_cache_file('test_project_add_not_null_column')):
-            os.remove(get_default_cache_file('test_project_add_not_null_column'))
-        linter = MigrationLinter(fixtures.ADD_NOT_NULL_COLUMN_PROJECT)
+        cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_alter_column.pickle')
+        if os.path.exists(cache_file):
+            os.remove(cache_file)
+        linter = MigrationLinter(fixtures.ALTER_COLUMN_PROJECT)
         linter.lint_all_migrations()
-        cache = linter.get_cache()
-        self.assertEqual(cache['hash']['result'], 'OK')
-        self.assertEqual(cache['hashERR']['result'], 'ERR')
-        self.assertDictEqual(cache['hashERR']['errors'], ...)
+        cache = Cache(
+            fixtures.ALTER_COLUMN_PROJECT,
+            DEFAULT_CACHE_PATH
+        )
+        cache.load()
 
-        with open( self.MIGRATION_FILE, "a") as f:
+        self.assertEqual(cache['8589aa107b6da296c4b49cd2681d2230']['result'], 'OK',
+                         'If this fails, tearDown might have failed to remove '
+                         'the modification from tests/test_project_fixtures/'
+                         'test_project_alter_column/test_app/migrations/0001_initial.py'
+                         )
+        self.assertEqual(cache['8f54c4a434cfaa9838e8ca12eb988255']['result'], 'ERR')
+        self.assertListEqual(
+            cache['8f54c4a434cfaa9838e8ca12eb988255']['errors'],
+            [{u'err_msg': u'ALTERING columns (Could be backward compatible. '
+                          u'You may ignore this migration.)',
+              u'code': u'ALTER_COLUMN',
+              u'table': u'test_app_a',
+              u'column': None}
+             ]
+        )
+
+        # Modify migration
+        with open(self.MIGRATION_FILE, "a") as f:
             f.write("# modification at the end of the file\n")
 
+        # Start the Linter again -> Cache should look different now
+        linter = MigrationLinter(fixtures.ALTER_COLUMN_PROJECT)
         linter.lint_all_migrations()
-        cache = linter.get_cache()
-
-        self.assertNotIn('hash', cache)
-        self.assertEqual(cache['newhash']['result'], 'OK')
-
+        cache = Cache(
+            fixtures.ALTER_COLUMN_PROJECT,
+            DEFAULT_CACHE_PATH
+        )
+        cache.load()
 
+        pp = pprint.PrettyPrinter(indent=4)
+        pp.pprint(cache)
 
+        self.assertNotIn('8589aa107b6da296c4b49cd2681d2230', cache)
+        self.assertEqual(cache['44f9d6d019d6f158946c2819b2bd8bf9']['result'], 'OK')

tests/functional/test_cmd_line_call.py

@@ -13,10 +13,10 @@
 # limitations under the License.
 
 import os
+import shutil
 import unittest
 from subprocess import Popen, PIPE
-from django_migration_linter import utils
-from django_migration_linter.utils import get_default_cache_file
+from django_migration_linter import utils, DEFAULT_CACHE_PATH
 from tests import fixtures
 import sys
 
@@ -29,7 +29,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         cls.linter_exec = '{0}/bin/{1}'.format(sys.prefix,  linter_name) if hasattr(sys, 'real_prefix') else linter_name
 
     def test_call_linter_cmd_line_working(self):
-        cmd = '{0} {1}'.format(self.linter_exec, fixtures.CORRECT_PROJECT)
+        cmd = '{0} --no-cache {1}'.format(self.linter_exec, fixtures.CORRECT_PROJECT)
 
         process = Popen(
             cmd, shell=True, stdout=PIPE, stderr=PIPE)
@@ -42,7 +42,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[2].endswith('OK'))
 
     def test_call_linter_cmd_line_errors(self):
-        cmd = '{0} {1}'.format(
+        cmd = '{0} --no-cache {1}'.format(
             self.linter_exec,
             fixtures.ADD_NOT_NULL_COLUMN_PROJECT)
 
@@ -58,12 +58,12 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue('RENAMING tables' in lines[2])
 
     def test_call_linter_cmd_line_exclude_apps(self):
-        cmd = '{0} {1} --exclude-apps test_app2'.format(
+        cmd = '{0} --no-cache {1} --exclude-apps test_app2'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT)
 
         process = Popen(
-            cmd, shell=True, stdout=PIPE, stderr=PIPE)
+            cmd, shell=True, stdout=PIPE, stderr=sys.stderr)
         process.wait()
         self.assertEqual(process.returncode, 0)
         lines = list(map(utils.clean_bytes_to_str, process.stdout.readlines()))
@@ -73,7 +73,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[2].endswith('IGNORE'))
 
     def test_call_linter_cmd_line_include_apps(self):
-        cmd = '{0} {1} --include-apps test_app2'.format(
+        cmd = '{0} --no-cache {1} --include-apps test_app2'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT)
 
@@ -88,7 +88,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[2].endswith('OK'))
 
     def test_call_linter_cmd_line_ignore_name(self):
-        cmd = '{0} {1} --ignore-name 0001_initial'.format(
+        cmd = '{0} --no-cache {1} --ignore-name 0001_initial'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT)
 
@@ -103,7 +103,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[2].endswith('OK'))
 
     def test_call_linter_cmd_line_ignore_name_contains(self):
-        cmd = '{0} {1} --ignore-name-contains 0001'.format(
+        cmd = '{0} --no-cache {1} --ignore-name-contains 0001'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT)
 
@@ -118,8 +118,9 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[2].endswith('IGNORE'))
 
     def test_call_linter_cmd_line_cache(self):
-        if os.path.exists(get_default_cache_file('test_correct_project')):
-            os.remove(get_default_cache_file('test_correct_project'))
+        cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_correct_project.pickle')
+        if os.path.exists(cache_file):
+            os.remove(cache_file)
         cmd = '{0} {1}'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT
@@ -128,15 +129,15 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         process = Popen(
             cmd, shell=True, stdout=PIPE, stderr=PIPE)
         process.wait()
-        self.assertTrue(os.path.exists(get_default_cache_file('test_correct_project')))
+        self.assertTrue(os.path.exists(DEFAULT_CACHE_PATH))
 
     def test_call_linter_cmd_line_cache_path(self):
-        if os.path.exists('/tmp/migration-linter-cache-tests'):
-            os.remove('/tmp/migration-linter-cache-tests')
+        if os.path.exists('/tmp/migration-linter-cache-tests/'):
+            shutil.rmtree('/tmp/migration-linter-cache-tests/')
         cmd = '{0} {1} --cache-path={2}'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT,
-            '/tmp/migration-linter-cache-tests'
+            '/tmp/migration-linter-cache-tests/'
         )
 
         process = Popen(
@@ -146,10 +147,12 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(os.path.exists('/tmp/migration-linter-cache-tests'))
 
     def test_call_linter_cmd_line_no_cache(self):
-        if os.path.exists(get_default_cache_file('test_correct_project')):
-            os.remove(get_default_cache_file('test_correct_project'))
+        cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_correct_project.pickle')
+        print(cache_file)
+        if os.path.exists(cache_file):
+            os.remove(cache_file)
 
-        cmd = '{0} {1} --no-cache'.format(
+        cmd = '{0} --no-cache {1}'.format(
             self.linter_exec,
             fixtures.CORRECT_PROJECT
         )
@@ -158,10 +161,10 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
             cmd, shell=True, stdout=PIPE, stderr=PIPE)
         process.wait()
         self.assertEqual(process.returncode, 0)
-        self.assertFalse(os.path.exists(get_default_cache_file('test_correct_project')))
+        self.assertFalse(os.path.exists(cache_file))
 
     def test_call_linter_cmd_line_git_id(self):
-        cmd = '{0} {1} d7125d5f4f0cc9623f670a66c54f131acc50032d'.format(
+        cmd = '{0} --no-cache {1} d7125d5f4f0cc9623f670a66c54f131acc50032d'.format(
             self.linter_exec,
             fixtures.MULTI_COMMIT_PROJECT)
         fixtures.prepare_git_project(fixtures.MULTI_COMMIT_PROJECT)
@@ -176,7 +179,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase):
         self.assertTrue(lines[1].startswith('*** Summary'))
 
     def test_call_linter_with_deleted_migrations(self):
-        cmd = '{0} {1} 154ecf6119325cc3b1f3f5a4e709bfbd61a4a4ba'.format(
+        cmd = '{0} --no-cache {1} 154ecf6119325cc3b1f3f5a4e709bfbd61a4a4ba'.format(
             self.linter_exec,
             fixtures.DELETED_MIGRATION_PROJECT)
         fixtures.prepare_git_project(fixtures.DELETED_MIGRATION_PROJECT)

tests/functional/test_multiple_calls.py

@@ -24,10 +24,10 @@ class TestMultipleLinters(unittest.TestCase):
         l2 = MigrationLinter(fixtures.RENAME_COLUMN_PROJECT)
         l3 = MigrationLinter(fixtures.CORRECT_PROJECT)
 
-        l1.lint_migration('test_app', '0002')
-        l2.lint_migration('test_app', '0002')
-        l3.lint_migration('test_app1', '0001')
-        l3.lint_migration('test_app1', '0002')
+        l1.lint_migration('test_app', '0002_add_new_not_null_field')
+        l2.lint_migration('test_app', '0002_rename_column')
+        l3.lint_migration('test_app1', '0001_initial')
+        l3.lint_migration('test_app1', '0002_a_new_null_field')
 
         self.assertTrue(l1.has_errors)
         self.assertTrue(l2.has_errors)

tests/unit/test_linter.py

@@ -32,13 +32,13 @@ class TestLinterFunctions(unittest.TestCase):
         linter = MigrationLinter(project_path)
         self.assertFalse(linter.has_errors)
 
-        linter.lint_migration('test_app', '0001')
+        linter.lint_migration('test_app', '0001_create_table')
         self.assertFalse(linter.has_errors)
 
-        linter.lint_migration('test_app', '0002')
+        linter.lint_migration('test_app', '0002_add_new_not_null_field')
         self.assertTrue(linter.has_errors)
 
-        linter.lint_migration('test_app', '0001')
+        linter.lint_migration('test_app', '0001_create_table')
         self.assertTrue(linter.has_errors)
 
     def test_linter_creation(self):
@@ -49,24 +49,6 @@ class TestLinterFunctions(unittest.TestCase):
         with self.assertRaises(ValueError):
             MigrationLinter(fixtures.NOT_DJANGO_GIT_PROJECT)
 
-    def test_split_migration_long_path(self):
-        input_path = 'apps/the_app/migrations/0001_stuff.py'
-        app, mig = MigrationLinter._split_migration_path(input_path)
-        self.assertEqual(app, 'the_app')
-        self.assertEqual(mig, '0001_stuff')
-
-    def test_split_migration_path(self):
-        input_path = 'the_app/migrations/0001_stuff.py'
-        app, mig = MigrationLinter._split_migration_path(input_path)
-        self.assertEqual(app, 'the_app')
-        self.assertEqual(mig, '0001_stuff')
-
-    def test_split_migration_full_path(self):
-        input_path = '/home/user/djangostuff/apps/the_app/migrations/0001_stuff.py'
-        app, mig = MigrationLinter._split_migration_path(input_path)
-        self.assertEqual(app, 'the_app')
-        self.assertEqual(mig, '0001_stuff')
-
     def test_ignore_migration_include_apps(self):
         linter = MigrationLinter(
             fixtures.ADD_NOT_NULL_COLUMN_PROJECT,

tests/unit/test_utils.py

@@ -16,7 +16,8 @@ import os
 import unittest
 
 from tests import fixtures
-from django_migration_linter.utils import is_django_project, is_git_project, is_directory, find_project_settings_module, split_path
+from django_migration_linter.utils import is_django_project, is_git_project, is_directory, find_project_settings_module, \
+    split_path, split_migration_path
 
 
 class UtilityFunctionTest(unittest.TestCase):
@@ -70,3 +71,21 @@ class UtilityFunctionTest(unittest.TestCase):
         self.assertEqual(splitted[0], 'foo')
         self.assertEqual(splitted[1], 'bar')
         self.assertEqual(splitted[2], 'fuz.py')
+
+    def test_split_migration_long_path(self):
+        input_path = 'apps/the_app/migrations/0001_stuff.py'
+        app, mig = split_migration_path(input_path)
+        self.assertEqual(app, 'the_app')
+        self.assertEqual(mig, '0001_stuff')
+
+    def test_split_migration_path(self):
+        input_path = 'the_app/migrations/0001_stuff.py'
+        app, mig = split_migration_path(input_path)
+        self.assertEqual(app, 'the_app')
+        self.assertEqual(mig, '0001_stuff')
+
+    def test_split_migration_full_path(self):
+        input_path = '/home/user/djangostuff/apps/the_app/migrations/0001_stuff.py'
+        app, mig = split_migration_path(input_path)
+        self.assertEqual(app, 'the_app')
+        self.assertEqual(mig, '0001_stuff')