Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with gather migrations when app has custom defined label #262

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
42 changes: 28 additions & 14 deletions django_migration_linter/migration_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
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
Expand Down Expand Up @@ -379,25 +380,38 @@ def _gather_migrations_git(
).format(self.django_path, git_commit_id)
logger.info(f"Executing {git_diff_command}")
diff_process = Popen(git_diff_command, shell=True, stdout=PIPE, stderr=PIPE)

path_to_migration: Dict[str, Migration] = {}
for migration in self._gather_all_migrations():
spec = find_spec(migration.__module__)
if spec:
path_to_migration[str(spec.origin)] = migration

for line in map(
clean_bytes_to_str, diff_process.stdout.readlines() # type: ignore
):
# 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
]
suitable_migrations = [
migration
for path, migration in path_to_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,
)
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:
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@
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 = "app_with_custom_name"
46 changes: 38 additions & 8 deletions tests/functional/test_migration_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

from django_migration_linter import MigrationLinter
from tests import fixtures
from tests.test_project.app_with_custom_name.apps import DefaultConfig

V014_REVISION = "1245e7939dde000af1fd888070c2bd14b5ba29c6"


class BaseBackwardCompatibilityDetection:
Expand Down Expand Up @@ -39,15 +42,19 @@ def _test_linter_finds_no_errors(self, app=None, commit_id=None):
self.assertNotEqual(linter.nb_valid + linter.nb_erroneous, 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 ***

class BaseBackwardCompatibilityDetectionTests(BaseBackwardCompatibilityDetection):
def test_create_table_with_not_null_column(self):
app = fixtures.CREATE_TABLE_WITH_NOT_NULL_COLUMN
self._test_linter_finds_no_errors(app)
Expand Down Expand Up @@ -105,7 +112,7 @@ def test_accept_adding_manytomany_field(self):
self._test_linter_finds_no_errors(app)

def test_with_git_ref(self):
self._test_linter_finds_errors(commit_id="v0.1.4")
self._test_linter_finds_errors(commit_id=V014_REVISION)
Copy link
Author

@pirelle pirelle Aug 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but I had no labels in git when tests running, so changed it into revision number.


def test_failing_get_sql(self):
call_command("migrate", "app_unique_together", database=self.database)
Expand All @@ -116,7 +123,7 @@ def test_failing_get_sql(self):


class SqliteBackwardCompatibilityDetectionTestCase(
BaseBackwardCompatibilityDetection, unittest.TestCase
BaseBackwardCompatibilityDetectionTests, unittest.TestCase
):
databases = ["sqlite"]

Expand All @@ -135,13 +142,13 @@ def test_detect_make_column_not_null_with_lib_default(self):


class MySqlBackwardCompatibilityDetectionTestCase(
BaseBackwardCompatibilityDetection, unittest.TestCase
BaseBackwardCompatibilityDetectionTests, unittest.TestCase
):
databases = ["mysql"]


class PostgresqlBackwardCompatibilityDetectionTestCase(
BaseBackwardCompatibilityDetection, unittest.TestCase
BaseBackwardCompatibilityDetectionTests, unittest.TestCase
):
databases = ["postgresql"]

Expand All @@ -153,3 +160,26 @@ def test_create_index_exclusive(self):
linter = self._launch_linter(fixtures.CREATE_INDEX_EXCLUSIVE)
self.assertFalse(linter.has_errors)
self.assertTrue(linter.nb_warnings)


class CustomNamedAppGatherMigrationsTestCase(
BaseBackwardCompatibilityDetection, unittest.TestCase
):
databases = ["sqlite"]

def test_custom_named_app_gather_all_migrations(self):
linter = self._get_linter()
app_labels = [
migration.app_label for migration in linter._gather_all_migrations()
]
self.assertNotIn(fixtures.CUSTOM_APP_NAME, app_labels)
self.assertIn(DefaultConfig.label, app_labels)

def test_custom_named_app_gather_git_migrations(self):
linter = self._get_linter()
app_labels = [
migration.app_label
for migration in linter._gather_migrations_git(V014_REVISION)
]
self.assertNotIn(fixtures.CUSTOM_APP_NAME, app_labels)
self.assertIn(DefaultConfig.label, app_labels)
Empty file.
8 changes: 8 additions & 0 deletions tests/test_project/app_with_custom_name/apps.py
Original file line number Diff line number Diff line change
@@ -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 = "custom_name"
30 changes: 30 additions & 0 deletions tests/test_project/app_with_custom_name/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -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)),
],
)
]
Empty file.
8 changes: 8 additions & 0 deletions tests/test_project/app_with_custom_name/models.py
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions tests/test_project/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"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",
]

MIDDLEWARE = [
Expand Down