Make chat_type a native enum and test migrations with pytest-alembic - #4
Merged
Conversation
sa.Enum(native_enum=False, create_constraint=True) produced a permanent autogenerate false positive: Postgres reflects the CHECK body as chat_type::text = ANY (ARRAY[...]), which never matches what Alembic renders from the model, so every run proposed dropping ck_chats_chattype. That kept alembic check from ever being usable as a drift gate. chat_type is now a native Postgres enum and migrations/env.py imports alembic-postgresql-enum for its autogenerate hooks - which wrote the whole conversion, USING clause and working downgrade included. tests/migrations/ runs the four pytest-alembic built-ins, including test_model_definitions_match_ddl (alembic check as a test) and test_up_down_consistency, which just test never covered. The suite cycles the schema, so it is excluded from the default run and from coverage, and gets its own just recipe and CI step. Settings.sync_db_dsn_parsed now owns the asyncpg -> psycopg2 DSN rewrite that env.py and the alembic_engine fixture both need. alembic.ini gains path_separator = os so Alembic's deprecation warning stops failing tests under filterwarnings = ["error"].
Litestar binds path/query/DI params by name and pytest binds fixtures by name, so handlers and tests legitimately take more than five parameters. Eight # noqa: PLR0913, PLR0917 annotations existed only to say that. max-positional-args defaults to max-args, so one setting covers both rules.
The migration tests were appended to the pytest job, so a failing app suite meant they never ran at all and a red job did not say which half broke. They also ran against a database the app suite had left at head, rewound by an `alembic downgrade base` step - implicit setup doing work that test_up_down_consistency already covers. A matrix over the two commands gives each an independent signal and a fresh database, while keeping one services block and one setup stanza. fail-fast is off so one leg's failure does not cancel the other. The same downgrade was redundant in `just test-migrations`: the compose db service has no volume, so the recipe's leading `down` already yields an empty database.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #3, which GitHub auto-closed when its stacked base branch was deleted
on merge of #2. Same branch, rebased onto
main.Rationale in
planning/changes/2026-08-21.03-native-enum-and-migration-tests.md.Native enum
sa.Enum(native_enum=False, create_constraint=True)produced a permanentautogenerate false positive — Postgres reflects the CHECK body as
chat_type::text = ANY (ARRAY[...]), which never matches what Alembic rendersfrom the model, so every run proposed dropping
ck_chats_chattype.alembic checkcould therefore never be a drift gate.chat_typeis now a native Postgres enum andmigrations/env.pyimportsalembic-postgresql-enumfor its autogenerate hooks. The library wrote theentire conversion migration,
USINGclause and working downgrade included.Migration tests
tests/migrations/runs fourpytest-alembicbuilt-ins:test_single_head_revision,test_upgrade,test_up_down_consistencyandtest_model_definitions_match_ddl(the last isalembic checkas a test).just testonly ever randowngrade base && upgrade head, which proves neitherper-revision reversibility nor the absence of branched heads.
The suite cycles the schema out from under
db_session's rollback fixture, soit is excluded from the default run (
--ignoreinaddopts,omitincoverage) and gets
just test-migrationsplus its own CI step.Also
Settings.sync_db_dsn_parsedowns theasyncpg→psycopg2DSN rewrite thatenv.pyand thealembic_enginefixture both need.alembic.inigainspath_separator = os; without it Alembic'sDeprecationWarningfails tests underfilterwarnings = ["error"].pylint.max-args = 10in the ruff config replaces eight# noqa: PLR0913, PLR0917annotations. Litestar binds path/query/DI params byname and pytest binds fixtures by name, so handlers and tests legitimately
exceed the default of five.
max-positional-argsdefaults tomax-args, soone setting covers both rules.
Verification
just test— 109 passed, 100% coverage.just test-migrations— 4 passed.just lint,just check-planning— clean.alembic upgrade head && alembic check— clean (previously reportedremove_constraint ck_chats_chattype);downgrade -1 && upgrade headround-trips.