From b3815b6b98df95f68b06f401417ac1be464e1f01 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 18 Dec 2018 15:33:26 +0100 Subject: [PATCH 1/8] #23 Add tests for caching --- django_migration_linter/constants.py | 21 +++++++ django_migration_linter/utils.py | 6 ++ setup.py | 8 ++- tests/functional/test_cache.py | 85 ++++++++++++++++++++++++++ tests/functional/test_cmd_line_call.py | 45 ++++++++++++++ tox.ini | 1 + 6 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 django_migration_linter/constants.py create mode 100644 tests/functional/test_cache.py diff --git a/django_migration_linter/constants.py b/django_migration_linter/constants.py new file mode 100644 index 0000000..3086825 --- /dev/null +++ b/django_migration_linter/constants.py @@ -0,0 +1,21 @@ +# Copyright 2018 3YOURMIND GmbH + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from pathlib import Path + +from appdirs import user_cache_dir + +__VERSION__ = '0.0.7' + +DEFAULT_CACHE_PATH = Path(user_cache_dir("django-migration-linter", version=__VERSION__)) diff --git a/django_migration_linter/utils.py b/django_migration_linter/utils.py index b2b8f23..4575a4f 100644 --- a/django_migration_linter/utils.py +++ b/django_migration_linter/utils.py @@ -16,6 +16,8 @@ from __future__ import print_function import os import sys +from django_migration_linter.constants import DEFAULT_CACHE_PATH + def is_django_project(path): django_manage_file = os.path.join(path, 'manage.py') @@ -54,3 +56,7 @@ def split_path(path): def clean_bytes_to_str(byte_input): return byte_input.decode('utf-8').strip() + + +def get_default_cache_file(project_name): + os.join(DEFAULT_CACHE_PATH, '{0}.pickle'.format(project_name)) diff --git a/setup.py b/setup.py index 79ca34d..c1f728c 100644 --- a/setup.py +++ b/setup.py @@ -18,11 +18,16 @@ from setuptools import setup, find_packages PROJECT_DIR = path.abspath(path.dirname(__file__)) +# Read __VERSION__ +exec(open('./django_migration_linter/constants.py').read()) + + with open(path.join(PROJECT_DIR, 'README.rst')) as f: long_description = f.read() install_requirements = [ 'django>=1.11', + 'appdirs==1.4.3' ] test_requirements = [ @@ -31,9 +36,10 @@ test_requirements = [ 'django-fake-database-backends', ] +# noinspection PyUnresolvedReferences setup( name='django-migration-linter', - version='0.0.7', + version=__VERSION__, description='Detect backward incompatible migrations for your django project', long_description=long_description, diff --git a/tests/functional/test_cache.py b/tests/functional/test_cache.py new file mode 100644 index 0000000..85a94ef --- /dev/null +++ b/tests/functional/test_cache.py @@ -0,0 +1,85 @@ +# Copyright 2018 3YOURMIND GmbH + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +import shutil +import tempfile +import unittest +from unittest import mock + +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 + + +class CacheTest(unittest.TestCase): + MIGRATION_FILE = os.path.join(ADD_NOT_NULL_COLUMN_PROJECT, 'test_app', 'migrations', '0001_create_table.py') + fd, temp_path = tempfile.mkstemp() + + def setUp(self): + shutil.copy2(self.MIGRATION_FILE, self.temp_path) + + def tearDown(self): + shutil.copy2(self.temp_path, self.MIGRATION_FILE) + 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')) + 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: + linter.lint_all_migrations() + self.assertEqual(popen_mock.call_count, 2) + + with mock.patch( + 'subprocess.Popen' + ) as popen_mock: + linter.lint_all_migrations() + + self.assertEqual(popen_mock.call_count, 0) + self.assertFalse(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) + 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 open( self.MIGRATION_FILE, "a") as f: + f.write("# modification at the end of the file\n") + + linter.lint_all_migrations() + cache = linter.get_cache() + + self.assertNotIn('hash', cache) + self.assertEqual(cache['newhash']['result'], 'OK') + + + diff --git a/tests/functional/test_cmd_line_call.py b/tests/functional/test_cmd_line_call.py index a3afd33..4f1c4b5 100644 --- a/tests/functional/test_cmd_line_call.py +++ b/tests/functional/test_cmd_line_call.py @@ -12,9 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import unittest from subprocess import Popen, PIPE from django_migration_linter import utils +from django_migration_linter.utils import get_default_cache_file from tests import fixtures import sys @@ -115,6 +117,49 @@ class CallLinterFromCommandLineTest(unittest.TestCase): self.assertTrue(lines[1].endswith('OK')) 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')) + cmd = '{0} {1}'.format( + self.linter_exec, + fixtures.CORRECT_PROJECT + ) + + process = Popen( + cmd, shell=True, stdout=PIPE, stderr=PIPE) + process.wait() + self.assertTrue(os.path.exists(get_default_cache_file('test_correct_project'))) + + 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') + cmd = '{0} {1} --cache-path={2}'.format( + self.linter_exec, + fixtures.CORRECT_PROJECT, + '/tmp/migration-linter-cache-tests' + ) + + process = Popen( + cmd, shell=True, stdout=PIPE, stderr=PIPE) + process.wait() + self.assertEqual(process.returncode, 0) + 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')) + + cmd = '{0} {1} --no-cache'.format( + self.linter_exec, + fixtures.CORRECT_PROJECT + ) + + process = Popen( + 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'))) + def test_call_linter_cmd_line_git_id(self): cmd = '{0} {1} d7125d5f4f0cc9623f670a66c54f131acc50032d'.format( self.linter_exec, diff --git a/tox.ini b/tox.ini index 7ffc655..a1e4ccc 100644 --- a/tox.ini +++ b/tox.ini @@ -11,6 +11,7 @@ pip_pre = true deps = django111: django>=1.11,<2.0 django20: django>=2.0 + appdirs==1.4.3 [testenv:lint] basepython = python3.6 From 9608e991500f8daea3565e982e0788bcdbb81ade Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 18 Dec 2018 15:54:47 +0100 Subject: [PATCH 2/8] #23 Fix errors and flake8 --- django_migration_linter/constants.py | 4 +++- django_migration_linter/utils.py | 2 +- setup.py | 16 ++++++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/django_migration_linter/constants.py b/django_migration_linter/constants.py index 3086825..a667d9a 100644 --- a/django_migration_linter/constants.py +++ b/django_migration_linter/constants.py @@ -18,4 +18,6 @@ from appdirs import user_cache_dir __VERSION__ = '0.0.7' -DEFAULT_CACHE_PATH = Path(user_cache_dir("django-migration-linter", version=__VERSION__)) +DEFAULT_CACHE_PATH = Path( + user_cache_dir("django-migration-linter", version=__VERSION__) +) diff --git a/django_migration_linter/utils.py b/django_migration_linter/utils.py index 4575a4f..0f83ad5 100644 --- a/django_migration_linter/utils.py +++ b/django_migration_linter/utils.py @@ -59,4 +59,4 @@ def clean_bytes_to_str(byte_input): def get_default_cache_file(project_name): - os.join(DEFAULT_CACHE_PATH, '{0}.pickle'.format(project_name)) + return os.path.join(DEFAULT_CACHE_PATH, '{0}.pickle'.format(project_name)) diff --git a/setup.py b/setup.py index c1f728c..7337984 100644 --- a/setup.py +++ b/setup.py @@ -11,15 +11,23 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +import ast +import re from os import path from setuptools import setup, find_packages PROJECT_DIR = path.abspath(path.dirname(__file__)) -# Read __VERSION__ -exec(open('./django_migration_linter/constants.py').read()) + +def get_version(): + constants = path.join(PROJECT_DIR, 'django_migration_linter', 'constants.py') + _version_re = re.compile(r"__VERSION__\s+=\s+(?P.*)") + with open(constants, "r") as f: + match = _version_re.search(f.read()) + version = match.group("version") if match is not None else '"unknown"' + return str(ast.literal_eval(version)) + with open(path.join(PROJECT_DIR, 'README.rst')) as f: @@ -39,7 +47,7 @@ test_requirements = [ # noinspection PyUnresolvedReferences setup( name='django-migration-linter', - version=__VERSION__, + version=get_version(), description='Detect backward incompatible migrations for your django project', long_description=long_description, From b68c0183bb6262fad6d37503dd11578281739125 Mon Sep 17 00:00:00 2001 From: Felix Date: Thu, 20 Dec 2018 14:45:35 +0100 Subject: [PATCH 3/8] Finish up cache and fix tests. Still TODO: Merge with ignore functionality. --- django_migration_linter/cache.py | 53 ++++++++++ django_migration_linter/constants.py | 10 +- django_migration_linter/migration_linter.py | 75 +++++++++++--- django_migration_linter/utils.py | 23 ++++- setup.py | 3 +- tests/functional/test_cache.py | 106 +++++++++++++------- tests/functional/test_cmd_line_call.py | 45 +++++---- tests/functional/test_multiple_calls.py | 8 +- tests/unit/test_linter.py | 24 +---- tests/unit/test_utils.py | 21 +++- tox.ini | 1 + 11 files changed, 263 insertions(+), 106 deletions(-) create mode 100644 django_migration_linter/cache.py diff --git a/django_migration_linter/cache.py b/django_migration_linter/cache.py new file mode 100644 index 0000000..0d3f524 --- /dev/null +++ b/django_migration_linter/cache.py @@ -0,0 +1,53 @@ +# Copyright 2018 3YOURMIND GmbH + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import hashlib +import os +import pickle + +from django_migration_linter.utils import split_path, compose_migration_path + + +class Cache(dict): + def __init__(self, django_folder, cache_path): + self.django_folder = django_folder + project_name = split_path(django_folder)[-2] + self.filename = os.path.join(cache_path, '{0}.pickle'.format(project_name)) + + if not os.path.exists(os.path.dirname(self.filename)): + os.makedirs(os.path.dirname(self.filename)) + + super(Cache, self).__init__() + + def load(self): + try: + f = open(self.filename, 'rb') + tmp_dict = pickle.load(f) + self.update(tmp_dict) + f.close() + except IOError: + pass + + def save(self): + f = open(self.filename, 'wb') + pickle.dump(self, f, 2) + f.close() + + def md5(self, app_name, migration): + path = compose_migration_path(self.django_folder, app_name, migration) + hash_md5 = hashlib.md5() + with open(path, "rb") as f: + for chunk in iter(lambda: f.read(4096), b""): + hash_md5.update(chunk) + return hash_md5.hexdigest() diff --git a/django_migration_linter/constants.py b/django_migration_linter/constants.py index a667d9a..1bdfc9b 100644 --- a/django_migration_linter/constants.py +++ b/django_migration_linter/constants.py @@ -12,12 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from pathlib import Path - from appdirs import user_cache_dir -__VERSION__ = '0.0.7' +__version__ = '0.0.7' -DEFAULT_CACHE_PATH = Path( - user_cache_dir("django-migration-linter", version=__VERSION__) +MIGRATION_FOLDER_NAME = 'migrations' + +DEFAULT_CACHE_PATH = user_cache_dir( + "django-migration-linter", version=__version__ ) diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index b2cd6aa..66fa34c 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -18,6 +18,10 @@ import os import re from subprocess import Popen, PIPE import sys + +from django_migration_linter.cache import Cache +from django_migration_linter.constants import DEFAULT_CACHE_PATH, MIGRATION_FOLDER_NAME +from django_migration_linter.utils import split_migration_path from . import utils from .sql_analyser import analyse_sql_statements @@ -25,7 +29,6 @@ logger = logging.getLogger(__name__) class MigrationLinter(object): - MIGRATION_FOLDER_NAME = 'migrations' def __init__(self, project_path, **kwargs): # Verify correctness @@ -45,6 +48,9 @@ class MigrationLinter(object): self.include_apps = kwargs.get('include_apps', None) self.exclude_apps = kwargs.get('exclude_apps', None) self.database = kwargs.get('database', None) or 'default' + self.cache_path = kwargs.get('cache_path', None) or DEFAULT_CACHE_PATH + self.no_cache = kwargs.get('no_cache', None) or False + self.python_exe = '{0}/bin/{1}'.format(sys.prefix, 'python') if \ hasattr(sys, 'real_prefix') else 'python' @@ -54,12 +60,37 @@ class MigrationLinter(object): self.nb_erroneous = 0 self.nb_total = 0 + # Initialise cache. Read from old, write to new to prune old entries. + self.old_cache = Cache(self.django_path, self.cache_path) + self.new_cache = Cache(self.django_path, self.cache_path) + if not self.no_cache: + self.old_cache.load() + def lint_migration(self, app_name, migration_name): print('({0}, {1})... '.format(app_name, migration_name), end='') self.nb_total += 1 + md5hash = self.old_cache.md5(app_name, migration_name) + if md5hash in self.old_cache: + if self.old_cache[md5hash]['result'] == 'IGNORE': + print('IGNORE (cached)') + self.nb_ignored += 1 + elif self.old_cache[md5hash]['result'] == 'OK': + print('OK (cached)') + self.nb_valid += 1 + else: + print('ERR (cached)') + self.nb_erroneous += 1 + + if 'errors' in self.old_cache[md5hash]: + self.print_errors(self.old_cache[md5hash]['errors']) + + self.new_cache[md5hash] = self.old_cache[md5hash] + return + if self.should_ignore_migration(app_name, migration_name): print('IGNORE') + self.new_cache[md5hash] = {'result': 'IGNORE'} self.nb_ignored += 1 return @@ -70,12 +101,17 @@ class MigrationLinter(object): errors = analysis_result['errors'] if not errors: print('OK') + self.new_cache[md5hash] = {'result': 'OK'} self.nb_valid += 1 return - # Print errors print('ERR') + self.new_cache[md5hash] = {'result': 'ERR', 'errors': errors} self.nb_erroneous += 1 + self.print_errors(errors) + + @staticmethod + def print_errors(errors): for err in errors: error_str = '\t{0}'.format(err['err_msg']) if err['table']: @@ -101,6 +137,9 @@ class MigrationLinter(object): for m in migrations: self.lint_migration(*m) + if not self.no_cache: + self.new_cache.save() + def print_summary(self): print('*** Summary:') print(('Valid migrations: {1}/{0} - ' @@ -166,10 +205,10 @@ class MigrationLinter(object): utils.clean_bytes_to_str, diff_process.stdout.readlines()): # Only gather lines that include added migrations if re.search( - '\/{0}\/.*\.py'.format(self.MIGRATION_FOLDER_NAME), + '\/{0}\/.*\.py'.format(MIGRATION_FOLDER_NAME), line) and \ '__init__' not in line: - app_name, migration_name = self._split_migration_path(line) + app_name, migration_name = split_migration_path(line) migrations.append((app_name, migration_name)) diff_process.wait() @@ -187,11 +226,11 @@ class MigrationLinter(object): migrations = [] for root, dirs, files in os.walk(self.django_path): for file_name in files: - if os.path.basename(root) == self.MIGRATION_FOLDER_NAME and \ + if os.path.basename(root) == MIGRATION_FOLDER_NAME and \ file_name.endswith('.py') and \ file_name != '__init__.py': full_migration_path = os.path.join(root, file_name) - app_name, migration_name = self._split_migration_path( + app_name, migration_name = split_migration_path( full_migration_path) migrations.append((app_name, migration_name)) return migrations @@ -205,14 +244,6 @@ class MigrationLinter(object): self.ignore_name_contains in migration_name)\ or (migration_name in self.ignore_name) - @classmethod - def _split_migration_path(cls, migration_path): - decomposed_path = utils.split_path(migration_path) - for i, p in enumerate(decomposed_path): - if p == cls.MIGRATION_FOLDER_NAME: - return (decomposed_path[i-1], - os.path.splitext(decomposed_path[i+1])[0]) - def _main(): import argparse @@ -253,6 +284,17 @@ def _main(): help=('specify the database for which to generate the SQL. ' 'Defaults to default')) + cache_group = parser.add_mutually_exclusive_group( + required=False) + cache_group.add_argument( + '--cache-path', + type=str, + help='specify a directory that should be used to store cache-files in.') + cache_group.add_argument( + '--no-cache', + action='store_true', + help='don\'t use a cache') + incl_excl_group = parser.add_mutually_exclusive_group( required=False) incl_excl_group.add_argument( @@ -280,7 +322,10 @@ def _main(): ignore_name=args.ignore_name, include_apps=args.include_apps, exclude_apps=args.exclude_apps, - database=args.database) + database=args.database, + cache_path=args.cache_path, + no_cache=args.no_cache + ) linter.lint_all_migrations( git_commit_id=args.commit_id) linter.print_summary() diff --git a/django_migration_linter/utils.py b/django_migration_linter/utils.py index 0f83ad5..8e26011 100644 --- a/django_migration_linter/utils.py +++ b/django_migration_linter/utils.py @@ -13,10 +13,11 @@ # limitations under the License. from __future__ import print_function + import os import sys -from django_migration_linter.constants import DEFAULT_CACHE_PATH +from django_migration_linter.constants import MIGRATION_FOLDER_NAME def is_django_project(path): @@ -54,9 +55,25 @@ def split_path(path): return (split_path(a) if (len(a) > 0 and a != '/') else []) + [b] +def split_migration_path(migration_path): + decomposed_path = split_path(migration_path) + for i, p in enumerate(decomposed_path): + if p == MIGRATION_FOLDER_NAME: + return (decomposed_path[i-1], + os.path.splitext(decomposed_path[i+1])[0]) + + +def compose_migration_path(django_folder, app_name, migration): + return os.path.join( + django_folder, + app_name, + MIGRATION_FOLDER_NAME, + '{0}.py'.format(migration) + ) + + def clean_bytes_to_str(byte_input): return byte_input.decode('utf-8').strip() -def get_default_cache_file(project_name): - return os.path.join(DEFAULT_CACHE_PATH, '{0}.pickle'.format(project_name)) + diff --git a/setup.py b/setup.py index 7337984..dd6bee3 100644 --- a/setup.py +++ b/setup.py @@ -22,14 +22,13 @@ PROJECT_DIR = path.abspath(path.dirname(__file__)) def get_version(): constants = path.join(PROJECT_DIR, 'django_migration_linter', 'constants.py') - _version_re = re.compile(r"__VERSION__\s+=\s+(?P.*)") + _version_re = re.compile(r"__version__\s+=\s+(?P.*)") with open(constants, "r") as f: match = _version_re.search(f.read()) version = match.group("version") if match is not None else '"unknown"' return str(ast.literal_eval(version)) - with open(path.join(PROJECT_DIR, 'README.rst')) as f: long_description = f.read() diff --git a/tests/functional/test_cache.py b/tests/functional/test_cache.py index 85a94ef..4ea631f 100644 --- a/tests/functional/test_cache.py +++ b/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') diff --git a/tests/functional/test_cmd_line_call.py b/tests/functional/test_cmd_line_call.py index 4f1c4b5..e651ea2 100644 --- a/tests/functional/test_cmd_line_call.py +++ b/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) diff --git a/tests/functional/test_multiple_calls.py b/tests/functional/test_multiple_calls.py index bea9262..0920c1b 100644 --- a/tests/functional/test_multiple_calls.py +++ b/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) diff --git a/tests/unit/test_linter.py b/tests/unit/test_linter.py index 0a4e47e..37fc754 100644 --- a/tests/unit/test_linter.py +++ b/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, diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 88d20af..7ac89d5 100644 --- a/tests/unit/test_utils.py +++ b/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') diff --git a/tox.ini b/tox.ini index a1e4ccc..a420feb 100644 --- a/tox.ini +++ b/tox.ini @@ -12,6 +12,7 @@ deps = django111: django>=1.11,<2.0 django20: django>=2.0 appdirs==1.4.3 + py27: mock==2.0.0 [testenv:lint] basepython = python3.6 From f351b71d6a6924e38b812794e753f50f48212c5b Mon Sep 17 00:00:00 2001 From: Felix Date: Thu, 20 Dec 2018 14:53:25 +0100 Subject: [PATCH 4/8] Fix flake8 --- django_migration_linter/cache.py | 3 ++- django_migration_linter/migration_linter.py | 6 ++++-- django_migration_linter/utils.py | 3 --- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/django_migration_linter/cache.py b/django_migration_linter/cache.py index 0d3f524..46e3a63 100644 --- a/django_migration_linter/cache.py +++ b/django_migration_linter/cache.py @@ -23,7 +23,8 @@ class Cache(dict): def __init__(self, django_folder, cache_path): self.django_folder = django_folder project_name = split_path(django_folder)[-2] - self.filename = os.path.join(cache_path, '{0}.pickle'.format(project_name)) + self.filename = \ + os.path.join(cache_path, '{0}.pickle'.format(project_name)) if not os.path.exists(os.path.dirname(self.filename)): os.makedirs(os.path.dirname(self.filename)) diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index 66fa34c..a5ac985 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -20,7 +20,8 @@ from subprocess import Popen, PIPE import sys from django_migration_linter.cache import Cache -from django_migration_linter.constants import DEFAULT_CACHE_PATH, MIGRATION_FOLDER_NAME +from django_migration_linter.constants import DEFAULT_CACHE_PATH, \ + MIGRATION_FOLDER_NAME from django_migration_linter.utils import split_migration_path from . import utils from .sql_analyser import analyse_sql_statements @@ -289,7 +290,8 @@ def _main(): cache_group.add_argument( '--cache-path', type=str, - help='specify a directory that should be used to store cache-files in.') + help='specify a directory that should be used to' + 'store cache-files in.') cache_group.add_argument( '--no-cache', action='store_true', diff --git a/django_migration_linter/utils.py b/django_migration_linter/utils.py index 8e26011..355df5f 100644 --- a/django_migration_linter/utils.py +++ b/django_migration_linter/utils.py @@ -74,6 +74,3 @@ def compose_migration_path(django_folder, app_name, migration): def clean_bytes_to_str(byte_input): return byte_input.decode('utf-8').strip() - - - From d3805d5210d4e3ce8fd7fcc5d221a3a327dc36d5 Mon Sep 17 00:00:00 2001 From: Felix Date: Sun, 23 Dec 2018 22:49:36 +0100 Subject: [PATCH 5/8] Integrate IGNORE into cache --- django_migration_linter/cache.py | 1 + django_migration_linter/migration_linter.py | 1 + tests/functional/test_cache.py | 26 +++++++++++++++++++-- tests/functional/test_cmd_line_call.py | 2 +- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/django_migration_linter/cache.py b/django_migration_linter/cache.py index 46e3a63..ca03b2d 100644 --- a/django_migration_linter/cache.py +++ b/django_migration_linter/cache.py @@ -41,6 +41,7 @@ class Cache(dict): pass def save(self): + print(self) f = open(self.filename, 'wb') pickle.dump(self, f, 2) f.close() diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index a04f5a2..1614f8a 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -103,6 +103,7 @@ class MigrationLinter(object): if analysis_result['ignored']: print('IGNORE') + self.new_cache[md5hash] = {'result': 'IGNORE'} self.nb_ignored += 1 return diff --git a/tests/functional/test_cache.py b/tests/functional/test_cache.py index 4ea631f..7c68ed4 100644 --- a/tests/functional/test_cache.py +++ b/tests/functional/test_cache.py @@ -28,7 +28,6 @@ else: import mock - class CacheTest(unittest.TestCase): MIGRATION_FILE = os.path.join(fixtures.ALTER_COLUMN_PROJECT, 'test_app', 'migrations', '0001_initial.py') fd, temp_path = tempfile.mkstemp() @@ -73,7 +72,30 @@ class CacheTest(unittest.TestCase): self.assertTrue(linter.has_errors) def test_cache_ignored(self): - pass + cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_ignore_migration.pickle') + if os.path.exists(cache_file): + os.remove(cache_file) + linter = MigrationLinter(fixtures.IGNORE_MIGRATION_PROJECT) + + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock: + linter.lint_all_migrations() + sql_mock.assert_called() + + cache = Cache( + fixtures.IGNORE_MIGRATION_PROJECT, + DEFAULT_CACHE_PATH + ) + cache.load() + + self.assertEqual(cache['63230606af0eccaef7f1f78c537c624c']['result'], 'OK') + self.assertEqual(cache['5c5ca1780a9f28439c1defc1f32af894']['result'], 'IGNORE') + + # Start the Linter again -> should use cache now. + linter = MigrationLinter(fixtures.IGNORE_MIGRATION_PROJECT) + + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock: + linter.lint_all_migrations() + sql_mock.assert_not_called() def test_cache_modified(self): cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_alter_column.pickle') diff --git a/tests/functional/test_cmd_line_call.py b/tests/functional/test_cmd_line_call.py index 8518a9e..d15a9cd 100644 --- a/tests/functional/test_cmd_line_call.py +++ b/tests/functional/test_cmd_line_call.py @@ -67,7 +67,7 @@ class CallLinterFromCommandLineTest(unittest.TestCase): fixtures.CORRECT_PROJECT) process = Popen( - cmd, shell=True, stdout=PIPE, stderr=sys.stderr) + cmd, shell=True, stdout=PIPE, stderr=PIPE) process.wait() self.assertEqual(process.returncode, 0) lines = list(map(utils.clean_bytes_to_str, process.stdout.readlines())) From 03e48b7575b49a21443213bdfa27bf7ad9faf9df Mon Sep 17 00:00:00 2001 From: Felix Date: Sun, 23 Dec 2018 23:06:18 +0100 Subject: [PATCH 6/8] Adjust readme --- README.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.rst b/README.rst index d1c5161..9c47e3f 100644 --- a/README.rst +++ b/README.rst @@ -44,6 +44,8 @@ Usage ``--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]`` Ignore migrations that are in the specified django apps. ``--verbose or -v`` Print more information during execution. ``--database DATABASE`` Specify the database for which to generate the SQL. Defaults to *default*. +``--cache-path PATH`` specify a directory that should be used to store cache-files in. +``--no-cache`` Don't use a cache. ================================================== =========================================================================================================================== Examples @@ -68,6 +70,15 @@ You can also ignore migrations by adding this to your migrations: ] # ... +Cache +----- +By default, the linter uses a cache to prevent linting the same migration again. +The default location of the cache on Linux is +``/home//.cache/django-migration-linter//.pickle``. + +Since the linter uses hashes, modifying migration files, will re-run the linter on that migration. +If you want to run the linter without cache, use the flag ``--no-cache``. +If you want to invalidate the cache, delete the cache folder. Tests ----- From 1c28412ebd9777f05910305d1ee57234607765af Mon Sep 17 00:00:00 2001 From: Felix Date: Sun, 23 Dec 2018 23:28:35 +0100 Subject: [PATCH 7/8] Improve the cache React to Comments and add a Test --- django_migration_linter/cache.py | 13 ++++------- django_migration_linter/migration_linter.py | 1 - tests/functional/test_cache.py | 26 ++++++++++++++++++--- tests/functional/test_cmd_line_call.py | 1 - 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/django_migration_linter/cache.py b/django_migration_linter/cache.py index ca03b2d..e0f00ea 100644 --- a/django_migration_linter/cache.py +++ b/django_migration_linter/cache.py @@ -33,18 +33,15 @@ class Cache(dict): def load(self): try: - f = open(self.filename, 'rb') - tmp_dict = pickle.load(f) - self.update(tmp_dict) - f.close() + with open(self.filename, 'rb') as f: + tmp_dict = pickle.load(f) + self.update(tmp_dict) except IOError: pass def save(self): - print(self) - f = open(self.filename, 'wb') - pickle.dump(self, f, 2) - f.close() + with open(self.filename, 'wb') as f: + pickle.dump(self, f, protocol=pickle.HIGHEST_PROTOCOL) def md5(self, app_name, migration): path = compose_migration_path(self.django_folder, app_name, migration) diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index 1614f8a..e3d1fd3 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -91,7 +91,6 @@ class MigrationLinter(object): if self.should_ignore_migration(app_name, migration_name): print('IGNORE') - self.new_cache[md5hash] = {'result': 'IGNORE'} self.nb_ignored += 1 return diff --git a/tests/functional/test_cache.py b/tests/functional/test_cache.py index 7c68ed4..f6352e1 100644 --- a/tests/functional/test_cache.py +++ b/tests/functional/test_cache.py @@ -65,7 +65,7 @@ class CacheTest(unittest.TestCase): # 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: + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: linter.lint_all_migrations() sql_mock.assert_not_called() @@ -77,7 +77,7 @@ class CacheTest(unittest.TestCase): os.remove(cache_file) linter = MigrationLinter(fixtures.IGNORE_MIGRATION_PROJECT) - with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock: + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: linter.lint_all_migrations() sql_mock.assert_called() @@ -93,10 +93,30 @@ class CacheTest(unittest.TestCase): # Start the Linter again -> should use cache now. linter = MigrationLinter(fixtures.IGNORE_MIGRATION_PROJECT) - with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock: + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: linter.lint_all_migrations() sql_mock.assert_not_called() + def test_cache_ignored_command_line(self): + cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_ignore_migration.pickle') + if os.path.exists(cache_file): + os.remove(cache_file) + linter = MigrationLinter(fixtures.IGNORE_MIGRATION_PROJECT, + ignore_name_contains='0001') + + with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: + linter.lint_all_migrations() + sql_mock.assert_called() + + cache = Cache( + fixtures.IGNORE_MIGRATION_PROJECT, + DEFAULT_CACHE_PATH + ) + cache.load() + + self.assertNotIn('63230606af0eccaef7f1f78c537c624c', cache) + self.assertEqual(cache['5c5ca1780a9f28439c1defc1f32af894']['result'], 'IGNORE') + def test_cache_modified(self): cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_alter_column.pickle') if os.path.exists(cache_file): diff --git a/tests/functional/test_cmd_line_call.py b/tests/functional/test_cmd_line_call.py index d15a9cd..b8ddabe 100644 --- a/tests/functional/test_cmd_line_call.py +++ b/tests/functional/test_cmd_line_call.py @@ -172,7 +172,6 @@ class CallLinterFromCommandLineTest(unittest.TestCase): def test_call_linter_cmd_line_no_cache(self): 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) From 5be5e3f769013e6f5bb40d3644f9daca12c151fc Mon Sep 17 00:00:00 2001 From: Felix Date: Sun, 23 Dec 2018 23:46:24 +0100 Subject: [PATCH 8/8] Use call_count instead of assert_called() because later was introduced in python 3.6. --- django_migration_linter/cache.py | 2 +- tests/functional/test_cache.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/django_migration_linter/cache.py b/django_migration_linter/cache.py index e0f00ea..0ddd08c 100644 --- a/django_migration_linter/cache.py +++ b/django_migration_linter/cache.py @@ -41,7 +41,7 @@ class Cache(dict): def save(self): with open(self.filename, 'wb') as f: - pickle.dump(self, f, protocol=pickle.HIGHEST_PROTOCOL) + pickle.dump(self, f, protocol=2) def md5(self, app_name, migration): path = compose_migration_path(self.django_folder, app_name, migration) diff --git a/tests/functional/test_cache.py b/tests/functional/test_cache.py index f6352e1..ae289b2 100644 --- a/tests/functional/test_cache.py +++ b/tests/functional/test_cache.py @@ -47,7 +47,7 @@ class CacheTest(unittest.TestCase): with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql)as sql_mock: linter.lint_all_migrations() - sql_mock.assert_called() + self.assertEqual(sql_mock.call_count, 2) cache = Cache( fixtures.ADD_NOT_NULL_COLUMN_PROJECT, @@ -67,7 +67,7 @@ class CacheTest(unittest.TestCase): 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(sql_mock.call_count, 0) self.assertTrue(linter.has_errors) @@ -79,7 +79,7 @@ class CacheTest(unittest.TestCase): with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: linter.lint_all_migrations() - sql_mock.assert_called() + self.assertEqual(sql_mock.call_count, 2) cache = Cache( fixtures.IGNORE_MIGRATION_PROJECT, @@ -95,7 +95,7 @@ class CacheTest(unittest.TestCase): 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(sql_mock.call_count, 0) def test_cache_ignored_command_line(self): cache_file = os.path.join(DEFAULT_CACHE_PATH, 'test_project_ignore_migration.pickle') @@ -106,7 +106,7 @@ class CacheTest(unittest.TestCase): with mock.patch.object(MigrationLinter, 'get_sql', wraps=linter.get_sql) as sql_mock: linter.lint_all_migrations() - sql_mock.assert_called() + self.assertEqual(sql_mock.call_count, 1) cache = Cache( fixtures.IGNORE_MIGRATION_PROJECT,