Fixes of TM tests

Author: jeffmaki

alembic_osm/versions/5303f61d7b3a_add_osm_augmented_diff_function.py

@@ -14,14 +14,22 @@
 
 # revision identifiers, used by Alembic.
 revision: str = "5303f61d7b3a"
-down_revision: Union[str, None] = "9221408912dd"
+down_revision: Union[str, None] = "a1b2c3d4e5f6"
 branch_labels: Union[str, Sequence[str], None] = None
 depends_on: Union[str, Sequence[str], None] = None
 
 _SQL_DIR = pathlib.Path(__file__).parent.parent / "sql"
 
 
 def upgrade() -> None:
+    # This SQL function references OSM core tables (nodes, ways, way_nodes)
+    # owned by the Rails website. Disable body validation for this transaction
+    # so the function can be created even when those tables aren't present yet
+    # (a fresh DB where alembic runs before the Rails schema load, or the
+    # integration-test container). pg_dump does the same when restoring
+    # SQL-language functions. The function is only invoked at runtime, by
+    # which point the OSM tables exist.
+    op.execute(text("SET LOCAL check_function_bodies = off"))
     op.execute(text((_SQL_DIR / "osm_augmented_diff.sql").read_text()))
 
 

api/src/tasking/projects/repository.py

@@ -1006,11 +1006,25 @@ async def add_role(
 
         missing = await self._missing_user_auth_uids([body.user_id])
         if missing:  # User never logged in, so we try to provision them from TDEI
-            provisioned_users = await self._provision_users_from_tdei(
+            still_missing = await self._provision_users_from_tdei(
                 missing,
                 project_group_id=project_group_id,
                 bearer_token=user_token,
             )
+            # If TDEI doesn't list the user either, surface the same structured
+            # 422 the create path returns rather than falling through to a raw
+            # foreign-key violation with a generic string detail.
+            if still_missing:
+                raise HTTPException(
+                    status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                    detail={
+                        "message": (
+                            "The `user_id` is not a member of this "
+                            "workspace's project group in TDEI."
+                        ),
+                        "missing_user_ids": still_missing,
+                    },
+                )
 
         from sqlalchemy import text
 

pyproject.toml

@@ -37,11 +37,22 @@ dependencies = [
     "shapely>=2.1.2",
 ]
 
+[project.optional-dependencies]
+# Integration tests boot a real PostGIS database via testcontainers (needs a
+# running Docker daemon). Install with `uv sync --extra integration` or
+# `--all-extras`, then run them with `pytest -m integration`.
+integration = [
+    "testcontainers[postgres]>=4.0.0",
+]
+
 [tool.pytest.ini_options]
 addopts = "-v --cov=api --cov-report=term-missing"
 testpaths = ["tests"]
 asyncio_mode = "auto"
 asyncio_default_fixture_loop_scope = "function"
+markers = [
+    "integration: tests that require a live PostGIS database (Docker + testcontainers)",
+]
 
 [tool.black]
 line-length = 88

scripts/ci.sh

@@ -2,14 +2,26 @@
 # Run the CI checks locally, mirroring .github/workflows/ci.yml.
 #
 # Runs every check by default; a failure in one step does not stop the others,
-# and the script exits non-zero if any step failed. Pass --fail-fast to stop at
-# the first failure instead.
+# and the script exits non-zero if any step failed.
+#
+# Flags:
+#   --fail-fast     stop at the first failing step instead of running all
+#   --integration   also run the integration suite (`pytest -m integration`),
+#                   which boots a real PostGIS database via testcontainers and
+#                   therefore needs a running Docker daemon
 set -uo pipefail
 
 cd "$(dirname "$0")/.."
 
 fail_fast=0
-[[ "${1:-}" == "--fail-fast" ]] && fail_fast=1
+integration=0
+for arg in "$@"; do
+  case "${arg}" in
+    --fail-fast)   fail_fast=1 ;;
+    --integration) integration=1 ;;
+    *) echo "Unknown option: ${arg}" >&2; exit 2 ;;
+  esac
+done
 
 failed=()
 
@@ -42,4 +54,8 @@ run "Check code formatting (black)"  uv run black --check .
 run "Type-check with pyright"        uvx pyright --pythonpath .venv/bin/python api tests
 run "Run tests"                      uv run pytest tests
 
+if [[ "${integration}" == 1 ]]; then
+  run "Run integration tests"        uv run pytest tests -m integration
+fi
+
 summary_and_exit

tests/integration/conftest.py

@@ -180,13 +180,71 @@ def _pg_url(_pg_urls: tuple[str, str]) -> str:
 # ---------------------------------------------------------------------------
 
 
+async def _bootstrap_osm_database(osm_url: str) -> None:
+    """Prepare the provisioned OSM database for the osm alembic tree.
+
+    Two things the tree assumes but does not create itself:
+
+    * **PostGIS** — the tasking_* migration requires the extension and raises
+      if it is missing. The container image ships PostGIS but the extension
+      must be enabled per-database, and the freshly ``CREATE DATABASE``'d
+      ``osm_test`` doesn't inherit it.
+    * **users** — owned by the OSM Rails website, not these alembic trees:
+      migration ``9221408912dd`` adds a unique constraint to it and the
+      ``tasking_*`` foreign keys reference ``users.id`` / ``users.auth_uid``,
+      all assuming it already exists. We stand up a stripped-down version
+      (just the columns the migrations and ``_insert_user_row`` touch).
+    """
+    from sqlalchemy import text
+    from sqlalchemy.ext.asyncio import create_async_engine
+
+    engine = create_async_engine(osm_url, isolation_level="AUTOCOMMIT")
+    try:
+        async with engine.connect() as conn:
+            await conn.execute(text("CREATE EXTENSION IF NOT EXISTS postgis"))
+            await conn.execute(
+                text(
+                    # Columns mirror the subset of the OSM Rails `users` table
+                    # that the tasking code reads or writes: the FK targets
+                    # (id, auth_uid), the seed/display columns, and every column
+                    # the TDEI auto-provisioning INSERT populates
+                    # (see TaskingProjectRepository._provision_users_from_tdei).
+                    "CREATE TABLE IF NOT EXISTS users ("
+                    "  id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,"
+                    "  auth_uid varchar,"
+                    "  email varchar,"
+                    "  display_name varchar,"
+                    "  auth_provider varchar,"
+                    "  status varchar,"
+                    "  pass_crypt varchar,"
+                    "  data_public boolean,"
+                    "  email_valid boolean,"
+                    "  terms_seen boolean,"
+                    "  creation_time timestamp,"
+                    "  terms_agreed timestamp,"
+                    "  tou_agreed timestamp"
+                    ")"
+                )
+            )
+    finally:
+        await engine.dispose()
+
+
 @pytest.fixture(scope="session")
 def _migrated_db(_pg_urls: tuple[str, str]) -> tuple[str, str]:
     """Run alembic upgrades for both trees against their own databases.
 
     Returns ``(task_url, osm_url)`` after both heads are reached.
     """
+    import asyncio
+
     task_url, osm_url = _pg_urls
+
+    # PostGIS + the OSM-website-owned `users` table must exist before the osm
+    # alembic tree upgrades (its migrations require the extension and
+    # constrain `users.auth_uid`).
+    asyncio.run(_bootstrap_osm_database(osm_url))
+
     repo_root = os.path.dirname(
         os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
     )
@@ -214,39 +272,50 @@ def _migrated_db(_pg_urls: tuple[str, str]) -> tuple[str, str]:
 # ---------------------------------------------------------------------------
 
 
+async def _insert_workspace_row(task_url: str) -> None:
+    """Insert the seed workspace row into the TASK database."""
+    from uuid import uuid4
+
+    from sqlalchemy import text
+    from sqlalchemy.ext.asyncio import create_async_engine
+
+    engine = create_async_engine(task_url, future=True)
+    try:
+        async with engine.begin() as conn:
+            await conn.execute(
+                text(
+                    "INSERT INTO workspaces "
+                    '(id, type, title, "tdeiProjectGroupId", "createdAt", '
+                    ' "createdBy", "createdByName", "externalAppAccess") '
+                    "VALUES (:id, :type, :title, :pgid, NOW(), :uid, :uname, 0) "
+                    "ON CONFLICT (id) DO NOTHING"
+                ),
+                {
+                    "id": SEED_WORKSPACE_ID,
+                    "type": "osw",
+                    "title": "Test Workspace",
+                    "pgid": str(SEED_PROJECT_GROUP_ID),
+                    "uid": str(uuid4()),
+                    "uname": "seed",
+                },
+            )
+    finally:
+        await engine.dispose()
+
+
 @pytest.fixture(scope="session")
-async def _seed_workspace_row(_migrated_db: tuple[str, str]) -> int:
+def _seed_workspace_row(_migrated_db: tuple[str, str]) -> int:
     """Insert one workspace row so tenancy/permission gates resolve.
 
     Workspaces live in the TASK database (alongside teams, project groups),
-    so we point this at task_url.
+    so we point this at task_url. Kept synchronous (driving the async insert
+    via ``asyncio.run``) because a session-scoped *async* fixture would clash
+    with pytest-asyncio's function-scoped event loop.
     """
-    from uuid import uuid4
-
-    from sqlalchemy import text
-    from sqlalchemy.ext.asyncio import create_async_engine
+    import asyncio
 
     task_url, _osm_url = _migrated_db
-    engine = create_async_engine(task_url, future=True)
-    async with engine.begin() as conn:
-        await conn.execute(
-            text(
-                "INSERT INTO workspaces "
-                '(id, type, title, "tdeiProjectGroupId", "createdAt", '
-                ' "createdBy", "createdByName", "externalAppAccess") '
-                "VALUES (:id, :type, :title, :pgid, NOW(), :uid, :uname, 0) "
-                "ON CONFLICT (id) DO NOTHING"
-            ),
-            {
-                "id": SEED_WORKSPACE_ID,
-                "type": "osw",
-                "title": "Test Workspace",
-                "pgid": str(SEED_PROJECT_GROUP_ID),
-                "uid": str(uuid4()),
-                "uname": "seed",
-            },
-        )
-    await engine.dispose()
+    asyncio.run(_insert_workspace_row(task_url))
     return SEED_WORKSPACE_ID
 
 
@@ -319,6 +388,23 @@ def _clear_token() -> None:
 # ---------------------------------------------------------------------------
 
 
+@pytest.fixture
+def app():
+    """Real app for the integration suite.
+
+    Overrides the unit-suite ``app`` fixture (tests/conftest.py), which swaps
+    in ``FakeSession`` objects for the DB dependencies. Here we want the real
+    engines wired up by the autouse ``_per_test_db_sessions`` fixture, so this
+    just yields the actual FastAPI app untouched. The shared ``client``
+    fixture depends on ``app`` by name and picks this up for integration
+    tests. Per-test overrides (sessions, ``validate_token``) are installed and
+    torn down by their own fixtures.
+    """
+    from api.main import app as fastapi_app
+
+    return fastapi_app
+
+
 @pytest.fixture(autouse=True)
 async def _per_test_db_sessions(_pg_urls):
     from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine