diff --git a/README.md b/README.md index 4b2d7aa..f8ce72a 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ from myapp.db import Base @pytest.mark.integration class TestMyMigrations(MigrationTestBase): - """All five tests inherited automatically.""" + """All seven tests inherited automatically.""" @pytest.fixture def orm_metadata(self) -> MetaData: @@ -62,6 +62,8 @@ That's it! You now have: - `test_stairway_upgrade_downgrade` — each migration forward and back - `test_migrations_up_to_date` — schema matches ORM models +- `test_check_constraints_match` — CHECK constraints match ORM models, by name +- `test_enum_values_match` — enum values match ORM models, in order - `test_single_head_revision` — no unmerged branches - `test_downgrade_all_the_way` — full downgrade to base - `test_naming_conventions` — indexes and FKs follow conventions diff --git a/alembic_gauntlet/testing/base.py b/alembic_gauntlet/testing/base.py index d036834..feaf062 100644 --- a/alembic_gauntlet/testing/base.py +++ b/alembic_gauntlet/testing/base.py @@ -16,7 +16,7 @@ class MigrationTestBase( ): """Base class for database migration tests. - Inherit from this class and provide the following fixtures to get all five + Inherit from this class and provide the following fixtures to get all seven migration tests for free: Required fixtures: @@ -30,7 +30,10 @@ class MigrationTestBase( Optional class attributes: - ``migration_diff_ignore_tables: list[str]`` — table names to exclude from - schema diff and naming checks (e.g. auto-generated partition tables). + the schema diff, check constraint, enum and naming checks (e.g. auto-generated + partition tables). + - ``migration_diff_compare_server_default: bool`` — also compare server defaults + in the schema diff. Off by default; see ``MigrationConsistencyMixin``. - ``allowed_index_prefixes``, ``allowed_index_suffixes``, ``allowed_fk_prefixes``, ``allowed_fk_suffixes``, ``allowed_check_prefixes``, ``allowed_check_suffixes``, ``allowed_uq_prefixes``, ``allowed_uq_suffixes``, ``allowed_pk_prefixes``, diff --git a/alembic_gauntlet/testing/consistency_mixin.py b/alembic_gauntlet/testing/consistency_mixin.py index 71f87bb..02ca7e9 100644 --- a/alembic_gauntlet/testing/consistency_mixin.py +++ b/alembic_gauntlet/testing/consistency_mixin.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, ClassVar import pytest from alembic.autogenerate import compare_metadata @@ -11,7 +11,12 @@ from alembic.script import ScriptDirectory from sqlalchemy import text -from alembic_gauntlet.utils.diff import DEFAULT_IGNORE_TABLES, is_ignored_diff_item +from alembic_gauntlet.utils.diff import ( + DEFAULT_IGNORE_TABLES, + compare_check_constraints, + compare_enums, + is_ignored_diff_item, +) from alembic_gauntlet.utils.migrations import ( get_all_revisions, get_current_revision, @@ -31,7 +36,16 @@ class MigrationConsistencyMixin: - """Core migration correctness tests.""" + """Core migration correctness tests. + + Optional class attributes: + - ``migration_diff_compare_server_default: bool`` — pass ``compare_server_default`` + to Alembic in ``test_migrations_up_to_date``. Off by default, because a default + present on one side only — a Python-side ``default=`` in the model, a + ``server_default`` in the migration — is reported as drift. + """ + + migration_diff_compare_server_default: ClassVar[bool] = False async def test_stairway_upgrade_downgrade( self, @@ -96,7 +110,10 @@ def _run_check(sync_conn: Connection) -> MigrationDiff: sync_conn.execute(text("SELECT set_config('search_path', :s, true)"), {"s": quoted}) ctx = MigrationContext.configure( sync_conn, - opts={"version_table_schema": isolated_migration_schema}, + opts={ + "version_table_schema": isolated_migration_schema, + "compare_server_default": self.migration_diff_compare_server_default, + }, ) diff = compare_metadata(ctx, orm_metadata) assert isinstance(diff, list) @@ -110,6 +127,58 @@ def _run_check(sync_conn: Connection) -> MigrationDiff: "Run: alembic revision --autogenerate" ) + async def test_check_constraints_match( + self, + alembic_config: Config, + migration_engine: AsyncEngine, + isolated_migration_schema: str, + orm_metadata: MetaData, + ) -> None: + """Verify the CHECK constraints after a full upgrade match the SQLAlchemy ORM metadata, by name. + + ``compare_metadata()`` never compares CHECK constraints, so a constraint a + migration forgot is invisible to ``test_migrations_up_to_date``. + """ + await run_alembic_upgrade( + migration_engine, + alembic_config, + target_schema=isolated_migration_schema, + ) + + ignore_tables = frozenset(getattr(self, "migration_diff_ignore_tables", ())) + async with migration_engine.connect() as conn: + differences = await conn.run_sync( + lambda sc: compare_check_constraints(sc, orm_metadata, isolated_migration_schema, ignore_tables) + ) + + assert not differences, "CHECK constraints are out of sync with ORM models:\n" + "\n".join(differences) + + async def test_enum_values_match( + self, + alembic_config: Config, + migration_engine: AsyncEngine, + isolated_migration_schema: str, + orm_metadata: MetaData, + ) -> None: + """Verify every native ``Enum`` column's values after a full upgrade match the type in the database, in order. + + ``compare_metadata()`` never compares enum members, so a value a migration + forgot to add is invisible to ``test_migrations_up_to_date``. + """ + await run_alembic_upgrade( + migration_engine, + alembic_config, + target_schema=isolated_migration_schema, + ) + + ignore_tables = frozenset(getattr(self, "migration_diff_ignore_tables", ())) + async with migration_engine.connect() as conn: + differences = await conn.run_sync( + lambda sc: compare_enums(sc, orm_metadata, isolated_migration_schema, ignore_tables) + ) + + assert not differences, "Enum values are out of sync with ORM models:\n" + "\n".join(differences) + async def test_single_head_revision(self, alembic_config: Config) -> None: """Verify there is exactly one head revision (no unmerged branches).""" script = ScriptDirectory.from_config(alembic_config) diff --git a/alembic_gauntlet/utils/diff.py b/alembic_gauntlet/utils/diff.py index 24fcd30..6f8b4bb 100644 --- a/alembic_gauntlet/utils/diff.py +++ b/alembic_gauntlet/utils/diff.py @@ -1,7 +1,17 @@ -"""Schema diff filtering for migration consistency tests.""" +"""Schema diff helpers for migration consistency tests.""" from __future__ import annotations +from typing import TYPE_CHECKING + +from alembic.util.sqla_compat import _get_constraint_final_name +from sqlalchemy import CheckConstraint, Enum, inspect +from sqlalchemy.dialects.postgresql.base import PGInspector + +if TYPE_CHECKING: + from sqlalchemy import MetaData + from sqlalchemy.engine import Connection + DEFAULT_IGNORE_TABLES: frozenset[str] = frozenset({"alembic_version"}) @@ -26,3 +36,102 @@ def is_ignored_diff_item(diff_item: tuple, ignore_tables: frozenset[str]) -> boo name = getattr(table, "name", None) if table is not None else None return name in ignore_tables if name else False return False + + +def compare_check_constraints( + sync_conn: Connection, + metadata: MetaData, + schema: str, + ignore_tables: frozenset[str] = frozenset(), +) -> list[str]: + """Compare the CHECK constraints ``metadata`` declares with the ones in ``schema``, by name. + + Alembic's ``compare_metadata()`` never looks at CHECK constraints. Names are resolved + the way the DDL would render them — the metadata's naming convention applied, a + deferred name such as ``Boolean(create_constraint=True)`` filled in, anything over 63 + characters truncated — through the same Alembic helper autogenerate uses for index + and unique constraint names. + + A named constraint the models declare and the database lacks is always reported. + One the database has and the models do not is reported unless the table also + declares an unnamed check constraint, which carries whatever name PostgreSQL gave + it. Unnamed constraints are never compared. + + Args: + sync_conn: Synchronous SQLAlchemy connection. + metadata: SQLAlchemy ``MetaData`` of the ORM models. + schema: PostgreSQL schema the migrations ran in. + ignore_tables: Table names to skip. + + Returns: + One line per difference; empty when the constraints match. + """ + inspector = inspect(sync_conn) + differences: list[str] = [] + for table in metadata.sorted_tables: + if table.name in ignore_tables: + continue + names = { + _get_constraint_final_name(constraint, sync_conn.dialect) + for constraint in table.constraints + if isinstance(constraint, CheckConstraint) + } + expected = {name for name in names if name} + actual = {name for c in inspector.get_check_constraints(table.name, schema=schema) if (name := c["name"])} + for name in sorted(expected - actual): + differences.append( + f"Check constraint '{name}' on table '{table.name}' is in the models but not in the database." + ) + if None not in names: + for name in sorted(actual - expected): + differences.append( + f"Check constraint '{name}' on table '{table.name}' is in the database but not in the models." + ) + return differences + + +def compare_enums( + sync_conn: Connection, + metadata: MetaData, + schema: str, + ignore_tables: frozenset[str] = frozenset(), +) -> list[str]: + """Compare the values of every native ``Enum`` column in ``metadata`` with its type in the database. + + Alembic's ``compare_metadata()`` never looks at enum members. Values are compared as + ordered lists, so a value added in the wrong position counts as a difference. The type + is looked up in ``Enum.schema`` when set, otherwise in ``schema``. Non-native and + unnamed enums have no type in the database and are skipped; a type in the database + that no column uses is not reported. + + Args: + sync_conn: Synchronous SQLAlchemy connection. + metadata: SQLAlchemy ``MetaData`` of the ORM models. + schema: PostgreSQL schema the migrations ran in. + ignore_tables: Table names to skip. + + Returns: + One line per difference; empty when the enums match. + """ + inspector = inspect(sync_conn) + assert isinstance(inspector, PGInspector) + expected: dict[tuple[str, str], list[str]] = {} + for table in metadata.sorted_tables: + if table.name in ignore_tables: + continue + for column in table.columns: + enum = column.type + if isinstance(enum, Enum) and enum.native_enum and enum.name: + expected[(enum.schema or schema, enum.name)] = list(enum.enums) + actual = {(e["schema"], e["name"]): e["labels"] for e in inspector.get_enums(schema="*")} + + differences: list[str] = [] + for (type_schema, name), values in sorted(expected.items()): + labels = actual.get((type_schema, name)) + if labels is None: + differences.append( + f"Enum type '{name}' in schema '{type_schema}' is in the models but not in the database." + ) + elif labels != values: + differences.append(f"Enum type '{name}' has values {labels} in the database and {values} in the models.") + return differences diff --git a/docs/agents.md b/docs/agents.md index 56f7bfc..2ea98e0 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -10,7 +10,7 @@ | Requires | Python 3.10+, PostgreSQL, SQLAlchemy 2, Alembic 1.8+, pytest 7+ | | Install | `pip install "alembic-gauntlet[asyncio]"` · extras: `asyncio` (pytest-asyncio), `testcontainers` | | Also install | an async PostgreSQL driver (`asyncpg`); `pytest-asyncio` comes with the `asyncio` extra and has to run in `asyncio_mode = "auto"` | -| Entry point | `MigrationTestBase` — inherit it, supply two fixtures, get five tests | +| Entry point | `MigrationTestBase` — inherit it, supply two fixtures, get seven tests | | Pytest plugin | `alembic_gauntlet.fixtures` is registered under `pytest11`, so `alembic_config` and `migration_engine` exist with no import and no conftest entry | | Async / sync | everything that touches the database is a coroutine over an `AsyncEngine`; the naming, diff and validation helpers are ordinary sync functions | | Source | | @@ -35,12 +35,13 @@ a method that sounds plausible. ## Scope **It does** run your real Alembic history against a real PostgreSQL database, inside a -throwaway schema, as five pytest tests you inherit: every revision up and back down one +throwaway schema, as seven pytest tests you inherit: every revision up and back down one step at a time, a full downgrade to base, an autogenerate diff of the migrated database -against your ORM metadata, a single-head check, and a naming check over every index, -foreign key, check, unique and primary key constraint. The pieces underneath — upgrade, -downgrade, current revision, all revisions, an isolated schema — are public, so you can -write your own checks with them. +against your ORM metadata, a name-by-name comparison of its CHECK constraints, a +value-by-value comparison of its enum types, a single-head check, and a naming check over +every index, foreign key, check, unique and primary key constraint. The pieces underneath +— upgrade, downgrade, current revision, all revisions, an isolated schema, the two +comparisons — are public, so you can write your own checks with them. **It does not** create the database, write your `env.py`, or run migrations anywhere but a test. It never shells out to the `alembic` command; it drives `alembic.command` in-process @@ -73,7 +74,7 @@ Five nouns and one contract. database in the wrong schema and every test above becomes theatre. See [Configuring env.py](guide/env-py.md). * **`MigrationTestBase`** is `MigrationSchemaMixin` + `MigrationConsistencyMixin` + - `MigrationNamingMixin`. Inherit the mixins directly when you want fewer than five tests; + `MigrationNamingMixin`. Inherit the mixins directly when you want fewer than seven tests; `MigrationSchemaMixin` has to be among them, because the other two request `isolated_migration_schema`. @@ -127,7 +128,7 @@ asyncio_mode = "auto" asyncio_default_fixture_loop_scope = "function" ``` -That is the whole integration: five tests, named in the table below, collected from the +That is the whole integration: seven tests, named in the table below, collected from the base class. ## The API @@ -136,7 +137,7 @@ base class. | Name | Kind | Signature and result | |---|---|---| -| `MigrationTestBase` | class | the base class carrying all five tests | +| `MigrationTestBase` | class | the base class carrying all seven tests | | `run_alembic_upgrade` | coroutine | `(engine, alembic_config, target_schema="public", revision="head") -> None` | | `run_alembic_downgrade` | coroutine | `(engine, alembic_config, target_schema="public", revision="base") -> None` | | `get_current_revision` | coroutine | `(engine, target_schema="public") -> str | None` — `None` means base | @@ -154,7 +155,7 @@ pass them by name. |---|---| | `MigrationTestBase` | the three mixins, plus `migration_diff_ignore_tables: ClassVar[list[str]] = []` | | `MigrationSchemaMixin` | the `isolated_migration_schema` fixture, and nothing else | -| `MigrationConsistencyMixin` | `test_stairway_upgrade_downgrade`, `test_migrations_up_to_date`, `test_single_head_revision`, `test_downgrade_all_the_way` | +| `MigrationConsistencyMixin` | `test_stairway_upgrade_downgrade`, `test_migrations_up_to_date`, `test_check_constraints_match`, `test_enum_values_match`, `test_single_head_revision`, `test_downgrade_all_the_way`, and `migration_diff_compare_server_default: ClassVar[bool] = False` | | `MigrationNamingMixin` | `test_naming_conventions`, the ten `allowed_*` class attributes and the rule resolution | | `MigrationDiff` | type alias `list[tuple[MigrateOperation, ...]]` — what `compare_metadata()` returns | @@ -169,12 +170,14 @@ pass them by name. | `migration_engine` | plugin | function | `AsyncEngine` with `NullPool`, disposed afterwards | | `isolated_migration_schema` | `MigrationSchemaMixin` | function | `test_mig_<8 hex chars>`, dropped with `CASCADE` | -### The five tests +### The seven tests | Test | Requests | Asserts | |---|---|---| | `test_stairway_upgrade_downgrade` | config, engine, schema | for each revision base → head: upgrade to it, current revision matches, downgrade one step, current revision matches the step below, upgrade back | -| `test_migrations_up_to_date` | config, engine, schema, `orm_metadata` | after a full upgrade, `compare_metadata()` against your metadata is empty once ignored tables are filtered out | +| `test_migrations_up_to_date` | config, engine, schema, `orm_metadata` | after a full upgrade, `compare_metadata()` against your metadata is empty once ignored tables are filtered out; server defaults take part only when `migration_diff_compare_server_default` is on | +| `test_check_constraints_match` | config, engine, schema, `orm_metadata` | after a full upgrade, every named CHECK constraint in your metadata exists under the name the DDL would give it, and a table whose model names all of its check constraints has no others | +| `test_enum_values_match` | config, engine, schema, `orm_metadata` | after a full upgrade, every native, named `Enum` column's values equal the database type's labels, in order | | `test_single_head_revision` | config | `ScriptDirectory.get_revisions("heads")` has exactly one entry | | `test_downgrade_all_the_way` | config, engine, schema | upgrade to head, then step down through every revision to base; the final current revision is `None` | | `test_naming_conventions` | config, engine, schema, `orm_metadata` | after a full upgrade, every index, foreign key, check, unique and primary key name in the schema matches the resolved rules | @@ -187,7 +190,8 @@ message — there is no custom exception for a failed check. | Attribute | Default | Effect | |---|---|---| -| `migration_diff_ignore_tables` | `[]` | added to `DEFAULT_IGNORE_TABLES` (`{"alembic_version"}`) for the diff test, and drops those tables from the naming test | +| `migration_diff_ignore_tables` | `[]` | added to `DEFAULT_IGNORE_TABLES` (`{"alembic_version"}`) for the diff test, and drops those tables from the check constraint, enum and naming tests | +| `migration_diff_compare_server_default` | `False` | passes `compare_server_default` to Alembic in the diff test; rule 9 below says what agrees and what is reported | | `allowed_index_prefixes` | `["idx_", "uq_"]` | index names | | `allowed_index_suffixes` | `["_idx", "_pkey", "_key"]` | index names, one optional trailing digit tolerated (`users_pkey1`) | | `allowed_fk_prefixes` / `allowed_fk_suffixes` | `["fk_"]` / `["_fkey"]` | foreign key constraint names | @@ -211,6 +215,8 @@ because PostgreSQL implements a unique constraint as an index. | `alembic_gauntlet.utils.naming.validate_naming_results` | `(results, allowed_index_prefixes, …, allowed_pk_suffixes) -> None` | asserts; every `allowed_*` argument is required | | `alembic_gauntlet.utils.diff.is_ignored_diff_item` | `(diff_item, ignore_tables) -> bool` | filters `remove_table` and `remove_index` items only | | `alembic_gauntlet.utils.diff.DEFAULT_IGNORE_TABLES` | `frozenset({"alembic_version"})` | the baseline of the diff filter | +| `alembic_gauntlet.utils.diff.compare_check_constraints` | `(sync_conn, metadata, schema, ignore_tables=frozenset()) -> list[str]` | **sync**; one line per CHECK constraint that differs by name, empty when in sync | +| `alembic_gauntlet.utils.diff.compare_enums` | `(sync_conn, metadata, schema, ignore_tables=frozenset()) -> list[str]` | **sync**; one line per enum type whose values differ, in order, empty when in sync | | `alembic_gauntlet.utils.validation.validate_schema_name` | `(name, connection=None) -> None` | format, 63-byte length, and reserved words when a connection is given | | `alembic_gauntlet.utils.validation.get_pg_reserved_words` | `(connection) -> set[str]` | reads `pg_get_keywords()` | | `alembic_gauntlet.utils.convention.rules_from_metadata` | `(metadata) -> NamingConventionRules` | the layer-two extraction, as a dataclass of ten lists | @@ -251,32 +257,62 @@ and `pk_constraint`. 7. **The isolated schema is dropped with `CASCADE`.** Everything a migration created inside it is gone at the end of the test — and nothing it created outside it is cleaned up at all. -8. **`migration_diff_ignore_tables` only silences removals.** `is_ignored_diff_item` - recognises `remove_table` and `remove_index`, meaning tables in the database that your - models do not know about. A table your models declare and the migrations never created - is always reported, whatever you list. -9. **A name passes on a prefix *or* a suffix.** The checks are not per-object-type +8. **`migration_diff_ignore_tables` only silences removals in the diff test.** + `is_ignored_diff_item` recognises `remove_table` and `remove_index`, meaning tables in + the database that your models do not know about. A table your models declare and the + migrations never created is always reported, whatever you list. The check constraint, + enum and naming tests skip the listed tables outright. +9. **Server defaults are compared only on request.** `compare_metadata()` skips them + unless `compare_server_default` is set, and the diff test passes + `migration_diff_compare_server_default`, which is `False`. With it on, Alembic's + PostgreSQL comparison compares the two texts and, when they differ, asks the server + whether the expressions are equal: `text("true")`, `sa.true()` and `"true"` all agree + with a column defaulted to `true`, `func.now()` agrees with `now()` and + `CURRENT_TIMESTAMP`, `"0"` with `0`, `text("'{}'")` with `'{}'::jsonb`, and + `gen_random_uuid()` with itself. Reported: a different value, two different volatile + functions (`clock_timestamp()` against `now()`), and a default present on one side only + — a Python-side `default=` in the model is not a server default. A serial or identity + primary key is never compared. +10. **CHECK constraints are compared by name, and only named ones.** + `test_check_constraints_match` resolves each metadata constraint to the name the DDL + would give it — the convention applied, a deferred `Boolean(create_constraint=True)` + name filled in, a name over 63 characters truncated — and compares that with + `get_check_constraints`. Expressions are never compared; PostgreSQL rewrites + `amount > 0` as `(amount > (0)::numeric)`. An unnamed metadata constraint is skipped, + and on its table the test also stops reporting database constraints the models lack, + because PostgreSQL gave the unnamed one a name of its own. Name every check constraint; + a `ck` template with `%(constraint_name)s` enforces that. Alembic's own name-based + detection is a plugin that was on by default in 1.19.0 and 1.19.1 and is opt-in from + 1.19.2; this test does not use it, and on those two versions the diff test reports + named CHECK constraints as well. +11. **Enum values are compared in order, per type the models use.** + `test_enum_values_match` reads `get_enums()` and compares the labels with `Enum.enums` + as lists for every native, named `Enum` column, looking in `Enum.schema` when set and + in the isolated schema otherwise. A type nothing references, a non-native enum and an + unnamed one are not compared. A migration that adds a value with `ALTER TYPE … ADD + VALUE` has to put it where the model has it. +12. **A name passes on a prefix *or* a suffix.** The checks are not per-object-type exclusive and not anchored to your convention: `users_pkey` passes the primary key rule on `_pkey` even when you set `allowed_pk_prefixes = ["pk_"]`, because the default suffix is still in the resolved set. Empty both lists for a category and nothing can pass it. -10. **Layer two replaces, layer three overrides, and the walk stops at the mixin.** An +13. **Layer two replaces, layer three overrides, and the walk stops at the mixin.** An explicit `allowed_*` attribute counts when it is set on your class or an intermediate base; the MRO walk breaks at `MigrationNamingMixin`, so its own sentinel values never win. A convention template with no literal part — `"%(table_name)s_%(column_0_name)s"` — contributes nothing and leaves the defaults in place. -11. **PostgreSQL only.** Isolated schemas, `pg_get_keywords()`, `DROP SCHEMA … CASCADE` +14. **PostgreSQL only.** Isolated schemas, `pg_get_keywords()`, `DROP SCHEMA … CASCADE` and the constraint inspection are PostgreSQL. There is no SQLite or MySQL path, and a `cockroachdb+asyncpg` URL is not a supported target. -12. **Mixin fixtures are class fixtures.** `isolated_migration_schema` is defined on +15. **Mixin fixtures are class fixtures.** `isolated_migration_schema` is defined on `MigrationSchemaMixin`, so it exists only inside a class that inherits it. A module-level test function cannot request it; call `create_isolated_migration_schema` instead. -13. **`create_isolated_migration_schema` is an async generator, not a context manager.** +16. **`create_isolated_migration_schema` is an async generator, not a context manager.** Drive it with `async for`, or wrap it in your own fixture; it has no `__aenter__`, so `async with` fails before the schema is ever created. -14. **Keep `NullPool` if you replace `migration_engine`.** Function scope plus `NullPool` +17. **Keep `NullPool` if you replace `migration_engine`.** Function scope plus `NullPool` is what keeps a schema-scoped `search_path` from leaking into the next test and what makes `pytest -n auto` safe. -15. **Schema names are validated before they reach SQL.** `validate_schema_name` runs on +18. **Schema names are validated before they reach SQL.** `validate_schema_name` runs on every `target_schema` the runners are given, and rejects anything that is not a plain identifier, is longer than 63 characters, or is a PostgreSQL reserved word. Do not build schema names from unvalidated input and interpolate them yourself. @@ -327,6 +363,27 @@ def migration_db_url() -> str: # asyncio_mode = "auto" ``` +```python +# WRONG — expecting the diff test to report a wrong server default +class TestMigrations(MigrationTestBase): + ... # passes with is_active DEFAULT false where the model says true + +# RIGHT — turn the comparison on; CHECK constraints and enum values have their own tests +class TestMigrations(MigrationTestBase): + migration_diff_compare_server_default = True +``` + +```python +# WRONG — an unnamed CHECK constraint is never compared, and a migration that spells the +# conventional name out gets the convention applied to it again (chk_orders_chk_orders_…) +__table_args__ = (CheckConstraint("amount > 0"),) +op.create_table("orders", ..., sa.CheckConstraint("amount > 0", name="chk_orders_amount_positive")) + +# RIGHT — name it in the model and let the convention resolve it; op.f() keeps a name as written +__table_args__ = (CheckConstraint("amount > 0", name="amount_positive"),) # chk_orders_amount_positive +op.create_table("orders", ..., sa.CheckConstraint("amount > 0", name=op.f("chk_orders_amount_positive"))) +``` + ```python # WRONG — an env.py that builds its own engine and ignores the injected one def run_migrations_online() -> None: @@ -370,7 +427,7 @@ Fetch a page when the task is the one named beside it. | Page | Read it when | |---|---| | [Home](index.md) | the one-paragraph pitch and the shortest possible example | -| [Quick start](guide/quickstart.md) | setting the suite up step by step, and what each of the five tests catches | +| [Quick start](guide/quickstart.md) | setting the suite up step by step, and what each of the seven tests catches | | [Configuration](guide/configuration.md) | every fixture and class attribute, with worked examples and the naming resolution order | | [Configuring env.py](guide/env-py.md) | the `connection` / `target_schema` contract, `SET LOCAL`, advisory locks, `include_object` filtering | | [Advanced](guide/advanced.md) | composing mixins by hand, custom checks on the isolated schema, partitioned tables, CI shapes | diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index 8edfd78..b1a32f0 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -49,6 +49,8 @@ class TestConsistency(MigrationSchemaMixin, MigrationConsistencyMixin): Now you get: - ✅ `test_stairway_upgrade_downgrade` - ✅ `test_migrations_up_to_date` +- ✅ `test_check_constraints_match` +- ✅ `test_enum_values_match` - ✅ `test_single_head_revision` - ✅ `test_downgrade_all_the_way` - ❌ `test_naming_conventions` (not included) diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index c63dac0..d2f0a9a 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -19,7 +19,8 @@ def orm_metadata(self) -> MetaData: return Base.metadata ``` -**When to use**: Always required for `test_migrations_up_to_date` and `test_naming_conventions`. +**When to use**: Always required for `test_migrations_up_to_date`, `test_check_constraints_match`, +`test_enum_values_match` and `test_naming_conventions`. ### migration_db_url @@ -97,7 +98,8 @@ async def migration_engine(self, migration_db_url: str) -> AsyncEngine: ### migration_diff_ignore_tables -**Purpose**: Ignore specific tables in schema consistency checks. +**Purpose**: Ignore specific tables in the schema consistency, CHECK constraint, enum and +naming checks. ```python from typing import ClassVar @@ -114,6 +116,42 @@ class TestMyMigrations(MigrationTestBase): - External tables (e.g., PostGIS extension tables) - Temporary tables +### migration_diff_compare_server_default + +**Purpose**: Compare server defaults in `test_migrations_up_to_date`. + +```python +from typing import ClassVar + + +class TestMyMigrations(MigrationTestBase): + migration_diff_compare_server_default: ClassVar[bool] = True +``` + +**Default**: `False` — Alembic's `compare_metadata()` skips server defaults unless asked, +so a migration that says `server_default=sa.text("false")` where the model says +`text("true")` passes the diff test until you turn this on. + +**What agrees**: on PostgreSQL, Alembic compares the two texts and, when they differ, asks +the server whether the expressions are equal, so the spelling rarely matters: + +| Database default | Model spellings that agree | +|------------------|----------------------------| +| `true` | `text("true")`, `sa.true()`, `"true"`, `text("TRUE")`, `"1"` | +| `now()` | `func.now()`, `text("now()")`, `text("CURRENT_TIMESTAMP")`, `func.current_timestamp()` | +| `0` | `"0"`, `text("0")`, `"'0'"` | +| `'pending'::character varying` | `"pending"`, `text("'pending'")` | +| `'{}'::jsonb` | `text("'{}'::jsonb")`, `text("'{}'")`, `"{}"` | +| `gen_random_uuid()` | `text("gen_random_uuid()")`, `func.gen_random_uuid()` | + +**What is reported**: +- A different value: `false` in the database, `text("true")` in the model +- Two different volatile functions: `clock_timestamp()` against `now()` +- A default on one side only — a Python-side `default=True` in the model is not a server + default, so a migration with `server_default` and a model without one is drift + +A serial or identity primary key is never compared. + ### allowed_index_prefixes **Purpose**: Allowed prefixes for index names. diff --git a/docs/guide/quickstart.md b/docs/guide/quickstart.md index 0f7a677..2987915 100644 --- a/docs/guide/quickstart.md +++ b/docs/guide/quickstart.md @@ -56,10 +56,12 @@ class TestMyMigrations(MigrationTestBase): return "postgresql+asyncpg://user:pass@localhost:5432/testdb" ``` -That's it! You now have 5 tests automatically: +That's it! You now have 7 tests automatically: - `test_stairway_upgrade_downgrade` — each migration forward and back - `test_migrations_up_to_date` — schema matches ORM models +- `test_check_constraints_match` — CHECK constraints match ORM models, by name +- `test_enum_values_match` — enum values match ORM models, in order - `test_single_head_revision` — no unmerged branches - `test_downgrade_all_the_way` — full downgrade to base - `test_naming_conventions` — indexes and FKs follow conventions @@ -114,6 +116,8 @@ When tests pass, you'll see: ``` tests/migrations/test_migrations.py::TestMyMigrations::test_stairway_upgrade_downgrade PASSED tests/migrations/test_migrations.py::TestMyMigrations::test_migrations_up_to_date PASSED +tests/migrations/test_migrations.py::TestMyMigrations::test_check_constraints_match PASSED +tests/migrations/test_migrations.py::TestMyMigrations::test_enum_values_match PASSED tests/migrations/test_migrations.py::TestMyMigrations::test_single_head_revision PASSED tests/migrations/test_migrations.py::TestMyMigrations::test_downgrade_all_the_way PASSED tests/migrations/test_migrations.py::TestMyMigrations::test_naming_conventions PASSED @@ -151,6 +155,42 @@ This test: - Model changes not reflected in migrations - Drift between database and code +**Does not catch** what `compare_metadata()` does not compare: server defaults, unless +you set `migration_diff_compare_server_default = True` (see +[Configuration](configuration.md#migration_diff_compare_server_default) for which +spellings agree), CHECK constraints and enum values. The next two tests cover those. + +### test_check_constraints_match + +**CHECK constraint check** — ensures every CHECK constraint your models declare exists. + +This test: +1. Runs all migrations to HEAD +2. Resolves each named `CheckConstraint` in your metadata to the name the DDL would give it +3. Compares those names with the constraints on each table + +**Catches**: +- A CHECK constraint the model declares and no migration created +- A CHECK constraint left in the database after the model dropped it +- A migration that named the constraint differently from the model + +Unnamed constraints are not compared, and expressions are never compared — PostgreSQL +rewrites them. Name every CHECK constraint, ideally through a `ck` naming convention. + +### test_enum_values_match + +**Enum value check** — ensures every enum type has the values your models have. + +This test: +1. Runs all migrations to HEAD +2. Reads every enum type from the database +3. Compares the values of each native `Enum` column with the type's labels, in order + +**Catches**: +- A value added to the model and never added with `ALTER TYPE ... ADD VALUE` +- A value the migration created that the model does not have +- A value added in the wrong position + ### test_single_head_revision **Branch detection** — ensures no unmerged migration branches. @@ -253,6 +293,19 @@ Your migrations are out of sync with ORM models. Run: alembic revision --autogenerate -m "sync models" ``` +### Test fails: "CHECK constraints are out of sync" + +A named CHECK constraint is in your models and not in the database, or the other way +round. The message names it. Add the migration, or name the constraint the same way on +both sides — under a `ck` naming convention, `op.create_check_constraint("amount_positive", ...)` +resolves to the same name as the model. + +### Test fails: "Enum values are out of sync" + +The message shows the values in the database and in the model. Add a migration with +`op.execute("ALTER TYPE order_status ADD VALUE 'shipped'")`, positioned with +`BEFORE`/`AFTER` where the model has it. + ### Test fails: "Multiple head revisions" You have unmerged migration branches. Merge them: diff --git a/docs/index.md b/docs/index.md index 16a8635..bc586fb 100644 --- a/docs/index.md +++ b/docs/index.md @@ -8,6 +8,7 @@ ✅ **Stairway tests** — verify every migration can upgrade and downgrade ✅ **Schema consistency** — ensure migrations match your ORM models +✅ **CHECK constraints and enum values** — compared where autogenerate does not look ✅ **Naming conventions** — validate index and foreign key names ✅ **Branch detection** — catch unmerged migration branches ✅ **Isolated schemas** — parallel-safe test execution @@ -48,7 +49,7 @@ from myapp.db import Base @pytest.mark.integration class TestMyMigrations(MigrationTestBase): - """All five tests inherited automatically.""" + """All seven tests inherited automatically.""" @pytest.fixture def orm_metadata(self) -> MetaData: @@ -63,6 +64,8 @@ That's it! You now have: - `test_stairway_upgrade_downgrade` — each migration forward and back - `test_migrations_up_to_date` — schema matches ORM models +- `test_check_constraints_match` — CHECK constraints match ORM models, by name +- `test_enum_values_match` — enum values match ORM models, in order - `test_single_head_revision` — no unmerged branches - `test_downgrade_all_the_way` — full downgrade to base - `test_naming_conventions` — indexes and FKs follow conventions @@ -80,6 +83,7 @@ Alembic migrations are powerful but error-prone. Common issues: - ❌ Migration works forward but breaks on downgrade - ❌ Forgot to run `alembic revision --autogenerate` after model changes +- ❌ A CHECK constraint or an enum value the migration got wrong and autogenerate never reports - ❌ Unmerged migration branches cause conflicts - ❌ Inconsistent naming breaks your team's conventions diff --git a/docs/reference/index.md b/docs/reference/index.md index 505a65f..90591dd 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -66,6 +66,8 @@ Auto-generated API documentation from source code using [mkdocstrings](https://m heading_level: 4 members: - is_ignored_diff_item + - compare_check_constraints + - compare_enums - DEFAULT_IGNORE_TABLES ### Naming utilities diff --git a/tests/integration/sample_app_with_drift/alembic.ini b/tests/integration/sample_app_with_drift/alembic.ini new file mode 100644 index 0000000..8434cc2 --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic.ini @@ -0,0 +1,6 @@ +[alembic] +# Paths are set dynamically in tests — these placeholders are never used at runtime. +script_location = tests/integration/sample_app_with_drift/alembic +version_locations = tests/integration/sample_app_with_drift/alembic/versions/clean +path_separator = os +sqlalchemy.url = postgresql+asyncpg://user:pass@localhost/dbname diff --git a/tests/integration/sample_app_with_drift/alembic/env.py b/tests/integration/sample_app_with_drift/alembic/env.py new file mode 100644 index 0000000..37076a6 --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic/env.py @@ -0,0 +1,49 @@ +"""Alembic env.py for the drift sample app (issue #37).""" + +from __future__ import annotations + +import asyncio +import os + +from alembic import context +from sqlalchemy import text +from sqlalchemy.engine import Connection +from sqlalchemy.ext.asyncio import create_async_engine +from sqlalchemy.pool import NullPool + +from tests.integration.sample_app_with_drift.models import Base + +config = context.config + +target_schema: str = config.attributes.get("target_schema") or os.getenv("MIGRATION_SCHEMA", "public") + +target_metadata = Base.metadata + + +def do_run_migrations(connection: Connection) -> None: + if target_schema != "public": + connection.execute(text(f'SET search_path TO "{target_schema}"')) + + context.configure( + connection=connection, + target_metadata=target_metadata, + version_table_schema=target_schema, + include_schemas=False, + ) + + with context.begin_transaction(): + context.run_migrations() + + +async def run_migrations_online() -> None: + url = config.get_main_option("sqlalchemy.url", "") + engine = create_async_engine(url, poolclass=NullPool) + async with engine.connect() as conn: + await conn.run_sync(do_run_migrations) + await engine.dispose() + + +if config.attributes.get("connection") is not None: + do_run_migrations(config.attributes["connection"]) +else: + asyncio.run(run_migrations_online()) diff --git a/tests/integration/sample_app_with_drift/alembic/versions/check_missing/001_users_and_orders.py b/tests/integration/sample_app_with_drift/alembic/versions/check_missing/001_users_and_orders.py new file mode 100644 index 0000000..13bb498 --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic/versions/check_missing/001_users_and_orders.py @@ -0,0 +1,43 @@ +"""Create users and orders without the amount check the model declares. + +Revision ID: 0001 +Revises: +Create Date: 2026-09-07 00:00:00.000000 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision: str = "0001" +down_revision: str | None = None +branch_labels: str | None = None +depends_on: str | None = None + +order_status = sa.Enum("new", "paid", "shipped", name="order_status") + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("email", sa.String(255), nullable=False), + sa.Column("is_active", sa.Boolean(), server_default=sa.text("true"), nullable=False), + sa.PrimaryKeyConstraint("id", name="pk_users"), + ) + op.create_table( + "orders", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("amount", sa.Numeric(12, 2), nullable=False), + sa.Column("status", order_status, nullable=False), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name="fk_orders_user_id_users"), + sa.PrimaryKeyConstraint("id", name="pk_orders"), + ) + + +def downgrade() -> None: + op.drop_table("orders") + op.drop_table("users") + order_status.drop(op.get_bind()) diff --git a/tests/integration/sample_app_with_drift/alembic/versions/clean/001_users_and_orders.py b/tests/integration/sample_app_with_drift/alembic/versions/clean/001_users_and_orders.py new file mode 100644 index 0000000..f5ff9a5 --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic/versions/clean/001_users_and_orders.py @@ -0,0 +1,44 @@ +"""Create users and orders, as the models describe them. + +Revision ID: 0001 +Revises: +Create Date: 2026-09-07 00:00:00.000000 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision: str = "0001" +down_revision: str | None = None +branch_labels: str | None = None +depends_on: str | None = None + +order_status = sa.Enum("new", "paid", "shipped", name="order_status") + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("email", sa.String(255), nullable=False), + sa.Column("is_active", sa.Boolean(), server_default=sa.text("true"), nullable=False), + sa.PrimaryKeyConstraint("id", name="pk_users"), + ) + op.create_table( + "orders", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("amount", sa.Numeric(12, 2), nullable=False), + sa.Column("status", order_status, nullable=False), + sa.CheckConstraint("amount > 0", name=op.f("chk_orders_amount_positive")), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name="fk_orders_user_id_users"), + sa.PrimaryKeyConstraint("id", name="pk_orders"), + ) + + +def downgrade() -> None: + op.drop_table("orders") + op.drop_table("users") + order_status.drop(op.get_bind()) diff --git a/tests/integration/sample_app_with_drift/alembic/versions/enum_value/001_users_and_orders.py b/tests/integration/sample_app_with_drift/alembic/versions/enum_value/001_users_and_orders.py new file mode 100644 index 0000000..2937606 --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic/versions/enum_value/001_users_and_orders.py @@ -0,0 +1,44 @@ +"""Create users and orders; order_status lacks the 'shipped' value the model has. + +Revision ID: 0001 +Revises: +Create Date: 2026-09-07 00:00:00.000000 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision: str = "0001" +down_revision: str | None = None +branch_labels: str | None = None +depends_on: str | None = None + +order_status = sa.Enum("new", "paid", name="order_status") + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("email", sa.String(255), nullable=False), + sa.Column("is_active", sa.Boolean(), server_default=sa.text("true"), nullable=False), + sa.PrimaryKeyConstraint("id", name="pk_users"), + ) + op.create_table( + "orders", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("amount", sa.Numeric(12, 2), nullable=False), + sa.Column("status", order_status, nullable=False), + sa.CheckConstraint("amount > 0", name=op.f("chk_orders_amount_positive")), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name="fk_orders_user_id_users"), + sa.PrimaryKeyConstraint("id", name="pk_orders"), + ) + + +def downgrade() -> None: + op.drop_table("orders") + op.drop_table("users") + order_status.drop(op.get_bind()) diff --git a/tests/integration/sample_app_with_drift/alembic/versions/server_default/001_users_and_orders.py b/tests/integration/sample_app_with_drift/alembic/versions/server_default/001_users_and_orders.py new file mode 100644 index 0000000..613bf9a --- /dev/null +++ b/tests/integration/sample_app_with_drift/alembic/versions/server_default/001_users_and_orders.py @@ -0,0 +1,44 @@ +"""Create users and orders; is_active defaults to false where the model says true. + +Revision ID: 0001 +Revises: +Create Date: 2026-09-07 00:00:00.000000 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision: str = "0001" +down_revision: str | None = None +branch_labels: str | None = None +depends_on: str | None = None + +order_status = sa.Enum("new", "paid", "shipped", name="order_status") + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("email", sa.String(255), nullable=False), + sa.Column("is_active", sa.Boolean(), server_default=sa.text("false"), nullable=False), + sa.PrimaryKeyConstraint("id", name="pk_users"), + ) + op.create_table( + "orders", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("amount", sa.Numeric(12, 2), nullable=False), + sa.Column("status", order_status, nullable=False), + sa.CheckConstraint("amount > 0", name=op.f("chk_orders_amount_positive")), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name="fk_orders_user_id_users"), + sa.PrimaryKeyConstraint("id", name="pk_orders"), + ) + + +def downgrade() -> None: + op.drop_table("orders") + op.drop_table("users") + order_status.drop(op.get_bind()) diff --git a/tests/integration/sample_app_with_drift/models.py b/tests/integration/sample_app_with_drift/models.py new file mode 100644 index 0000000..09f13a8 --- /dev/null +++ b/tests/integration/sample_app_with_drift/models.py @@ -0,0 +1,40 @@ +"""SQLAlchemy ORM models with a server default, a CHECK constraint and an enum (issue #37).""" + +from __future__ import annotations + +from decimal import Decimal + +from sqlalchemy import Boolean, CheckConstraint, Enum, ForeignKey, MetaData, Numeric, String, text +from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column + + +class Base(DeclarativeBase): + metadata = MetaData( + naming_convention={ + "ix": "idx_%(column_0_label)s", + "uq": "uq_%(table_name)s_%(column_0_name)s", + "ck": "chk_%(table_name)s_%(constraint_name)s", + "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s", + "pk": "pk_%(table_name)s", + }, + ) + + +class UserDB(Base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + email: Mapped[str] = mapped_column(String(255), nullable=False) + is_active: Mapped[bool] = mapped_column(Boolean, nullable=False, server_default=text("true")) + + +class OrderDB(Base): + """The convention turns ``amount_positive`` into ``chk_orders_amount_positive``.""" + + __tablename__ = "orders" + __table_args__ = (CheckConstraint("amount > 0", name="amount_positive"),) + + id: Mapped[int] = mapped_column(primary_key=True) + user_id: Mapped[int] = mapped_column(ForeignKey("users.id"), nullable=False) + amount: Mapped[Decimal] = mapped_column(Numeric(12, 2), nullable=False) + status: Mapped[str] = mapped_column(Enum("new", "paid", "shipped", name="order_status"), nullable=False) diff --git a/tests/integration/test_drift_issue_37.py b/tests/integration/test_drift_issue_37.py new file mode 100644 index 0000000..6fb6316 --- /dev/null +++ b/tests/integration/test_drift_issue_37.py @@ -0,0 +1,110 @@ +"""Integration tests: the three kinds of drift ``compare_metadata()`` misses (issue #37). + +One history in four variants, selected through ``version_locations``: the models as +written, a wrong server default, a missing CHECK constraint, and an enum without one of +its values. The clean variant passes every inherited test; each drift variant fails +exactly the test written for it. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import ClassVar + +import pytest +from alembic.config import Config +from sqlalchemy import MetaData +from sqlalchemy.ext.asyncio import AsyncEngine + +from alembic_gauntlet import MigrationTestBase +from tests.integration.sample_app_with_drift.models import Base + +_SAMPLE_APP = Path(__file__).parent / "sample_app_with_drift" + + +class _DriftApp(MigrationTestBase): + variant: ClassVar[str] + + @pytest.fixture + def alembic_config(self) -> Config: + config = Config(str(_SAMPLE_APP / "alembic.ini")) + config.set_main_option("script_location", str(_SAMPLE_APP / "alembic")) + config.set_main_option("version_locations", str(_SAMPLE_APP / "alembic" / "versions" / self.variant)) + return config + + @pytest.fixture + def orm_metadata(self) -> MetaData: + return Base.metadata + + +@pytest.mark.integration +class TestDriftCleanHistory(_DriftApp): + """The history that matches the models passes all seven tests, server defaults compared too.""" + + variant = "clean" + migration_diff_compare_server_default = True + + +@pytest.mark.integration +class TestDriftServerDefaultNotCompared(_DriftApp): + """With the attribute at its default, a wrong server default is not reported.""" + + variant = "server_default" + + +@pytest.mark.integration +class TestDriftServerDefaultCompared(_DriftApp): + """With the attribute on, the up-to-date test reports the wrong server default.""" + + variant = "server_default" + migration_diff_compare_server_default = True + + async def test_migrations_up_to_date( + self, + alembic_config: Config, + migration_engine: AsyncEngine, + isolated_migration_schema: str, + orm_metadata: MetaData, + ) -> None: + with pytest.raises(AssertionError, match="is_active"): + await super().test_migrations_up_to_date( + alembic_config, migration_engine, isolated_migration_schema, orm_metadata + ) + + +@pytest.mark.integration +class TestDriftCheckConstraintMissing(_DriftApp): + """A CHECK constraint the models declare and the migration never created.""" + + variant = "check_missing" + + async def test_check_constraints_match( + self, + alembic_config: Config, + migration_engine: AsyncEngine, + isolated_migration_schema: str, + orm_metadata: MetaData, + ) -> None: + with pytest.raises(AssertionError, match="chk_orders_amount_positive"): + await super().test_check_constraints_match( + alembic_config, migration_engine, isolated_migration_schema, orm_metadata + ) + + +@pytest.mark.integration +class TestDriftEnumValueMissing(_DriftApp): + """An enum created without a value the models have.""" + + variant = "enum_value" + + async def test_enum_values_match( + self, + alembic_config: Config, + migration_engine: AsyncEngine, + isolated_migration_schema: str, + orm_metadata: MetaData, + ) -> None: + with pytest.raises(AssertionError, match="order_status"): + await super().test_enum_values_match( + alembic_config, migration_engine, isolated_migration_schema, orm_metadata + ) diff --git a/tests/integration/test_migration_base.py b/tests/integration/test_migration_base.py index de38459..26578a6 100644 --- a/tests/integration/test_migration_base.py +++ b/tests/integration/test_migration_base.py @@ -16,11 +16,13 @@ @pytest.mark.integration class TestSampleMigrations(MigrationTestBase): - """Run all five MigrationTestBase checks against the sample app migrations. + """Run all seven MigrationTestBase checks against the sample app migrations. Tests inherited: - test_stairway_upgrade_downgrade - test_migrations_up_to_date + - test_check_constraints_match + - test_enum_values_match - test_single_head_revision - test_downgrade_all_the_way - test_naming_conventions diff --git a/tests/unit/test_diff.py b/tests/unit/test_diff.py index 253a5d5..83c90c3 100644 --- a/tests/unit/test_diff.py +++ b/tests/unit/test_diff.py @@ -1,10 +1,23 @@ -"""Unit tests for schema diff filtering.""" +"""Unit tests for the schema diff helpers.""" -from unittest.mock import MagicMock +from collections.abc import Iterator +from unittest.mock import MagicMock, patch import pytest +from sqlalchemy import Boolean, CheckConstraint, Column, Enum, Integer, MetaData, Table +from sqlalchemy.dialects import postgresql +from sqlalchemy.dialects.postgresql.base import PGInspector + +from alembic_gauntlet.utils.diff import ( + DEFAULT_IGNORE_TABLES, + compare_check_constraints, + compare_enums, + is_ignored_diff_item, +) -from alembic_gauntlet.utils.diff import DEFAULT_IGNORE_TABLES, is_ignored_diff_item +_CONN = MagicMock(dialect=postgresql.dialect()) +_CK_CONVENTION = {"ck": "chk_%(table_name)s_%(constraint_name)s"} +_ORDER_STATUS = {"name": "order_status", "schema": "s", "visible": True, "labels": ["new", "paid", "shipped"]} def _make_table(name: str) -> MagicMock: @@ -94,3 +107,142 @@ def test__is_ignored_diff_item__table_name_none__returns_false() -> None: # Assert assert result is False + + +@pytest.fixture +def inspector() -> Iterator[MagicMock]: + """A PostgreSQL inspector with no constraints and no enums, patched into the module.""" + mock = MagicMock(spec=PGInspector) + mock.get_check_constraints.return_value = [] + mock.get_enums.return_value = [] + with patch("alembic_gauntlet.utils.diff.inspect", return_value=mock): + yield mock + + +def _orders(metadata: MetaData, *constraints: CheckConstraint, status: Enum | None = None) -> Table: + columns = [Column("id", Integer, primary_key=True), Column("amount", Integer)] + if status is not None: + columns.append(Column("status", status)) + return Table("orders", metadata, *columns, *constraints) + + +@pytest.mark.unit +def test__compare_check_constraints__missing_in_database__reported(inspector: MagicMock) -> None: + metadata = MetaData(naming_convention=_CK_CONVENTION) + _orders(metadata, CheckConstraint("amount > 0", name="amount_positive")) + + differences = compare_check_constraints(_CONN, metadata, "s") + + assert differences == [ + "Check constraint 'chk_orders_amount_positive' on table 'orders' is in the models but not in the database." + ] + inspector.get_check_constraints.assert_called_once_with("orders", schema="s") + + +@pytest.mark.unit +def test__compare_check_constraints__in_sync__empty(inspector: MagicMock) -> None: + metadata = MetaData(naming_convention=_CK_CONVENTION) + _orders(metadata, CheckConstraint("amount > 0", name="amount_positive")) + inspector.get_check_constraints.return_value = [{"name": "chk_orders_amount_positive", "sqltext": "amount > 0"}] + + assert compare_check_constraints(_CONN, metadata, "s") == [] + + +@pytest.mark.unit +def test__compare_check_constraints__unexpected_in_database__reported(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata) + inspector.get_check_constraints.return_value = [{"name": "chk_orders_stale", "sqltext": "amount < 100"}] + + assert compare_check_constraints(_CONN, metadata, "s") == [ + "Check constraint 'chk_orders_stale' on table 'orders' is in the database but not in the models." + ] + + +@pytest.mark.unit +def test__compare_check_constraints__unnamed_in_models__nothing_reported(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, CheckConstraint("amount > 0")) + inspector.get_check_constraints.return_value = [{"name": "orders_amount_check", "sqltext": "amount > 0"}] + + assert compare_check_constraints(_CONN, metadata, "s") == [] + + +@pytest.mark.unit +def test__compare_check_constraints__deferred_name__resolved_through_convention(inspector: MagicMock) -> None: + metadata = MetaData(naming_convention={"ck": "ck_%(table_name)s_%(column_0_name)s"}) + Table("users", metadata, Column("id", Integer, primary_key=True), Column("active", Boolean(create_constraint=True))) + + assert compare_check_constraints(_CONN, metadata, "s") == [ + "Check constraint 'ck_users_active' on table 'users' is in the models but not in the database." + ] + + +@pytest.mark.unit +def test__compare_check_constraints__ignored_table__skipped(inspector: MagicMock) -> None: + metadata = MetaData(naming_convention=_CK_CONVENTION) + _orders(metadata, CheckConstraint("amount > 0", name="amount_positive")) + + assert compare_check_constraints(_CONN, metadata, "s", frozenset({"orders"})) == [] + inspector.get_check_constraints.assert_not_called() + + +@pytest.mark.unit +def test__compare_enums__in_sync__empty(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", "shipped", name="order_status")) + inspector.get_enums.return_value = [_ORDER_STATUS] + + assert compare_enums(_CONN, metadata, "s") == [] + inspector.get_enums.assert_called_once_with(schema="*") + + +@pytest.mark.unit +@pytest.mark.parametrize( + "labels", + [["new", "paid"], ["new", "shipped", "paid"], ["new", "paid", "shipped", "refunded"]], + ids=["missing_value", "different_order", "extra_value"], +) +def test__compare_enums__values_differ__reported(inspector: MagicMock, labels: list[str]) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", "shipped", name="order_status")) + inspector.get_enums.return_value = [{**_ORDER_STATUS, "labels": labels}] + + assert compare_enums(_CONN, metadata, "s") == [ + f"Enum type 'order_status' has values {labels} in the database and ['new', 'paid', 'shipped'] in the models." + ] + + +@pytest.mark.unit +def test__compare_enums__type_missing__reported(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", name="order_status")) + + assert compare_enums(_CONN, metadata, "s") == [ + "Enum type 'order_status' in schema 's' is in the models but not in the database." + ] + + +@pytest.mark.unit +def test__compare_enums__explicit_schema__looked_up_there(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", "shipped", name="order_status", schema="types")) + inspector.get_enums.return_value = [{**_ORDER_STATUS, "schema": "types"}] + + assert compare_enums(_CONN, metadata, "s") == [] + + +@pytest.mark.unit +def test__compare_enums__non_native_enum__skipped(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", name="order_status", native_enum=False)) + + assert compare_enums(_CONN, metadata, "s") == [] + + +@pytest.mark.unit +def test__compare_enums__ignored_table__skipped(inspector: MagicMock) -> None: + metadata = MetaData() + _orders(metadata, status=Enum("new", "paid", name="order_status")) + + assert compare_enums(_CONN, metadata, "s", frozenset({"orders"})) == [] diff --git a/uv.lock b/uv.lock index 86bcb07..893b59a 100644 --- a/uv.lock +++ b/uv.lock @@ -8,7 +8,7 @@ resolution-markers = [ [[package]] name = "alembic" -version = "1.19.1" +version = "1.19.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "mako" }, @@ -16,9 +16,9 @@ dependencies = [ { name = "tomli", marker = "python_full_version < '3.11'" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/16/2b/e4153978368de59918115c9e01d3ebf58a558a7285efa7e960c383c4b59a/alembic-1.19.1.tar.gz", hash = "sha256:e0fca0518118c78acc493e31bcb5402f190057aaf6df8b5b95ce94c4789cf648", size = 2070816, upload-time = "2026-08-08T16:32:01.565Z" } +sdist = { url = "https://files.pythonhosted.org/packages/34/10/181eecdd552217d0342492bd6f3b8a96e973083379aace3d3402830ddc03/alembic-1.19.2.tar.gz", hash = "sha256:297950a8a91f6770eb82bfbce9bea55c728b90a5386c6e81430191a319d138b0", size = 2082643, upload-time = "2026-09-04T17:10:11.212Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/20/89/e62cc37b69ad357cc8ecd6e7367f5245f523d3cbb338a66197212bdf6749/alembic-1.19.1-py3-none-any.whl", hash = "sha256:b39018cb3d9413a19cbd54cf3c02ad33998641f0538eb77413a488a21c3e14be", size = 265946, upload-time = "2026-08-08T16:32:03.153Z" }, + { url = "https://files.pythonhosted.org/packages/9c/cb/9014784dcb0585977ae23b6f43331d0a33c51ac0d692506d12b4f5ee9f3b/alembic-1.19.2-py3-none-any.whl", hash = "sha256:32d553dcd577e6fe5c3c63e91468526d35e4dcecafe865d7db4e9b328fa93cb2", size = 267399, upload-time = "2026-09-04T17:10:12.796Z" }, ] [[package]]