perf(api): Stop replaying migrations, and give tests their own database

.github/workflows/api-pull-request.yml

@@ -69,7 +69,9 @@ jobs:
           linters: mypy
 
       - name: Run Tests
-        run: make test
+        env:
+          format: xml
+        run: make test-coverage
 
       - name: Upload Coverage
         uses: codecov/codecov-action@v5

api/.env-ci

@@ -5,5 +5,4 @@ CLICKHOUSE_PORT=9000
 CLICKHOUSE_USER=flagsmith
 CLICKHOUSE_PASSWORD=password
 CLICKHOUSE_DATABASE=default
-PYTEST_ADDOPTS=--cov . --cov-report xml -n auto --ci
 COVERAGE_CORE=sysmon

api/.env-local

@@ -5,5 +5,9 @@ CLICKHOUSE_PORT=9000
 CLICKHOUSE_USER=flagsmith
 CLICKHOUSE_PASSWORD=password
 CLICKHOUSE_DATABASE=default
+# The suite gets its own RAM-backed server, so it never competes with the
+# database `make serve` keeps your local data in. See `test-db` in
+# docker/api/docker-compose.local.yml.
+TEST_DATABASE_URL=postgresql://postgres:password@localhost:5434/flagsmith
+TEST_ANALYTICS_DATABASE_URL=postgresql://postgres:password@localhost:5434/analytics
 DJANGO_SETTINGS_MODULE=app.settings.local
-PYTEST_ADDOPTS=--cov . --cov-report html -n auto

api/Makefile

@@ -45,6 +45,14 @@ docker-up:
 	docker compose up --force-recreate --remove-orphans -d
 	docker compose ps
 
+# `docker-up` recreates every container, which costs several seconds and drops
+# open connections. Targets that only need the services to be reachable use
+# this instead: it is near-free once the stack is up, and Compose's `--wait`
+# blocks on the healthchecks so nothing has to poll for a database afterwards.
+.PHONY: docker-ready
+docker-ready:
+	@bash scripts/ensure-services.sh
+
 .PHONY: docker-down
 docker-down:
 	docker compose stop
@@ -66,9 +74,21 @@ wait-for-db:
 	uv run python manage.py waitfordb
 	uv run python manage.py waitfordb --database analytics
 
+# Every xdist worker pays for a Python start, a Django setup and a test
+# database clone, so fanning out only earns its keep on a large run. A bare
+# `make test` runs the whole suite and gets the fan-out; `make test opts=...`
+# is a focused run and stays in-process. Put `-n auto` in `opts` to override.
+PYTEST_PARALLELISM = $(if $(strip $(opts)),,-n auto)
+
 .PHONY: test
-test: docker-up wait-for-db
-	uv run pytest $(opts)
+test: docker-ready
+	uv run pytest $(PYTEST_PARALLELISM) $(opts)
+
+# Coverage roughly doubles the suite's runtime, so it is opt-in rather than
+# part of every `make test`. CI and anyone checking a diff's coverage use this.
+.PHONY: test-coverage
+test-coverage: docker-ready
+	uv run pytest --cov . --cov-report $(or $(format),html) $(PYTEST_PARALLELISM) $(opts)
 
 .PHONY: django-make-migrations
 django-make-migrations: docker-up wait-for-db

api/README.md

@@ -13,10 +13,18 @@ To run linters, run `make lint`.
 
 To run tests, run `make test`.
 
-To run a subset of tests or an individual test, run `make test opts='<pytest args>'`. If the number of test is too low for xdist, consider adding `-n0` to pytest args.
+To run a subset of tests or an individual test, run `make test opts='<pytest args>'`. A bare `make test` fans out across your cores with xdist; passing `opts` is taken to mean a focused run and stays in-process, since each worker costs a Python start, a Django setup and a database clone. Put `-n auto` in `opts` to fan out anyway.
+
+To measure coverage, run `make test-coverage`. It is not part of `make test` because it roughly doubles the runtime.
 
 To prepare a dev database, run `make docker-up django-migrate`.
 
+#### Test databases
+
+Tests run against `test-db`, a separate PostgreSQL service that keeps its data in RAM. It is deliberately not the `db` service `make serve` uses: nothing the suite writes is worth keeping, and holding it in memory keeps the suite off the host's disk. Losing it costs nothing, so `docker compose restart test-db` is always a safe reset.
+
+You should never need `--reuse-db`. Rather than running the ~600 migrations for every worker on every run, the suite migrates once into a template database and clones it, which takes about a tenth of a second. The template is keyed on a digest of the migration files, so editing a migration -- or switching to a branch that has different ones -- silently builds a new template instead of handing you a stale schema. Migration tests use the same mechanism to reach the state they test, rather than replaying history for each one. See `tests/migration_snapshots.py`.
+
 To bring up a dev server, run `make serve`, or `make serve-with-task-processor` to run the Task processor alongside the server.
 
 ### Code guidelines: testing

api/app/settings/test.py

@@ -1,15 +1,35 @@
+import dj_database_url
+
 from app.settings.common import *  # noqa
 from app.settings.common import (
     DATABASES,
     INSTALLED_APPS,
     LDAP_INSTALLED,
     REST_FRAMEWORK,
+    env,
 )
 
+# Point the suite at its own PostgreSQL server when one is configured, so it
+# never shares a disk -- or a page cache -- with the database `make serve`
+# keeps your local data in. See `test-db` in the dev Compose file.
+for alias, variable in (
+    ("default", "TEST_DATABASE_URL"),
+    ("analytics", "TEST_ANALYTICS_DATABASE_URL"),
+):
+    if alias in DATABASES and (url := env.str(variable, default="")):
+        DATABASES[alias] = {**DATABASES[alias], **dj_database_url.parse(url)}
+
+# Django's default hasher deliberately costs ~350ms per call. Fixtures like
+# `admin_user` and `admin_master_api_key` hash a password on nearly every test,
+# which made hashing one of the suite's largest line items. Nothing here
+# depends on the algorithm, so use the cheapest one.
+PASSWORD_HASHERS = ["django.contrib.auth.hashers.MD5PasswordHasher"]
+
 # TODO: remove once permissions are an enum --
 # https://github.com/Flagsmith/flagsmith/issues/7850
 DATABASES["default"]["ENGINE"] = "core.db_backends.postgresql"
 
+
 if LDAP_INSTALLED:
     INSTALLED_APPS = INSTALLED_APPS + ["flagsmith_ldap"]
     LDAP_DEFAULT_FLAGSMITH_ORGANISATION_ID = None

api/pyproject.toml

@@ -475,7 +475,8 @@ omit = [
 pythonpath = ['.']
 addopts = [
   '--ds=app.settings.test',
-  '-vvvv',
+  # `-v` and above print a line per test, which on a suite this size costs
+  # several seconds of pure formatting and buries failures in the noise.
   '-p',
   'no:warnings',
   '--dist=worksteal',

api/scripts/ensure-services.sh

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+# Make sure the local Compose services are reachable, cheaply.
+#
+# `docker compose up --wait` is the correct thing to run, but it needs a couple
+# of seconds just to parse the project and talk to the daemon -- a large slice
+# of a focused `make test`. Opening a socket to each service costs a
+# millisecond and answers the same question in the overwhelmingly common case
+# where the stack is already up, so try that first and fall back to Compose.
+set -euo pipefail
+
+# Pull "host" and "port" out of a URL like postgresql://user:pw@host:5432/name.
+endpoint_of() {
+  sed -E 's#^[^:]+://([^@]*@)?([^:/]+):([0-9]+).*$#\2 \3#' <<<"$1"
+}
+
+reachable() {
+  # Bash's /dev/tcp needs no subprocess, unlike nc(1), which is not everywhere.
+  (exec 3<>"/dev/tcp/$1/$2") 2>/dev/null
+}
+
+services=(
+  "$(endpoint_of "${TEST_DATABASE_URL:-${DATABASE_URL}}")"
+  "$(endpoint_of "${TEST_ANALYTICS_DATABASE_URL:-${ANALYTICS_DATABASE_URL}}")"
+  "${CLICKHOUSE_HOST:-localhost} ${CLICKHOUSE_PORT:-9000}"
+)
+
+for service in "${services[@]}"; do
+  # shellcheck disable=SC2086 # deliberate split into host and port
+  reachable ${service} || exec docker compose up --remove-orphans --wait -d
+done

api/tests/conftest.py

@@ -43,9 +43,7 @@
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.core.cache import caches
-from django.db import connections
-from django.db.backends.base.creation import TEST_DATABASE_PREFIX
-from django.test.utils import setup_databases
+from django.db import DEFAULT_DB_ALIAS, connections
 from django_test_migrations.migrator import Migrator
 from flag_engine.segments.constants import EQUAL
 from moto import mock_dynamodb  # type: ignore[import-untyped]
@@ -54,13 +52,12 @@
 from pyfakefs.fake_filesystem import FakeFilesystem
 from pytest import FixtureRequest
 from pytest_django.fixtures import SettingsWrapper
-from pytest_django.plugin import blocking_manager_key
+from pytest_django.plugin import DjangoDbBlocker
 from pytest_mock import MockerFixture
 from rest_framework.test import APIClient
 from task_processor.task_run_method import TaskRunMethod
 from urllib3 import BaseHTTPResponse
 from urllib3.connectionpool import HTTPConnectionPool
-from xdist import get_xdist_worker_id  # type: ignore[import-untyped]
 
 from api_keys.models import MasterAPIKey
 from api_keys.user import APIKeyUser
@@ -116,6 +113,12 @@
 
 # TODO: Delete alias as per https://github.com/Flagsmith/flagsmith/issues/7818
 from segments.types import SegmentRule as SegmentRuleType
+from tests.migration_snapshots import (
+    MigrationSnapshots,
+    SnapshotMigrator,
+    build_snapshots,
+    template_backed_test_databases,
+)
 from tests.types import (
     AdminClientAuthType,
     EnableFeaturesFixture,
@@ -141,28 +144,15 @@
 # ---------------------------------------------------------------------------
 
 
-def pytest_addoption(parser: pytest.Parser) -> None:
-    parser.addoption(
-        "--ci",
-        action="store_true",
-        default=False,
-        help="Enable CI mode",
-    )
-
+@pytest.fixture(scope="session", autouse=True)
+def _template_backed_test_databases() -> typing.Generator[None, None, None]:
+    """Clone test databases from a migrated template rather than migrating.
 
-@pytest.hookimpl(trylast=True)
-def pytest_configure(config: pytest.Config) -> None:
-    if (
-        config.option.ci
-        and config.option.dist != "no"
-        and not hasattr(config, "workerinput")
-    ):
-        with config.stash[blocking_manager_key].unblock():
-            setup_databases(
-                verbosity=config.option.verbose,
-                interactive=False,
-                parallel=config.option.numprocesses,
-            )
+    Autouse and session scoped so that it wraps `django_db_setup`, whichever
+    worker gets there first. See `tests.migration_snapshots`.
+    """
+    with template_backed_test_databases():
+        yield
 
 
 # ---------------------------------------------------------------------------
@@ -213,27 +203,6 @@ def fs(fs: FakeFilesystem) -> FakeFilesystem:
     return fs
 
 
-@pytest.fixture(scope="session")
-def django_db_setup(request: pytest.FixtureRequest) -> None:
-    if (
-        request.config.option.ci
-        # xdist worker id is either `gw[0-9]+` or `master`
-        and (xdist_worker_id_suffix := get_xdist_worker_id(request)[2:]).isnumeric()
-    ):
-        # Django's test database clone indices start at 1,
-        # Pytest's worker indices are 0-based
-        test_db_suffix = str(int(xdist_worker_id_suffix) + 1)
-    else:
-        # Tests are run on main node, which assumes -n0
-        return request.getfixturevalue("django_db_setup")  # type: ignore[no-any-return] # pragma: no cover
-
-    from django.conf import settings
-
-    for db_settings in settings.DATABASES.values():
-        test_db_name = f"{TEST_DATABASE_PREFIX}{db_settings['NAME']}_{test_db_suffix}"
-        db_settings["NAME"] = test_db_name
-
-
 @pytest.fixture()
 def mock_influxdb_client(mocker: MockerFixture) -> MagicMock:
     client: MagicMock = mocker.patch.object(InfluxDBWrapper, "get_client").return_value
@@ -1615,6 +1584,53 @@ def dynamo_environment_wrapper(
     return wrapper
 
 
+@pytest.fixture(scope="session")
+def migration_snapshots(
+    django_db_setup: None,
+    django_db_blocker: DjangoDbBlocker,
+) -> typing.Generator[dict[str, MigrationSnapshots], None, None]:
+    """Cache each database's migration states as template databases.
+
+    Built lazily, on the first migration test a worker runs, from a database
+    that `django_db_setup` has just migrated -- so the `latest` snapshot the
+    `migrator` fixture restores on teardown carries the rows that data
+    migrations create, which the old `migrate`-forward teardown could not.
+    """
+    snapshots: dict[str, MigrationSnapshots] = {}
+    with django_db_blocker.unblock():
+        yield snapshots
+        for snapshot in snapshots.values():
+            snapshot.close()
+
+
+@pytest.fixture()
+def migrator_factory(
+    request: pytest.FixtureRequest,
+    transactional_db: None,
+    django_db_use_migrations: bool,
+    migration_snapshots: dict[str, MigrationSnapshots],
+) -> MigratorFactory:
+    """Override `django_test_migrations`' fixture of the same name.
+
+    Identical in behaviour -- including keeping the name, so the plugin still
+    recognises these tests and marks them `migration_test` -- except that
+    migration states are cloned from template databases rather than replayed
+    migration by migration.
+    """
+    if not django_db_use_migrations:  # pragma: no cover
+        pytest.skip("--no-migrations was specified")
+
+    def factory(database_name: str | None = None) -> Migrator:
+        alias = database_name or DEFAULT_DB_ALIAS
+        if alias not in migration_snapshots:
+            migration_snapshots[alias] = build_snapshots(alias)
+        migrator = SnapshotMigrator(alias, migration_snapshots[alias])
+        request.addfinalizer(migrator.reset)
+        return migrator
+
+    return typing.cast(MigratorFactory, factory)
+
+
 @pytest.fixture()
 def migrator(migrator_factory: MigratorFactory) -> Migrator:
     if settings.SKIP_MIGRATION_TESTS:  # pragma: no cover