Merge pull request #1280 from makeabilitylab/1279-ci-test-shim-package-split

Add CI + test-settings shim + split tests into a package (#1279)

Author: jonfroehlich

.github/workflows/test.yml

@@ -0,0 +1,60 @@
+name: Tests
+
+# Runs the Django test suite on every push to master (our test-server deploy
+# trigger) and on every pull request. A failing run is a red ✗ + email — it
+# reports status only and does not block the push or the deploy.
+on:
+  push:
+    branches: [master]
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    # Postgres service mirrors the local-dev / server `db` container
+    # (postgres:16, makeability / admin / password).
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_DB: makeability
+          POSTGRES_USER: admin
+          POSTGRES_PASSWORD: password
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd "pg_isready -U admin -d makeability"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+
+    env:
+      # settings_test.py reads these to reach the Postgres service above.
+      DATABASE_HOST: localhost
+      DATABASE_PORT: 5432
+      DJANGO_ENV: DEBUG
+
+    steps:
+      - uses: actions/checkout@v4
+
+      # Mirror the Dockerfile's system deps: ImageMagick + Ghostscript power the
+      # PDF→thumbnail path that Artifact.save() runs (exercised by the Talk
+      # fixtures); libpq-dev is needed to build psycopg2 from source.
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends imagemagick ghostscript libpq-dev
+          sudo cp imagemagick-policy.xml /etc/ImageMagick-6/policy.xml
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.13"
+          cache: pip
+
+      - name: Install Python dependencies
+        run: pip install -r requirements.txt
+
+      - name: Run tests
+        run: python manage.py test website --settings=makeabilitylab.settings_test --verbosity=2

CLAUDE.md

@@ -37,11 +37,12 @@ A superuser is required to use `/admin` and add content; create one with `python
 
 ## Tests and accessibility checks
 
-- Tests: `python manage.py test website` (inside container). The suite has two styles, both in `website/tests.py`:
+- Tests: `python manage.py test website --settings=makeabilitylab.settings_test` (inside container). The tests live in the `website/tests/` package (one `test_*.py` per concern; Django auto-discovers them) with shared DB fixtures in `website/tests/base.py`. The suite has two styles:
   - **Unit** — `SimpleTestCase` + `MagicMock` for pure logic (formatters, BibTeX generation, etc.); no DB, runs in ms.
-  - **Integration** — `DatabaseTestCase` (subclass of Django's `TestCase`) for view / queryset / template regressions; each test runs in a transaction and rolls back. Has fixture helpers `make_person` / `make_publication` / `make_news_item`.
+  - **Integration** — `DatabaseTestCase` (subclass of Django's `TestCase`, in `tests/base.py`) for view / queryset / template regressions; each test runs in a transaction and rolls back. Has fixture helpers `make_person` / `make_publication` / `make_talk` / `make_news_item`.
   - When fixing a bug reachable through a real queryset, URL, or view, add a regression test in the matching style before applying the fix (matches the tests-first workflow).
-  - **Gotcha:** `website/migrations/` is gitignored, so each env has its own history. If `manage.py test` fails at DB creation with `column "..." already exists`, drop the stale test DB with `docker exec makeabilitylabwebsite-db-1 psql -U admin -d postgres -c "DROP DATABASE IF EXISTS test_makeability;"` and re-run. See #1267 for the durable fix.
+  - **Always use the `--settings=makeabilitylab.settings_test` shim.** It sets `MIGRATION_MODULES = {'website': None}` so the test DB is built directly from the current models, sidestepping the gitignored, per-environment `website/migrations/` history. This is the durable fix for #1267 — without it, a fresh test DB can fail at creation with `column "..." already exists` (old workaround: `docker exec makeabilitylabwebsite-db-1 psql -U admin -d postgres -c "DROP DATABASE IF EXISTS test_makeability;"`).
+  - **CI:** `.github/workflows/test.yml` runs this same command on every push to `master` and every PR (free/unlimited for this public repo). It reports a green ✓ / red ✗ — it does not block pushes or the deploy. See the testing roadmap in #1278.
 - Accessibility (Pa11y CI + Axe, WCAG 2.0 AA): start the site, then `docker-compose -f docker-compose-local-dev.yml --profile testing run --rm a11y`. URLs to scan are configured in `.pa11yci.json`. Run this before submitting UI changes.
 
 ## Deployment

CONTRIBUTING.md

@@ -249,20 +249,22 @@ A superuser account is required to access the Django admin interface and add con
 
 ## Running the Test Suite
 
-The Python test suite lives in `website/tests.py` and runs inside the website container:
+The Python test suite lives in the `website/tests/` package (one `test_*.py` module per concern — Django auto-discovers them) and runs inside the website container:
 
 ```bash
-docker exec makeabilitylabwebsite-website-1 python manage.py test website
+docker exec makeabilitylabwebsite-website-1 python manage.py test website --settings=makeabilitylab.settings_test
 ```
 
+**Always pass `--settings=makeabilitylab.settings_test`** (see [Troubleshooting tests](#troubleshooting-tests) for why). The same command runs automatically in CI on every push to `master` and every PR — see [Continuous integration](#continuous-integration).
+
 The suite has two complementary styles:
 
 | Style | Base class | What it's for |
 |---|---|---|
 | **Unit** | `SimpleTestCase` + `MagicMock` | Pure logic — formatters, BibTeX generation, single-method behavior. No DB; runs in milliseconds. |
-| **Integration** | `DatabaseTestCase` (subclass of Django's `TestCase`) | View, queryset, template, and URL-routing regressions. Each test runs inside a transaction that is rolled back, so tests stay isolated. |
+| **Integration** | `DatabaseTestCase` (subclass of Django's `TestCase`, in `website/tests/base.py`) | View, queryset, template, and URL-routing regressions. Each test runs inside a transaction that is rolled back, so tests stay isolated. |
 
-The `DatabaseTestCase` base provides `make_person`, `make_publication`, and `make_news_item` helpers built on plain `Model.objects.create()` — use those rather than hand-rolling fixtures.
+The `DatabaseTestCase` base provides `make_person`, `make_publication`, `make_talk`, and `make_news_item` helpers built on plain `Model.objects.create()` — use those rather than hand-rolling fixtures.
 
 ### When to add a test
 
@@ -276,13 +278,19 @@ If a fix is genuinely not unit-testable (FD leaks, `super().save()`-dependent pa
 
 ### Troubleshooting tests
 
-`website/migrations/` is **gitignored** — each environment (your laptop, test, production) maintains its own migration history on disk. This sometimes drifts. If `manage.py test` fails at test-DB creation, the symptoms and fixes are:
+`website/migrations/` is **gitignored** — each environment (your laptop, test, production) maintains its own migration history on disk, which can drift. The `--settings=makeabilitylab.settings_test` shim is the durable fix (#1267): it sets `MIGRATION_MODULES = {'website': None}`, so the test runner builds the `website` schema directly from the current models instead of replaying that local history. **Use the shim and these symptoms shouldn't appear at all.**
+
+If you forget the shim and the legacy `manage.py test website` fails at test-DB creation:
 
-- **`database "test_makeability" already exists`** — a prior failed run left it half-built. Drop and retry:
+- **`database "test_makeability" already exists`** — a prior failed run left it half-built. Drop and retry (or just switch to the shim):
   ```bash
   docker exec makeabilitylabwebsite-db-1 psql -U admin -d postgres -c "DROP DATABASE IF EXISTS test_makeability;"
   ```
-- **`column "..." of relation "..." already exists`** — a local migration file duplicates a field that a later `0001_initial` regeneration already includes. Same fix (drop the test DB) usually clears it; if it persists, the offending migration is a local stale artifact that needs to be edited or removed. See [#1267](https://github.com/makeabilitylab/makeabilitylabwebsite/issues/1267) for the durable fix (test-only settings shim using `MIGRATION_MODULES`).
+- **`column "..." of relation "..." already exists`** — a local migration file duplicates a field that a later `0001_initial` regeneration already includes. The shim sidesteps this entirely.
+
+### Continuous integration
+
+`.github/workflows/test.yml` runs the suite (with the test-settings shim, against a Postgres 16 service container) on every push to `master` and every pull request. GitHub Actions is free and unlimited for this public repo. A failing run shows a red ✗ on the commit/PR and emails the author — it **reports** status, it does not block the push or the test-server deploy. The broader testing roadmap (coverage, Pa11y-in-CI, test backfill) is tracked in [#1278](https://github.com/makeabilitylab/makeabilitylabwebsite/issues/1278).
 
 ## Accessibility Testing
 
@@ -317,7 +325,7 @@ Edit `.pa11yci.json` to add or remove URLs to test. The `urls` array lists every
 
 - **One issue per branch**: Keep PRs focused on a single issue for easier review.
 
-- **Run the test suite**: `docker exec makeabilitylabwebsite-website-1 python manage.py test website` should pass before opening a PR. If your fix is reachable through a real queryset, view, or template, add a regression test (see [Running the Test Suite](#running-the-test-suite)).
+- **Run the test suite**: `docker exec makeabilitylabwebsite-website-1 python manage.py test website --settings=makeabilitylab.settings_test` should pass before opening a PR (CI runs the same command). If your fix is reachable through a real queryset, view, or template, add a regression test (see [Running the Test Suite](#running-the-test-suite)).
 
 - **Test locally**: Verify your changes work in the browser before submitting.
 

docs/DEPLOYMENT.md

@@ -97,7 +97,7 @@ View current and past versions on the [Releases page](https://github.com/makeabi
 2. Confirm the Python test suite passes locally:
 
    ```bash
-   docker exec makeabilitylabwebsite-website-1 python manage.py test website
+   docker exec makeabilitylabwebsite-website-1 python manage.py test website --settings=makeabilitylab.settings_test
    ```
 
    (See [Running the Test Suite](../CONTRIBUTING.md#running-the-test-suite) in `CONTRIBUTING.md` for what the suite covers and how to add to it.)

makeabilitylab/settings_test.py

@@ -0,0 +1,45 @@
+"""
+Test-only Django settings.
+
+Run with:
+    python manage.py test website --settings=makeabilitylab.settings_test
+
+Why this exists
+---------------
+``website/migrations/`` is gitignored, so every environment (laptop, CI, the
+servers) carries its own migration history. That drift intermittently breaks a
+fresh test-DB build with ``column "..." already exists`` (see #1267 and
+CLAUDE.md). Setting ``MIGRATION_MODULES = {'website': None}`` tells Django to
+ignore the website app's migration history entirely and build its tables
+directly from the current models during test-DB setup (run_syncdb), which is
+both reproducible across environments and the durable fix for that flakiness.
+
+Only the *website* app is affected; third-party apps (admin, auth, ckeditor,
+sortedm2m, easy_thumbnails, image_cropping, ...) keep their shipped migrations.
+"""
+import os
+
+from makeabilitylab.settings import *  # noqa: F401,F403
+
+# Build website tables from models instead of replaying gitignored migrations.
+MIGRATION_MODULES = {"website": None}
+
+# Let CI point the database at its Postgres service container. Locally (inside
+# the website container) these env vars are unset, so we inherit HOST='db' from
+# the base settings fallback; CI sets DATABASE_HOST=localhost.
+DATABASES["default"]["HOST"] = os.environ.get(  # noqa: F405
+    "DATABASE_HOST", DATABASES["default"]["HOST"]  # noqa: F405
+)
+DATABASES["default"]["PORT"] = os.environ.get(  # noqa: F405
+    "DATABASE_PORT", DATABASES["default"].get("PORT", "5432")  # noqa: F405
+)
+
+# The base settings wire a RotatingFileHandler to /code/media/debug.log (a
+# container path). Django evaluates LOGGING at startup, so on any host without
+# that directory — a CI runner, a fresh checkout — django.setup() crashes
+# before a single test runs. Swap just the 'file' handler for a no-op; this
+# keeps every logger's handler reference valid while never touching disk.
+LOGGING["handlers"]["file"] = {"class": "logging.NullHandler"}  # noqa: F405
+
+# Speed up the auth tests (Data Health suite creates real superuser rows).
+PASSWORD_HASHERS = ["django.contrib.auth.hashers.MD5PasswordHasher"]