public/django-migration-linter
15
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Adapt 'warnings-as-errors' option to allow taking migration test codes as input

Browse Source
This commit is contained in:
David Wobrock committed 2022-03-13 17:26:40 +01:00
1 parent a04ecc078a
commit c7d9129db9
8 files changed
+114 -33
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

CHANGELOG.md
+1
View File
@@ -1,6 +1,7 @@
## 4.x.y (unreleased) ## 4.x.y (unreleased)
- Allow configuring logging for `makemigrations` command and unify behaviour with `lintmigrations` - Allow configuring logging for `makemigrations` command and unify behaviour with `lintmigrations`
- Adapt `--warnings-as-errors` option to allow selecting some migration tests only
## 4.0.0 ## 4.0.0
django_migration_linter/management/commands/lintmigrations.py
+12 -2
View File
@@ -11,7 +11,11 @@ from django.core.management.base import BaseCommand
from ...constants import __version__ from ...constants import __version__
from ...migration_linter import MessageType, MigrationLinter from ...migration_linter import MessageType, MigrationLinter
from ...sql_analyser.analyser import ANALYSER_STRING_MAPPING from ...sql_analyser.analyser import ANALYSER_STRING_MAPPING
from ..utils import configure_logging, register_linting_configuration_options from ..utils import (
configure_logging,
extract_warnings_as_errors_option,
register_linting_configuration_options,
)
CONFIG_NAME = "django_migration_linter" CONFIG_NAME = "django_migration_linter"
PYPROJECT_TOML = "pyproject.toml" PYPROJECT_TOML = "pyproject.toml"
@@ -155,6 +159,11 @@ class Command(BaseCommand):
if not options[k]: if not options[k]:
options[k] = v options[k] = v
(
warnings_as_errors_tests,
all_warnings_as_errors,
) = extract_warnings_as_errors_option(options["warnings_as_errors"])
linter = MigrationLinter( linter = MigrationLinter(
settings_path, settings_path,
ignore_name_contains=options["ignore_name_contains"], ignore_name_contains=options["ignore_name_contains"],
@@ -170,7 +179,8 @@ class Command(BaseCommand):
only_unapplied_migrations=options["unapplied_migrations"], only_unapplied_migrations=options["unapplied_migrations"],
exclude_migration_tests=options["exclude_migration_tests"], exclude_migration_tests=options["exclude_migration_tests"],
quiet=options["quiet"], quiet=options["quiet"],
warnings_as_errors=options["warnings_as_errors"], warnings_as_errors_tests=warnings_as_errors_tests,
all_warnings_as_errors=all_warnings_as_errors,
no_output=options["verbosity"] == 0, no_output=options["verbosity"] == 0,
analyser_string=options["sql_analyser"], analyser_string=options["sql_analyser"],
) )
django_migration_linter/management/commands/makemigrations.py
+12 -2
View File
@@ -9,7 +9,11 @@ from django.db.migrations.writer import MigrationWriter
from django_migration_linter import MigrationLinter from django_migration_linter import MigrationLinter
from ..utils import configure_logging, register_linting_configuration_options from ..utils import (
configure_logging,
extract_warnings_as_errors_option,
register_linting_configuration_options,
)
def ask_should_keep_migration(): def ask_should_keep_migration():
@@ -68,13 +72,19 @@ class Command(MakeMigrationsCommand):
else default_should_keep_migration else default_should_keep_migration
) )
(
warnings_as_errors_tests,
all_warnings_as_errors,
) = extract_warnings_as_errors_option(self.warnings_as_errors)
# Lint migrations # Lint migrations
linter = MigrationLinter( linter = MigrationLinter(
path=os.environ["DJANGO_SETTINGS_MODULE"], path=os.environ["DJANGO_SETTINGS_MODULE"],
database=self.database, database=self.database,
no_cache=True, no_cache=True,
exclude_migration_tests=self.exclude_migrations_tests, exclude_migration_tests=self.exclude_migrations_tests,
warnings_as_errors=self.warnings_as_errors, warnings_as_errors_tests=warnings_as_errors_tests,
all_warnings_as_errors=all_warnings_as_errors,
) )
for app_label, app_migrations in changes.items(): for app_label, app_migrations in changes.items():
django_migration_linter/management/utils.py
+17 -2
View File
@@ -21,8 +21,10 @@ def register_linting_configuration_options(parser):
parser.add_argument( parser.add_argument(
"--warnings-as-errors", "--warnings-as-errors",
action="store_true", type=str,
help="handle warnings as errors", nargs="*",
help="handle warnings as errors. Optionally specify the tests to handle as "
"errors (e.g. RUNPYTHON_REVERSIBLE)",
) )
@@ -35,3 +37,16 @@ def configure_logging(verbosity):
logger.disabled = True logger.disabled = True
else: else:
logging.basicConfig(format="%(message)s") logging.basicConfig(format="%(message)s")
def extract_warnings_as_errors_option(warnings_as_errors):
if isinstance(warnings_as_errors, list):
warnings_as_errors_tests = warnings_as_errors
# If the option is specified but without any test codes,
# all warnings become errors
all_warnings_as_errors = len(warnings_as_errors) == 0
else:
warnings_as_errors_tests = None
all_warnings_as_errors = False
return warnings_as_errors_tests, all_warnings_as_errors
django_migration_linter/migration_linter.py
+13 -3
View File
@@ -55,7 +55,8 @@ class MigrationLinter(object):
only_unapplied_migrations=False, only_unapplied_migrations=False,
exclude_migration_tests=None, exclude_migration_tests=None,
quiet=None, quiet=None,
warnings_as_errors=False, warnings_as_errors_tests=None,
all_warnings_as_errors=False,
no_output=False, no_output=False,
analyser_string=None, analyser_string=None,
): ):
@@ -74,7 +75,8 @@ class MigrationLinter(object):
self.only_applied_migrations = only_applied_migrations self.only_applied_migrations = only_applied_migrations
self.only_unapplied_migrations = only_unapplied_migrations self.only_unapplied_migrations = only_unapplied_migrations
self.quiet = quiet or [] self.quiet = quiet or []
self.warnings_as_errors = warnings_as_errors self.warnings_as_errors_tests = warnings_as_errors_tests
self.all_warnings_as_errors = all_warnings_as_errors
self.no_output = no_output self.no_output = no_output
self.sql_analyser_class = get_sql_analyser_class( self.sql_analyser_class = get_sql_analyser_class(
settings.DATABASES[self.database]["ENGINE"], settings.DATABASES[self.database]["ENGINE"],
@@ -179,9 +181,17 @@ class MigrationLinter(object):
if warnings_data: if warnings_data:
warnings += warnings_data warnings += warnings_data
if self.warnings_as_errors: if self.all_warnings_as_errors:
errors += warnings errors += warnings
warnings = [] warnings = []
elif self.warnings_as_errors_tests:
new_warnings = []
for w in warnings:
if w["code"] in self.warnings_as_errors_tests:
errors.append(w)
else:
new_warnings.append(w)
warnings = new_warnings
# Fixme: have a more generic approach to handling errors/warnings/ignored/ok? # Fixme: have a more generic approach to handling errors/warnings/ignored/ok?
if errors: if errors:
docs/makemigrations.md
+1 -1
View File
@@ -29,5 +29,5 @@ Deleted tests/test_project/app_correct/migrations/0003_a_column.py
Among the options that can be given, additionally to the default `makemigrations` options, you can count the options Among the options that can be given, additionally to the default `makemigrations` options, you can count the options
to configure what should be linted: to configure what should be linted:
* `--database DATABASE` - specify the database for which to generate the SQL. Defaults to *default*. * `--database DATABASE` - specify the database for which to generate the SQL. Defaults to *default*.
* `--warnings-as-errors` - handle warnings as errors and therefore return an error status code if we should. * `--warnings-as-errors [MIGRATION_TEST_CASE ...]` - handle warnings as errors. Optionally specify the selected tests using their code.
* `--exclude-migration-tests MIGRATION_TEST_CODE [...]` - specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). * `--exclude-migration-tests MIGRATION_TEST_CODE [...]` - specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN).
docs/usage.md
+21 -21
View File
@@ -19,27 +19,27 @@ The three main usages are:
Below the detailed command line options, which can all also be defined using a config file (`setup.cfg`, `tox.ini`, `pyproject.toml`, `.django_migration_linter.cfg`): Below the detailed command line options, which can all also be defined using a config file (`setup.cfg`, `tox.ini`, `pyproject.toml`, `.django_migration_linter.cfg`):
| Parameter | Description | | Parameter | Description |
|-------------------------------------------------------|-------------------------------------------------------------------------------------------------------| |-------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. | | `--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. |
| `--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. | | `--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. |
| `--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. | | `--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. |
| `--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. | | `--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. |
| `--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. | | `--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. |
| `--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. | | `--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. |
| `--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. | | `--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. |
| `--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). | | `--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). |
| `--verbosity or -v {0,1,2,3}` | Print more information during execution. | | `--verbosity or -v {0,1,2,3}` | Print more information during execution. |
| `--database DATABASE` | Specify the database for which to generate the SQL. Defaults to *default*. | | `--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. | | `--cache-path PATH` | specify a directory that should be used to store cache-files in. |
| `--no-cache` | Don't use a cache. | | `--no-cache` | Don't use a cache. |
| `--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. | | `--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. |
| `--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. | | `--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. |
| `--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. | | `--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. |
| `--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. | | `--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. |
| `--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. | | `--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. |
| `--warnings-as-errors` | Handle warnings as errors and therefore return an error status code if we should. | | `--warnings-as-errors [MIGRATION_TEST_CODE [...]]` | Handle warnings as errors and therefore return an error status code if we should. Optionally specify migration test codes to handle as errors. When no test code specified, all warnings are handled as errors. |
| `--sql-analyser` | Specify the SQL analyser that should be used. Allowed values: 'sqlite', 'mysql', 'postgresql'. | | `--sql-analyser` | Specify the SQL analyser that should be used. Allowed values: 'sqlite', 'mysql', 'postgresql'. |
## Django settings configuration ## Django settings configuration
tests/functional/test_data_migrations.py
+37 -2
View File
@@ -51,11 +51,11 @@ class DataMigrationDetectionTestCase(unittest.TestCase):
self.assertEqual(1, self.linter.nb_valid) self.assertEqual(1, self.linter.nb_valid)
self.assertFalse(self.linter.has_errors) self.assertFalse(self.linter.has_errors)
def test_warnings_as_errors(self): def test_all_warnings_as_errors(self):
self.linter = MigrationLinter( self.linter = MigrationLinter(
self.test_project_path, self.test_project_path,
include_apps=fixtures.DATA_MIGRATIONS, include_apps=fixtures.DATA_MIGRATIONS,
warnings_as_errors=True, all_warnings_as_errors=True,
) )
reverse_migration = self.linter.migration_loader.disk_migrations[ reverse_migration = self.linter.migration_loader.disk_migrations[
@@ -67,6 +67,41 @@ class DataMigrationDetectionTestCase(unittest.TestCase):
self.assertEqual(1, self.linter.nb_erroneous) self.assertEqual(1, self.linter.nb_erroneous)
self.assertTrue(self.linter.has_errors) self.assertTrue(self.linter.has_errors)
def test_warnings_as_errors_tests_matches(self):
self.linter = MigrationLinter(
self.test_project_path,
include_apps=fixtures.DATA_MIGRATIONS,
warnings_as_errors_tests=["RUNPYTHON_ARGS_NAMING_CONVENTION"],
)
reverse_migration = self.linter.migration_loader.disk_migrations[
("app_data_migrations", "0003_incorrect_arguments")
]
self.linter.lint_migration(reverse_migration)
self.assertEqual(0, self.linter.nb_warnings)
self.assertEqual(1, self.linter.nb_erroneous)
self.assertTrue(self.linter.has_errors)
def test_warnings_as_errors_tests_no_match(self):
self.linter = MigrationLinter(
self.test_project_path,
include_apps=fixtures.DATA_MIGRATIONS,
warnings_as_errors_tests=[
"RUNPYTHON_MODEL_IMPORT",
"RUNPYTHON_MODEL_VARIABLE_NAME",
],
)
reverse_migration = self.linter.migration_loader.disk_migrations[
("app_data_migrations", "0003_incorrect_arguments")
]
self.linter.lint_migration(reverse_migration)
self.assertEqual(1, self.linter.nb_warnings)
self.assertEqual(0, self.linter.nb_erroneous)
self.assertFalse(self.linter.has_errors)
class DataMigrationModelImportTestCase(unittest.TestCase): class DataMigrationModelImportTestCase(unittest.TestCase):
def test_missing_get_model_import(self): def test_missing_get_model_import(self):