ci: gate staging on extension tests (+ build-time caching) (#205)

* ci: gate staging build/deploy on extension test suite

Re-adds the CKAN extension tests that died with the Jenkins pipeline,
now as a 'test' job in build-deploy.yml that build/deploy depend on.

Brings up the docker-compose dev stack (the same flow the old
ci_setup.sh/ci_test.sh drove via adx) on pinned submodule commits and
runs the 5 Fjelltopp suites:
  unaids, validation, scheming, dhis2harvester, emailasusername

Gate semantics:
- push: tests must pass before build and deploy run
- redeploy (workflow_dispatch + image_tag): tests skipped, image
  already tested when built
- test failure blocks both build and deploy

Tests run against the dev image + bind-mounted submodules, matching
local 'adx test'. Not yet verified green under CKAN 2.11/Py3.10.

* ci: run tests on PRs + cache pipenv venv and React node_modules

- Add pull_request trigger so the extension-test job runs as a visible
  status check; build/deploy stay gated off pull_request events.
- Cache .adxvenv on Pipfile.lock — the dominant cost is bootstrap's
  'pipenv sync --dev' (CKAN + ~19 extensions), not the image build.
- Cache the unaids React node_modules on its yarn.lock.

Both caches are safe on miss, so the first run is a clean cold signal.

* ci: run all suites (no fail-fast) and save caches on failure

- Run all 5 extension suites and aggregate, so one run reports the full
  picture instead of stopping at the first failing suite.
- Split cache restore/save so .adxvenv and node_modules are saved even
  when tests fail, making triage runs warm instead of cold.

* test: fix extension test suite under CKAN 2.11/Py3.10

- Add 'mock' and 'pyfakefs' to dev-packages: ckanext-validation,
  -scheming and -emailasusername import the standalone 'mock' package
  (and pyfakefs in validation), which weren't installed, causing
  pytest collection errors. Dev-only — prod uses 'pipenv sync' without
  --dev, so these don't ship.
- run_tests: blank CKAN_SMTP_SERVER for test runs so suites never reach
  the dev stack's smtp4dev. Fixes ckanext-unaids'
  test_send_dataset_transfer_emails_errors, which asserts mail sending
  fails when no server is configured.

* deps: pin frictionless==5.13.1 and pyfakefs==4.6.* to match extensions

The validation/unaids extensions pin frictionless[ckan]==5.13.1 and
pyfakefs==4.6.* in their own requirements and pass their own CI against
those. Our merged Pipfile had frictionless>=5.0.0,<6.0.0 (drifted to a
newer 5.x that dropped Resource.__create__) and pyfakefs=* (6.x dropped
the CreateFile API), so ckanext-validation's test suite failed in our
stack only. Aligning the pins fixes it without touching the submodule;
frictionless is prod-facing but this matches the version the extensions
are built and tested against.

Author: cooper667

.github/workflows/build-deploy.yml

@@ -3,6 +3,10 @@ name: Build and Deploy CKAN (staging)
 on:
   push:
     branches: [master, ckan211-prod-deploy-pr]
+  pull_request:
+    # Runs the test job only (build/deploy are gated off pull_request below),
+    # so PRs get a visible test status check without deploying.
+    branches: [master, ckan211-prod-deploy-pr]
   workflow_dispatch:
     inputs:
       image_tag:
@@ -21,9 +25,124 @@ env:
   URL: https://dev.adr.fjelltopp.org
 
 jobs:
-  build:
+  test:
+    name: Extension tests
+    # Skip on a pure redeploy (workflow_dispatch with an existing image_tag);
+    # that image was already tested when it was built.
     if: github.event_name != 'workflow_dispatch' || inputs.image_tag == ''
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    env:
+      # Fresh runner DB — no need to restart the db container during testsetup.
+      SKIP_DB_RESTART: "True"
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          submodules: recursive
+
+      # The dominant cost is `pipenv sync --dev` at bootstrap (CKAN + ~19
+      # extensions), which lands in the bind-mounted .adxvenv. Caching it on
+      # Pipfile.lock makes warm runs a near no-op. .adxvenv is gitignored and
+      # written by root in-container, so there are no ownership issues here.
+      # Restore + save are split (rather than the combined cache action) so the
+      # venv is saved even when the test step fails — otherwise the expensive
+      # `pipenv sync` is repeated cold on every triage run. Saved at job end.
+      - name: Restore Python venv
+        id: venv-cache
+        uses: actions/cache/restore@v4
+        with:
+          path: .adxvenv
+          key: adxvenv-${{ runner.os }}-${{ hashFiles('Pipfile.lock') }}
+          restore-keys: |
+            adxvenv-${{ runner.os }}-
+
+      # Second cost: the entrypoint runs `yarn install` + build for the unaids
+      # React app on every boot. Cache its node_modules on the React yarn.lock.
+      - name: Restore unaids React node_modules
+        id: react-cache
+        uses: actions/cache/restore@v4
+        with:
+          path: submodules/ckanext-unaids/ckanext/unaids/react/node_modules
+          key: react-nm-${{ runner.os }}-${{ hashFiles('submodules/ckanext-unaids/ckanext/unaids/react/yarn.lock') }}
+          restore-keys: |
+            react-nm-${{ runner.os }}-
+
+      - name: Prepare .env
+        run: cp dev.env .env
+
+      - name: Build images and start the dev stack
+        run: |
+          ./adx build
+          ./adx up
+
+      - name: Wait for CKAN bootstrap
+        run: |
+          for i in $(seq 1 90); do
+            if docker logs ckan 2>&1 | grep -q 'CKAN bootstrapping finished, environment ready'; then
+              echo "CKAN ready after ${i} checks"
+              exit 0
+            fi
+            echo "Waiting for CKAN bootstrap (${i}/90)…"
+            sleep 10
+          done
+          echo "::error::CKAN did not finish bootstrapping in time"
+          docker logs ckan
+          exit 1
+
+      - name: Create test databases
+        run: ./adx testsetup
+
+      - name: Run extension tests
+        run: |
+          # Run every suite (don't fail-fast) so one run shows the full picture,
+          # then fail at the end if any suite failed.
+          failed=()
+          for ext in unaids validation scheming dhis2harvester emailasusername; do
+            echo "::group::ckanext-${ext}"
+            if ./adx test "${ext}" --no-interaction; then
+              echo "ckanext-${ext}: PASS"
+            else
+              echo "ckanext-${ext}: FAIL"
+              failed+=("${ext}")
+            fi
+            echo "::endgroup::"
+          done
+          echo "--- summary ---"
+          if [ ${#failed[@]} -ne 0 ]; then
+            echo "::error::Failing suites: ${failed[*]}"
+            exit 1
+          fi
+          echo "All extension suites passed"
+
+      - name: Dump CKAN logs on failure
+        if: failure()
+        run: docker logs ckan || true
+
+      - name: Save Python venv
+        if: always() && steps.venv-cache.outputs.cache-hit != 'true'
+        uses: actions/cache/save@v4
+        with:
+          path: .adxvenv
+          key: ${{ steps.venv-cache.outputs.cache-primary-key }}
+
+      - name: Save unaids React node_modules
+        if: always() && steps.react-cache.outputs.cache-hit != 'true'
+        uses: actions/cache/save@v4
+        with:
+          path: submodules/ckanext-unaids/ckanext/unaids/react/node_modules
+          key: ${{ steps.react-cache.outputs.cache-primary-key }}
+
+  build:
+    needs: test
+    # Run when tests pass (success) or were skipped (redeploy), and we're
+    # actually building (not a redeploy of an existing tag).
+    if: >-
+      always()
+      && github.event_name != 'pull_request'
+      && needs.test.result != 'failure'
+      && needs.test.result != 'cancelled'
+      && (github.event_name != 'workflow_dispatch' || inputs.image_tag == '')
+    runs-on: ubuntu-latest
     outputs:
       image_tag: ${{ steps.meta.outputs.version }}
     steps:
@@ -61,8 +180,15 @@ jobs:
           labels: ${{ steps.meta.outputs.labels }}
 
   deploy:
-    needs: build
-    if: always() && (needs.build.result == 'success' || needs.build.result == 'skipped')
+    needs: [test, build]
+    # Never deploy if tests failed/cancelled. Otherwise deploy when the image
+    # was built (success) or already exists (build skipped on redeploy).
+    if: >-
+      always()
+      && github.event_name != 'pull_request'
+      && needs.test.result != 'failure'
+      && needs.test.result != 'cancelled'
+      && (needs.build.result == 'success' || needs.build.result == 'skipped')
     runs-on: ubuntu-latest
     environment:
       name: staging

Pipfile

@@ -92,7 +92,10 @@ python-jose = "==3.3.0"
 setuptools = ">=65.0.0,<80.0.0"
 raven = "==6.10.0"
 tableschema = "==1.20.2"
-frictionless = ">=5.0.0,<6.0.0"
+# Pinned to match ckanext-validation/-unaids requirements.txt (the version
+# those extensions are built and tested against). Newer 5.x dropped
+# frictionless.Resource.__create__, which their test suites still patch.
+frictionless = "==5.13.1"
 markupsafe = "==2.1.5"
 
 [dev-packages]
@@ -120,6 +123,10 @@ pytest-rerunfailures = "==15.0"
 pytest-split = "==0.10.0"
 pytest-retry = "==1.7.0"
 pytest-mock = "*"
+mock = "*"
+# Pinned to match ckanext-validation dev-requirements.txt; pyfakefs 5.0+
+# removed the CamelCase API (CreateFile) its TestFiles suite uses.
+pyfakefs = "==4.6.*"
 coverage = "==7.7.1"
 junitparser = "==3.2.0"
 junit2html = "==31.0.2"

Pipfile.lock

@@ -162,7 +162,12 @@ def run_tests(args, extra):
     # for cases like ckanext-ytp-requests repository which uses ytp_requests test directory internally
     extension_sub_path = extension_sub_path.replace("-", "_")
     retcode = call_command([
+        # Blank out the SMTP server for test runs so tests never reach the
+        # dev stack's smtp4dev and send real mail. Some suites (e.g.
+        # ckanext-unaids' send_dataset_transfer_emails error path) assert that
+        # sending fails when no server is configured.
         f'docker exec {args.interaction} -e CKAN_SQLALCHEMY_URL={CKAN_TEST_SQLALCHEMY_URL} '
+        f'-e CKAN_SMTP_SERVER= '
         f'ckan /usr/local/bin/ckan-pytest --capture=no --disable-warnings '
         f'--ckan-ini={extension_path}/test.ini '
         f'{extension_path}/{extension_sub_path}/tests '