From 788d177dc4c54d623fa89ef664e473435a039117 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 21 Aug 2026 21:52:41 +0300 Subject: [PATCH 1/3] Make chat_type a native enum and test migrations with pytest-alembic 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"]. --- .github/workflows/main.yml | 2 + CLAUDE.md | 12 +++ Justfile | 3 + alembic.ini | 4 + app/database/tables.py | 3 +- app/settings.py | 5 + migrations/env.py | 12 ++- .../2026-08-21_chat_type_native_enum.py | 50 ++++++++++ migrations/versions/2026-08-21_messages.py | 7 -- ...8-21.03-native-enum-and-migration-tests.md | 93 +++++++++++++++++++ pyproject.toml | 7 +- tests/migrations/__init__.py | 0 tests/migrations/conftest.py | 15 +++ tests/migrations/test_migrations.py | 12 +++ tests/test_settings.py | 6 ++ 15 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 migrations/versions/2026-08-21_chat_type_native_enum.py create mode 100644 planning/changes/2026-08-21.03-native-enum-and-migration-tests.md create mode 100644 tests/migrations/__init__.py create mode 100644 tests/migrations/conftest.py create mode 100644 tests/migrations/test_migrations.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d6e136f..192bade 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -49,6 +49,8 @@ jobs: uv sync --all-extras --all-groups --no-install-project uv run alembic upgrade head uv run pytest . + uv run alembic downgrade base + uv run pytest tests/migrations --override-ini=addopts= env: SERVICE_ENVIRONMENT: ci PYTHONDONTWRITEBYTECODE: 1 diff --git a/CLAUDE.md b/CLAUDE.md index 8b2f4e8..7bcb684 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,6 +46,18 @@ on the host). Inside the container, raw commands look like `uv run pytest - `just test` cycles the DB (downgrade to `base`, upgrade to `head`) before pytest and tears the stack down before and after. Pass pytest args through, e.g. `just test tests/use_cases/test_create_chat.py -k race -x`. +- `just test-migrations` runs the `pytest-alembic` suite (`tests/migrations/`): + single head, upgrade, per-revision up/down consistency, and + model-definitions-match-DDL — the last being `alembic check` as a test. That + directory is excluded from `just test` (`--ignore` in `addopts`, and from + `coverage`'s `omit`) because it cycles the schema out from under the + transaction-rollback fixture, so it needs the `--override-ini=addopts=` the + recipe passes. CI runs both. +- Enum columns are native Postgres enums, and `alembic-postgresql-enum` is + imported by `migrations/env.py` for its autogenerate hooks — that is what + renders `CREATE TYPE` / `ALTER TYPE ... ADD VALUE` / `op.sync_enum_values` + instead of silently missing them. Add or rename an enum value and + `just migration` writes the type change for you. - `just migration "message"` takes a **single positional argument** — not a `-m` flag — quoted so a multi-word message survives as one token (the recipe shell-quotes it with `quote()` before handing it to `alembic diff --git a/Justfile b/Justfile index b02f2b0..34edc2e 100644 --- a/Justfile +++ b/Justfile @@ -12,6 +12,9 @@ test *args: down && down run: docker compose run --service-ports api sh -c "sleep 1 && uv run alembic upgrade head && uv run python -m app.api" +test-migrations *args: down && down + docker compose run api sh -c "sleep 1 && uv run alembic downgrade base && uv run pytest tests/migrations --override-ini=addopts= {{ args }}" + migration message: && down # `message` is a single named parameter, shell-quoted via quote() so a multi-word message # survives intact - a variadic *args parameter only ever joins tokens with spaces when diff --git a/alembic.ini b/alembic.ini index 455a1fa..9c8d7f5 100644 --- a/alembic.ini +++ b/alembic.ini @@ -11,6 +11,10 @@ file_template = %%(year)d-%%(month).2d-%%(day).2d_%%(slug)s # defaults to the current working directory. prepend_sys_path = . +# separator for lists such as prepend_sys_path; without it Alembic falls back to +# legacy splitting and warns, which filterwarnings = ["error"] turns into a failure. +path_separator = os + # timezone to use when rendering the date # within the migration file as well as the filename. # string value is passed to dateutil.tz.gettz() diff --git a/app/database/tables.py b/app/database/tables.py index 22983ca..27776d5 100644 --- a/app/database/tables.py +++ b/app/database/tables.py @@ -43,8 +43,7 @@ class ChatsTable(BigIntAuditBase): chat_type: orm.Mapped[ChatType] = orm.mapped_column( sa.Enum( ChatType, - native_enum=False, - create_constraint=True, + name="chattype", values_callable=lambda enum_cls: [member.value for member in enum_cls], ) ) diff --git a/app/settings.py b/app/settings.py index 3eb2d28..a5a771d 100644 --- a/app/settings.py +++ b/app/settings.py @@ -60,6 +60,11 @@ def ensure_jwt_secret_is_configured(self) -> None: def db_dsn_parsed(self) -> URL: return make_url(self.db_dsn) + @property + def sync_db_dsn_parsed(self) -> URL: + # Alembic drives psycopg2, not asyncpg. + return self.db_dsn_parsed.set(drivername="postgresql") + @property def api_bootstrapper_config(self) -> LitestarConfig: return LitestarConfig( diff --git a/migrations/env.py b/migrations/env.py index c5fb8a6..86f1311 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -1,14 +1,16 @@ from logging.config import fileConfig +import alembic_postgresql_enum from alembic import context -from sqlalchemy import URL, create_engine +from sqlalchemy import create_engine from app.database.tables import METADATA from app.settings import settings -def get_dsn() -> URL: - return settings.db_dsn_parsed.set(drivername="postgresql") +# Imported for its side effect: registering the autogenerate hooks that render CREATE TYPE / +# ALTER TYPE ... ADD VALUE for native Postgres enums. +_ = alembic_postgresql_enum config = context.config @@ -21,7 +23,7 @@ def get_dsn() -> URL: def run_migrations_offline() -> None: context.configure( - url=get_dsn(), + url=settings.sync_db_dsn_parsed, target_metadata=target_metadata, literal_binds=True, dialect_opts={"paramstyle": "named"}, @@ -31,7 +33,7 @@ def run_migrations_offline() -> None: def run_migrations_online() -> None: - connectable = create_engine(get_dsn()) + connectable = create_engine(settings.sync_db_dsn_parsed) with connectable.connect() as connection: context.configure(connection=connection, target_metadata=target_metadata) with context.begin_transaction(): diff --git a/migrations/versions/2026-08-21_chat_type_native_enum.py b/migrations/versions/2026-08-21_chat_type_native_enum.py new file mode 100644 index 0000000..db76f0d --- /dev/null +++ b/migrations/versions/2026-08-21_chat_type_native_enum.py @@ -0,0 +1,50 @@ +"""chat type native enum. + +Revision ID: fa15d87677c3 +Revises: 1be68642e392 +Create Date: 2026-08-21 18:45:32.661982 + +""" + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "fa15d87677c3" +down_revision = "1be68642e392" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + sa.Enum("direct", "group", name="chattype").create(op.get_bind()) + op.alter_column( + "chats", + "chat_type", + existing_type=sa.VARCHAR(length=6), + type_=sa.Enum("direct", "group", name="chattype"), + existing_nullable=False, + postgresql_using="chat_type::chattype", + ) + op.drop_constraint(op.f("ck_chats_chattype"), "chats", type_="check") + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_check_constraint( + op.f("ck_chats_chattype"), + "chats", + "chat_type::text = ANY (ARRAY['direct'::character varying, 'group'::character varying]::text[])", + ) + op.alter_column( + "chats", + "chat_type", + existing_type=sa.Enum("direct", "group", name="chattype"), + type_=sa.VARCHAR(length=6), + existing_nullable=False, + ) + sa.Enum("direct", "group", name="chattype").drop(op.get_bind()) + # ### end Alembic commands ### diff --git a/migrations/versions/2026-08-21_messages.py b/migrations/versions/2026-08-21_messages.py index 1470192..8a5b45a 100644 --- a/migrations/versions/2026-08-21_messages.py +++ b/migrations/versions/2026-08-21_messages.py @@ -38,13 +38,6 @@ def upgrade() -> None: op.create_index("ix_messages_chat_id_id", "messages", ["chat_id", "id"], unique=False) op.create_index(op.f("ix_messages_user_id"), "messages", ["user_id"], unique=False) # ### end Alembic commands ### - # NOTE: autogenerate also proposed `op.drop_constraint(op.f('ck_chats_chattype'), 'chats', type_='check')` - # here. That's a known Alembic false positive for `sa.Enum(native_enum=False, create_constraint=True)` - # columns: Postgres reflects the CHECK constraint body back as `chat_type::text = ANY (ARRAY[...])`, - # which never textually matches what Alembic renders from the model, so every autogenerate run - # "detects" this same constraint as removed even though nothing about `chats.chat_type` changed. - # Dropping it here would be unrelated to this migration's purpose (adding `messages`) and would - # silently remove enum validation from `chats.chat_type`, so it is intentionally omitted. def downgrade() -> None: diff --git a/planning/changes/2026-08-21.03-native-enum-and-migration-tests.md b/planning/changes/2026-08-21.03-native-enum-and-migration-tests.md new file mode 100644 index 0000000..f56bdd2 --- /dev/null +++ b/planning/changes/2026-08-21.03-native-enum-and-migration-tests.md @@ -0,0 +1,93 @@ +--- +summary: Converted chats.chat_type to a native Postgres enum with alembic-postgresql-enum and added a pytest-alembic suite, turning a permanently-dirty autogenerate into a green CI gate. +--- + +# Design: Native chat_type enum and a pytest-alembic migration suite + +## Summary + +`chats.chat_type` becomes a native Postgres enum instead of `VARCHAR` plus a +CHECK constraint, `migrations/env.py` imports `alembic-postgresql-enum`, and +`tests/migrations/` runs the four `pytest-alembic` built-ins. `alembic check` is +clean for the first time, and migration health is now enforced by CI rather than +by a comment asking readers to ignore a diff. + +## Motivation + +`sa.Enum(ChatType, native_enum=False, create_constraint=True)` produced a +permanent autogenerate false positive: Postgres reflects the CHECK body back as +`chat_type::text = ANY (ARRAY[...])`, which never textually matches what Alembic +renders from the model, so every run proposed +`op.drop_constraint('ck_chats_chattype')`. The cost was not cosmetic — it meant +`alembic check` could never be a drift gate, and every `just migration` produced +a spurious operation a human had to remember to delete. A note in +`migrations/versions/2026-08-21_messages.py` documented that trap rather than +removing it. + +The private `rchat` service already solved this: native `postgresql.ENUM` +columns plus `alembic-postgresql-enum`, with `pytest-alembic` covering migration +health in a separate CI job. This repo exists to show that stack working, so it +should show that part too. + +## Design + +**Native enum.** `sa.Enum(ChatType, name="chattype", values_callable=...)` — the +`values_callable` stays, so the type's labels remain the lowercase member values. +`alembic-postgresql-enum`, imported for its side effect in `migrations/env.py`, +autogenerated the whole conversion including the `USING` clause and a working +downgrade: + +```python +sa.Enum("direct", "group", name="chattype").create(op.get_bind()) +op.alter_column("chats", "chat_type", ..., postgresql_using="chat_type::chattype") +op.drop_constraint(op.f("ck_chats_chattype"), "chats", type_="check") +``` + +Producing that by hand is exactly the work the library exists to remove, and it +is what makes a future value addition a one-command change. + +**Migration tests.** `tests/migrations/test_migrations.py` imports +`test_single_head_revision`, `test_upgrade`, `test_up_down_consistency` and +`test_model_definitions_match_ddl` from `pytest_alembic.tests`. The last is +`alembic check` as a test; the other three are coverage `just test` never had — +it only ran `downgrade base && upgrade head`, which proves neither per-revision +reversibility nor the absence of branched heads. A local `alembic_engine` +fixture replaces pytest-alembic's default in-memory SQLite engine with the real +Postgres DSN. + +The suite is excluded from the default run — `--ignore=tests/migrations` in +`addopts`, `tests/migrations/*` in coverage's `omit` — because it cycles the +schema out from under `db_session`'s transaction-rollback fixture. `just +test-migrations` runs it with `--override-ini=addopts=`, and CI runs both. + +**Incidental.** `Settings.sync_db_dsn_parsed` now owns the +`postgresql+asyncpg` → `postgresql` rewrite, which `migrations/env.py` and the +`alembic_engine` fixture both need. `alembic.ini` gains `path_separator = os`: +without it Alembic emits a `DeprecationWarning` that `filterwarnings = ["error"]` +turns into a test failure. + +## Non-goals + +- Converting anything else. `chat_type` is the only enum column in the schema. +- Suppressing the diff with an `include_object` filter in `env.py`. It would + have been two lines, but it hides a class of real diffs and keys on a + constraint name that the next enum column would not share. + +## Testing + +- `just test` — 109 passed, 100% coverage. +- `just test-migrations` — 4 passed. +- `just lint` — clean. +- `alembic upgrade head && alembic check` — clean, where it previously reported + `remove_constraint ck_chats_chattype`. `alembic downgrade -1 && alembic + upgrade head` round-trips. + +## Risk + +- **Adding an enum value is now a migration.** That is the point — it was + previously an unenforced CHECK constraint edit — but it does mean a value + added to `ChatType` without running `just migration` fails + `test_model_definitions_match_ddl` rather than passing silently. +- **The conversion is not free on a large table.** `ALTER COLUMN ... TYPE` + rewrites `chats`. It is cheap now, while the table is effectively empty, and + gets more expensive the longer it waits. diff --git a/pyproject.toml b/pyproject.toml index 1d6308d..e416bff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,6 +17,7 @@ dependencies = [ "db-retry", # database "alembic", + "alembic-postgresql-enum", "psycopg2", "sqlalchemy[asyncio]", "asyncpg", @@ -34,6 +35,7 @@ dev = [ "pytest-asyncio", "asgi_lifespan", "modern-di-pytest>=3,<4", + "pytest-alembic", ] lint = ["ruff", "ty", "eof-fixer"] @@ -70,7 +72,7 @@ isort.no-lines-before = ["standard-library", "local-folder"] "migrations/*.py" = ["ERA001"] [tool.pytest.ini_options] -addopts = "--cov=. --cov-report term-missing --cov-fail-under=100" +addopts = "--cov=. --cov-report term-missing --cov-fail-under=100 --ignore=tests/migrations" asyncio_mode = "auto" asyncio_default_fixture_loop_scope = "function" filterwarnings = ["error"] @@ -83,6 +85,9 @@ concurrency = ["thread", "greenlet"] disable_warnings = ["couldnt-parse"] omit = [ "migrations/*", + # Excluded from the default pytest run (see addopts) because they cycle the schema; they + # run under `just test-migrations` instead, where coverage is off. + "tests/migrations/*", "app/api/__main__.py", # No __init__.py under planning/ (see the comment atop planning/index.py), so coverage's # package walk never reaches this file on its own - omit it explicitly rather than relying diff --git a/tests/migrations/__init__.py b/tests/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/migrations/conftest.py b/tests/migrations/conftest.py new file mode 100644 index 0000000..4f49702 --- /dev/null +++ b/tests/migrations/conftest.py @@ -0,0 +1,15 @@ +import typing + +import pytest +from sqlalchemy import Engine, create_engine + +from app.settings import settings + + +@pytest.fixture +def alembic_engine() -> typing.Iterator[Engine]: + # Overrides pytest-alembic's default in-memory SQLite engine: these tests are only + # meaningful against the Postgres the migrations actually target. + engine: typing.Final = create_engine(settings.sync_db_dsn_parsed) + yield engine + engine.dispose() diff --git a/tests/migrations/test_migrations.py b/tests/migrations/test_migrations.py new file mode 100644 index 0000000..1b937f7 --- /dev/null +++ b/tests/migrations/test_migrations.py @@ -0,0 +1,12 @@ +from pytest_alembic.tests import ( + test_model_definitions_match_ddl, + test_single_head_revision, + test_up_down_consistency, + test_upgrade, +) + + +_ = test_single_head_revision +_ = test_upgrade +_ = test_model_definitions_match_ddl +_ = test_up_down_consistency diff --git a/tests/test_settings.py b/tests/test_settings.py index 87e3209..b424dbd 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -9,6 +9,12 @@ def test_db_dsn_parsed_exposes_driver() -> None: assert settings.db_dsn_parsed.database == "dbname" +def test_sync_db_dsn_parsed_drops_the_async_driver() -> None: + settings = Settings(db_dsn="postgresql+asyncpg://user:pw@host/dbname") + assert settings.sync_db_dsn_parsed.drivername == "postgresql" + assert settings.sync_db_dsn_parsed.database == "dbname" + + def test_api_bootstrapper_config_carries_service_identity() -> None: settings = Settings(service_name="svc", service_version="9.9.9") config = settings.api_bootstrapper_config From 5731c96ff896f251586abd0516f357e569dcabd5 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 21 Aug 2026 21:56:00 +0300 Subject: [PATCH 2/3] Raise ruff's pylint.max-args to 10 instead of suppressing per site 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. --- app/api/endpoints/messages.py | 2 +- pyproject.toml | 4 ++++ tests/use_cases/test_unread_counts.py | 14 +++++++------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/api/endpoints/messages.py b/app/api/endpoints/messages.py index 79021fe..9cc8a92 100644 --- a/app/api/endpoints/messages.py +++ b/app/api/endpoints/messages.py @@ -36,7 +36,7 @@ async def send_message( @litestar.get("/chats/{chat_id:int}/messages/") -async def list_messages( # noqa: PLR0913 - each is a distinct Litestar-bound path/query/DI param +async def list_messages( chat_id: FromPath[int], request: AuthedRequest, fetch_messages_use_case: NamedDependency[FetchMessagesUseCase], diff --git a/pyproject.toml b/pyproject.toml index e416bff..7f5268d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,10 @@ ignore = [ ] isort.lines-after-imports = 2 isort.no-lines-before = ["standard-library", "local-folder"] +# Handlers and tests take their dependencies as parameters - Litestar binds path/query/DI +# params by name, pytest binds fixtures by name - so the default limit of 5 flags ordinary +# code. max-positional-args defaults to this, which covers PLR0917 as well. +pylint.max-args = 10 [tool.ruff.lint.extend-per-file-ignores] "tests/*.py" = ["S101", "PLR2004"] diff --git a/tests/use_cases/test_unread_counts.py b/tests/use_cases/test_unread_counts.py index 7051a30..5db319b 100644 --- a/tests/use_cases/test_unread_counts.py +++ b/tests/use_cases/test_unread_counts.py @@ -53,7 +53,7 @@ async def test_system_messages_count_as_unread( assert chats[0].unread_count == 1 -async def test_marking_read_clears_the_count( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency +async def test_marking_read_clears_the_count( fetch_chats_use_case: FetchChatsUseCase, mark_read_use_case: MarkReadUseCase, direct_chat: tables.ChatsTable, @@ -69,7 +69,7 @@ async def test_marking_read_clears_the_count( # noqa: PLR0913, PLR0917 - each i assert chats[0].unread_count == 0 -async def test_deleted_messages_are_not_unread( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency +async def test_deleted_messages_are_not_unread( fetch_chats_use_case: FetchChatsUseCase, delete_message_use_case: DeleteMessageUseCase, direct_chat: tables.ChatsTable, @@ -92,7 +92,7 @@ async def test_chat_with_no_messages_has_no_last_message( assert chats[0].unread_count == 0 -async def test_listing_orders_most_recently_active_chat_first( # noqa: PLR0913, PLR0917 - fixture-injected +async def test_listing_orders_most_recently_active_chat_first( fetch_chats_use_case: FetchChatsUseCase, create_chat_use_case: CreateChatUseCase, direct_chat: tables.ChatsTable, @@ -108,7 +108,7 @@ async def test_listing_orders_most_recently_active_chat_first( # noqa: PLR0913, assert [chat.id for chat in chats] == [direct_chat.id, other_chat.id] -async def test_unread_counts_differ_per_chat( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency +async def test_unread_counts_differ_per_chat( fetch_chats_use_case: FetchChatsUseCase, create_chat_use_case: CreateChatUseCase, direct_chat: tables.ChatsTable, @@ -142,7 +142,7 @@ async def test_non_member_cannot_mark_read( ) -async def test_marking_read_with_a_message_from_another_chat_is_rejected( # noqa: PLR0913, PLR0917 - fixture-injected +async def test_marking_read_with_a_message_from_another_chat_is_rejected( mark_read_use_case: MarkReadUseCase, create_chat_use_case: CreateChatUseCase, direct_chat: tables.ChatsTable, @@ -169,7 +169,7 @@ async def test_marking_read_rejects_an_unknown_message_id( ) -async def test_marking_read_is_monotonic( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency +async def test_marking_read_is_monotonic( fetch_chats_use_case: FetchChatsUseCase, mark_read_use_case: MarkReadUseCase, direct_chat: tables.ChatsTable, @@ -193,7 +193,7 @@ async def test_marking_read_is_monotonic( # noqa: PLR0913, PLR0917 - each is a assert chats[0].unread_count == 0 -async def test_deleting_the_newest_message_updates_preview_and_ordering( # noqa: PLR0913, PLR0917 - fixture-injected +async def test_deleting_the_newest_message_updates_preview_and_ordering( fetch_chats_use_case: FetchChatsUseCase, delete_message_use_case: DeleteMessageUseCase, create_chat_use_case: CreateChatUseCase, From d0a76e08b7eab0b3048711104a3cfad3808afaea Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 21 Aug 2026 22:05:29 +0300 Subject: [PATCH 3/3] Run the app and migration suites as two CI matrix legs 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. --- .github/workflows/main.yml | 18 ++++++++++++++---- Justfile | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 192bade..2c3daaf 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -27,6 +27,19 @@ jobs: pytest: runs-on: ubuntu-latest + name: pytest (${{ matrix.name }}) + strategy: + # Without this a failing app suite cancels the migration leg, collapsing two + # independent signals back into one. + fail-fast: false + matrix: + include: + - name: app + run: uv run alembic upgrade head && uv run pytest . + # No `alembic upgrade` first: pytest-alembic drives the schema itself, and running + # it against an empty database is what makes test_upgrade mean anything. + - name: migrations + run: uv run pytest tests/migrations --override-ini=addopts= services: postgres: image: postgres:17 @@ -47,10 +60,7 @@ jobs: - run: uv python pin 3.14 - run: | uv sync --all-extras --all-groups --no-install-project - uv run alembic upgrade head - uv run pytest . - uv run alembic downgrade base - uv run pytest tests/migrations --override-ini=addopts= + ${{ matrix.run }} env: SERVICE_ENVIRONMENT: ci PYTHONDONTWRITEBYTECODE: 1 diff --git a/Justfile b/Justfile index 34edc2e..529bf1c 100644 --- a/Justfile +++ b/Justfile @@ -13,7 +13,7 @@ run: docker compose run --service-ports api sh -c "sleep 1 && uv run alembic upgrade head && uv run python -m app.api" test-migrations *args: down && down - docker compose run api sh -c "sleep 1 && uv run alembic downgrade base && uv run pytest tests/migrations --override-ini=addopts= {{ args }}" + docker compose run api sh -c "sleep 1 && uv run pytest tests/migrations --override-ini=addopts= {{ args }}" migration message: && down # `message` is a single named parameter, shell-quoted via quote() so a multi-word message