From 0ee05a2b30c5c3c9490cd9e20e943526aa99ce63 Mon Sep 17 00:00:00 2001 From: Oleg Samokhin Date: Thu, 3 Aug 2023 16:06:24 +0500 Subject: [PATCH] Support apps with custom label. --- CHANGELOG.md | 9 ++- .../migration_linter.py | 55 ++++++++++++++----- tests/fixtures.py | 2 + tests/functional/test_migration_linter.py | 34 ++++++++++-- .../app_with_custom_name/__init__.py | 0 .../test_project/app_with_custom_name/apps.py | 8 +++ .../migrations/0001_initial.py | 30 ++++++++++ .../migrations/__init__.py | 0 .../app_with_custom_name/models.py | 8 +++ tests/test_project/settings.py | 1 + 10 files changed, 125 insertions(+), 22 deletions(-) create mode 100644 tests/test_project/app_with_custom_name/__init__.py create mode 100644 tests/test_project/app_with_custom_name/apps.py create mode 100644 tests/test_project/app_with_custom_name/migrations/0001_initial.py create mode 100644 tests/test_project/app_with_custom_name/migrations/__init__.py create mode 100644 tests/test_project/app_with_custom_name/models.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 63572a1..4cfd71f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,11 @@ -## x.y.z +## 6.0.0 + +Feature: +- **Breaking change**: Handle custom Django app label when gathering migrations from git reference. (#262) +This means that, an app that was previously referenced by its folder name, will now be referenced by its Django app label. Miscellaneous: - -- Add py.typed file +- Add py.typed file (#303) ## 5.2.0 diff --git a/src/django_migration_linter/migration_linter.py b/src/django_migration_linter/migration_linter.py index f516462..64c0849 100644 --- a/src/django_migration_linter/migration_linter.py +++ b/src/django_migration_linter/migration_linter.py @@ -7,8 +7,9 @@ import logging import os import re from enum import Enum, unique +from importlib.util import find_spec from subprocess import PIPE, Popen -from typing import Callable, Iterable +from typing import Callable, Dict, Iterable from django.conf import settings from django.core.management import call_command @@ -389,7 +390,6 @@ class MigrationLinter: def _gather_migrations_git( self, git_commit_id: str, migrations_list: list[tuple[str, str]] | None = None ) -> Iterable[Migration]: - migrations = [] # Get changes since specified commit git_diff_command = [ "git", @@ -406,25 +406,50 @@ class MigrationLinter: stderr=PIPE, cwd=self.django_path, ) + + diskpath_and_migration: Dict[str, Migration] = {} + for migration in self._gather_all_migrations(): + spec = find_spec(migration.__module__) + if spec: + diskpath_and_migration[str(spec.origin)] = migration + + migrations = [] for line in map( clean_bytes_to_str, diff_process.stdout.readlines() # type: ignore ): - # Only gather lines that include added migrations + # Only gather lines that include added migrations. if self.is_migration_file(line): - app_label, name = split_migration_path(line) - if migrations_list is None or (app_label, name) in migrations_list: - if (app_label, name) in self.migration_loader.disk_migrations: - migration = self.migration_loader.disk_migrations[ - app_label, name - ] + # Find the migration objects with similar path. + suitable_migrations = [ + migration + for path, migration in diskpath_and_migration.items() + if path.endswith(line) + ] + if suitable_migrations: + migration = suitable_migrations[0] + if ( + migrations_list is None + or (migration.app_label, migration.name) in migrations_list + ): migrations.append(migration) - else: - logger.info( - "Found migration file (%s, %s) " - "that is not present in loaded migration.", - app_label, - name, + if len(suitable_migrations) > 1: + # If multiple migration founds, we chose one randomly, but need + # to alert that it's not very precise. + logger.warning( + "Found multiple migration files matching altered " + "file path %s. Choose (%s, %s) opportunistically.", + line, + migration.app_label, + migration.name, ) + else: + app_label, name = split_migration_path(line) + logger.info( + "Found migration file (%s, %s) " + "that is not present in loaded migration.", + app_label, + name, + ) diff_process.wait() if diff_process.returncode != 0: diff --git a/tests/fixtures.py b/tests/fixtures.py index 5432c0d..77f5801 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -19,3 +19,5 @@ DATA_MIGRATIONS = "app_data_migrations" MAKE_NOT_NULL_WITH_DJANGO_DEFAULT = "app_make_not_null_with_django_default" MAKE_NOT_NULL_WITH_LIB_DEFAULT = "app_make_not_null_with_lib_default" CREATE_INDEX_EXCLUSIVE = "app_create_index_exclusive" +CUSTOM_APP_NAME_DIRECTORY = "app_with_custom_name" +CUSTOM_APP_LABEL = "my_custom_name" diff --git a/tests/functional/test_migration_linter.py b/tests/functional/test_migration_linter.py index 6c3cf2f..3d1fa5e 100644 --- a/tests/functional/test_migration_linter.py +++ b/tests/functional/test_migration_linter.py @@ -34,22 +34,32 @@ class BaseBackwardCompatibilityDetection: linter = self._launch_linter(app, commit_id) self.assertTrue(linter.has_errors) self.assertNotEqual(linter.nb_valid + linter.nb_erroneous, 0) + self.assertGreater(linter.nb_total, 0) def _test_linter_finds_no_errors(self, app=None, commit_id=None): linter = self._launch_linter(app, commit_id) self.assertFalse(linter.has_errors) self.assertNotEqual(linter.nb_valid + linter.nb_erroneous, 0) + self.assertGreater(linter.nb_total, 0) + + def _test_linter_linted_no_migration(self, app=None, commit_id=None): + linter = self._launch_linter(app, commit_id) + self.assertEqual(linter.nb_valid, 0) + self.assertEqual(linter.nb_erroneous, 0) + self.assertEqual(linter.nb_total, 0) def _launch_linter(self, app=None, commit_id=None): - linter = MigrationLinter( + linter = self._get_linter() + linter.lint_all_migrations(app_label=app, git_commit_id=commit_id) + return linter + + def _get_linter(self): + return MigrationLinter( self.test_project_path, database=self.database, no_cache=True, ) - linter.lint_all_migrations(app_label=app, git_commit_id=commit_id) - return linter - # *** Tests *** def test_create_table_with_not_null_column(self): app = fixtures.CREATE_TABLE_WITH_NOT_NULL_COLUMN self._test_linter_finds_no_errors(app) @@ -121,6 +131,22 @@ class BaseBackwardCompatibilityDetection: with self.assertRaises(ValueError): linter.get_sql("app_unique_together", "0003") + def test_custom_named_app(self): + # Folder name is not found. + app = fixtures.CUSTOM_APP_NAME_DIRECTORY + self._test_linter_linted_no_migration(app) + + # Django app name is found. + app = fixtures.CUSTOM_APP_LABEL + self._test_linter_finds_no_errors(app) + + # And with git ref: + app = fixtures.CUSTOM_APP_NAME_DIRECTORY + self._test_linter_linted_no_migration(app, commit_id="v0.1.4") + + app = fixtures.CUSTOM_APP_LABEL + self._test_linter_finds_no_errors(app, commit_id="v0.1.4") + class SqliteBackwardCompatibilityDetectionTestCase( BaseBackwardCompatibilityDetection, unittest.TestCase diff --git a/tests/test_project/app_with_custom_name/__init__.py b/tests/test_project/app_with_custom_name/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_with_custom_name/apps.py b/tests/test_project/app_with_custom_name/apps.py new file mode 100644 index 0000000..ab11f30 --- /dev/null +++ b/tests/test_project/app_with_custom_name/apps.py @@ -0,0 +1,8 @@ +from __future__ import annotations + +from django.apps import AppConfig + + +class DefaultConfig(AppConfig): + name = "tests.test_project.app_with_custom_name" + label = "my_custom_name" diff --git a/tests/test_project/app_with_custom_name/migrations/0001_initial.py b/tests/test_project/app_with_custom_name/migrations/0001_initial.py new file mode 100644 index 0000000..31d396c --- /dev/null +++ b/tests/test_project/app_with_custom_name/migrations/0001_initial.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2 on 2019-07-29 21:21 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="A", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("int_field", models.IntegerField()), + ("char_field", models.CharField(max_length=255)), + ], + ) + ] diff --git a/tests/test_project/app_with_custom_name/migrations/__init__.py b/tests/test_project/app_with_custom_name/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_with_custom_name/models.py b/tests/test_project/app_with_custom_name/models.py new file mode 100644 index 0000000..d30f51a --- /dev/null +++ b/tests/test_project/app_with_custom_name/models.py @@ -0,0 +1,8 @@ +from __future__ import annotations + +from django.db import models + + +class A(models.Model): + int_field = models.IntegerField() + char_field = models.CharField(max_length=255) diff --git a/tests/test_project/settings.py b/tests/test_project/settings.py index d71663d..361b429 100644 --- a/tests/test_project/settings.py +++ b/tests/test_project/settings.py @@ -59,6 +59,7 @@ INSTALLED_APPS = [ "tests.test_project.app_make_not_null_with_django_default", "tests.test_project.app_make_not_null_with_lib_default", "tests.test_project.app_create_index_exclusive", + "tests.test_project.app_with_custom_name.apps.DefaultConfig", ] if django.VERSION[0] >= 5: