From 03f00f610ccc5591872235b6ca635e444244d530 Mon Sep 17 00:00:00 2001 From: srikoushik Date: Wed, 4 Nov 2026 11:36:33 +0530 Subject: [PATCH] ci: check pull requests for new lint findings and new test failures Nothing runs on pull requests today, and dev already has 38 failing tests and 137 ruff findings, so a new problem is hard to spot by eye. #1419 and #1420 show the cost: a repeated dict key (ruff F601) and then a regression that an existing test caught, both merged unnoticed. Add a "Checks" workflow for pull requests into dev and main. Each job compares the pull request with its target branch and fails only on what the pull request adds, so existing problems don't block anyone and no list of known failures has to be maintained: - lint: ruff 0.17.3 with bug-catching rules only (syntax errors, repeated dict keys, redefinitions, undefined names), counted per file and rule; - tests: runs in the published ghcr.io/core-stack-org/core-stack-backend image with a Postgres service, generates migrations the way database-init.sh does, runs `manage.py check`, then the suite on both branches, and fails if a test fails only on the pull request. Settings get placeholder values and a throwaway Fernet key; no secrets are needed. The workflow only reports a status; whether it blocks merging is a branch-protection setting for the maintainers. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/no_new_ruff_findings.py | 72 +++++++++++++ .github/scripts/no_new_test_failures.sh | 86 ++++++++++++++++ .github/workflows/checks.yml | 128 ++++++++++++++++++++++++ 3 files changed, 286 insertions(+) create mode 100644 .github/scripts/no_new_ruff_findings.py create mode 100644 .github/scripts/no_new_test_failures.sh create mode 100644 .github/workflows/checks.yml diff --git a/.github/scripts/no_new_ruff_findings.py b/.github/scripts/no_new_ruff_findings.py new file mode 100644 index 000000000..dfb37553b --- /dev/null +++ b/.github/scripts/no_new_ruff_findings.py @@ -0,0 +1,72 @@ +"""Fail if the pull request adds ruff findings that its target branch doesn't have. + +Usage: no_new_ruff_findings.py + +Only rules that point at real bugs are checked: syntax errors, repeated dict +keys, redefinitions and undefined names. Findings are counted per file and +rule, so moving code around doesn't count as new. +""" + +import json +import os +import subprocess +import sys +from collections import Counter + +RULES = "E9,F63,F7,F82,F601,F811" + + +def findings(ruff, directory): + result = subprocess.run( + [ + ruff, + "check", + "--select", + RULES, + "--exclude", + "*.ipynb", + "--output-format", + "json", + "--no-cache", + ".", + ], + cwd=directory, + capture_output=True, + text=True, + check=False, + ) + if result.returncode not in (0, 1): + sys.exit(f"ruff failed in {directory}:\n{result.stderr}") + return json.loads(result.stdout or "[]") + + +def key(finding, directory): + path = os.path.relpath(finding["filename"], os.path.abspath(directory)) + return path, finding["code"] + + +def main(): + ruff, base, head = sys.argv[1:4] + base_counts = Counter(key(f, base) for f in findings(ruff, base)) + head_findings = findings(ruff, head) + head_counts = Counter(key(f, head) for f in head_findings) + + added = {k for k, n in head_counts.items() if n > base_counts[k]} + if not added: + print( + f"No new findings for {RULES} " + f"({sum(head_counts.values())} already on the target branch)." + ) + return + + print(f"This pull request adds ruff findings ({RULES}):") + for f in head_findings: + path, code = key(f, head) + if (path, code) in added: + row = f["location"]["row"] + print(f" {path}:{row}: {code} {f['message']}") + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/.github/scripts/no_new_test_failures.sh b/.github/scripts/no_new_test_failures.sh new file mode 100644 index 000000000..b0438c0d4 --- /dev/null +++ b/.github/scripts/no_new_test_failures.sh @@ -0,0 +1,86 @@ +#!/bin/bash +# Run `manage.py check` and the test suite on the target branch and on the +# pull request, and fail only if the pull request breaks `check` or adds +# failing tests. +# Usage: no_new_test_failures.sh +set -u +base="$1" +head="$2" + +prepare() { + # Migrations are git-ignored and generated at install time, the same + # way as ensure_local_migration_packages in database-init.sh. + find . -maxdepth 2 -name apps.py -type f | while IFS= read -r f; do + mkdir -p "$(dirname "$f")/migrations" + touch "$(dirname "$f")/migrations/__init__.py" + done + mkdir -p tmp logs +} + +run_branch() { + ( + cd "$1" || exit 1 + prepare + if python manage.py check > check.txt 2>&1; then + echo "manage.py check: ok" + else + echo "manage.py check: FAILED" + tail -n 3 check.txt + fi + python manage.py makemigrations --skip-checks > /dev/null 2>&1 + python manage.py test --noinput -v 2 > suite.txt 2>&1 + # A test counts once, whether it errors or fails. + grep -E '^(ERROR|FAIL):' suite.txt | sed -E 's/^(ERROR|FAIL): //' | sort -u > failing.txt + grep -E '^Ran |^(OK|FAILED)' suite.txt + ) +} + +check_ok() { tail -n 1 "$1/check.txt" | grep -q "no issues"; } + +echo "Target branch:" +run_branch "$base" +echo "Pull request:" +run_branch "$head" + +ran() { grep -q '^Ran ' "$1/suite.txt"; } +if ! ran "$head"; then + if ! ran "$base"; then + echo "Warning: the test suite can't run on the target branch either, so this isn't counted against the pull request:" + tail -n 5 "$head/suite.txt" + exit 0 + fi + echo "The test suite does not run with this pull request:" + tail -n 30 "$head/suite.txt" + exit 1 +fi +if ! ran "$base"; then + echo "The test suite can't run on the target branch but runs with this pull request. Failing tests here, for information:" + cat "$head/failing.txt" + exit 0 +fi + +status=0 +if check_ok "$base" && ! check_ok "$head"; then + echo "This pull request breaks manage.py check:" + cat "$head/check.txt" + status=1 +elif ! check_ok "$head"; then + echo "Warning: manage.py check also fails on the target branch, so it isn't counted against this pull request." +fi + +added=$(comm -13 "$base/failing.txt" "$head/failing.txt") +fixed=$(comm -23 "$base/failing.txt" "$head/failing.txt") +if [ -n "$fixed" ]; then + echo "Now passing:" + echo "$fixed" +fi +if [ -n "$added" ]; then + echo "Failing only with this pull request:" + echo "$added" + echo "Full output of the pull request's run:" + cat "$head/suite.txt" + status=1 +else + echo "No new failing tests ($(wc -l < "$head/failing.txt" | tr -d ' ') already failing on the target branch)." +fi +exit $status diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml new file mode 100644 index 000000000..76b47f576 --- /dev/null +++ b/.github/workflows/checks.yml @@ -0,0 +1,128 @@ +name: Checks + +# Each job compares the pull request with its target branch and fails only +# on problems the pull request adds, so existing issues on dev don't block it. + +on: + pull_request: + branches: [dev, main] + +permissions: + contents: read + +concurrency: + group: checks-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + lint: + name: No new ruff findings + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + path: head + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base + - name: Install ruff + run: | + python3 -m venv /tmp/ruff + /tmp/ruff/bin/pip install --quiet ruff==0.17.3 + - name: Compare with the target branch + run: python3 head/.github/scripts/no_new_ruff_findings.py /tmp/ruff/bin/ruff base head + + tests: + name: Django check and no new test failures + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: corestack + POSTGRES_PASSWORD: corestack + POSTGRES_DB: corestack + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + env: + # Placeholder settings: the tests mock external services. + DB_HOST: localhost + DB_PORT: "5432" + DB_NAME: corestack + DB_USER: corestack + DB_PASSWORD: corestack + SECRET_KEY: ci-only-not-a-secret + ALLOWED_HOSTS: localhost,127.0.0.1 + CORS_ALLOWED_ORIGINS: https://localhost + MISSING_LAYER_RECIPIENTS: ci@example.com + ADMIN_GROUP_ID: "1" + GEE_DEFAULT_ACCOUNT_ID: "1" + GEE_HELPER_ACCOUNT_ID: "1" + GEE_SERVICE_ACCOUNT_KEY_PATH: /tmp/none.json + GEE_HELPER_SERVICE_ACCOUNT_KEY_PATH: /tmp/none.json + GEE_DATASETS_SERVICE_ACCOUNT_KEY_PATH: /tmp/none.json + GEE_STORAGE_PROJECT: ci + GEE_STORAGE_PROJECT_HELPER: ci + GCS_BUCKET_NAME: ci + NREGA_BUCKET: ci + CE_BUCKET_NAME: ci + CE_BUCKET_URL: https://localhost + CE_API_URL: https://localhost + BASE_API_URL: https://localhost + LOCAL_COMPUTE_API_URL: https://localhost + OVERPASS_URL: https://localhost + ODK_USERNAME: ci + ODK_PASSWORD: ci + ODK_USER_EMAIL_SYNC: ci@example.com + ODK_USER_PASSWORD_SYNC: ci + AUTH_TOKEN_FB_META: ci + AUTH_TOKEN_360: ci + CALL_PATCH_API_KEY: ci + EARTH_DATA_USER: ci + EARTH_DATA_PASSWORD: ci + USERNAME_GESDISC: ci + PASSWORD_GESDISC: ci + EMAIL_HOST_USER: ci@example.com + EMAIL_HOST_PASSWORD: ci + ES_AUTH: ci + S3_ACCESS_KEY: ci + S3_SECRET_KEY: ci + S3_BUCKET: ci + S3_REGION: ap-south-1 + EXCEL_DIR: /tmp/ci-excel + # Run Celery tasks in-process: CI has no broker or workers, and with + # eager mode off the tests that dispatch tasks wait on broker retries. + CELERY_TASK_ALWAYS_EAGER: "True" + steps: + - uses: actions/checkout@v6 + with: + path: head + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base + # Built from the pull request's own environment.yml, so a dependency + # missing from it shows up here. + - uses: mamba-org/setup-micromamba@v3 + with: + environment-file: head/installation/environment.yml + environment-name: corestackenv + cache-environment: true + # The same geetools fix the Dockerfile applies. + - name: Install geetools as the Dockerfile does + shell: bash -el {0} + run: | + python -m pip uninstall -y geetools || true + micromamba install -y -n corestackenv -c conda-forge --force-reinstall geetools=1.15.0 + - name: Generate a throwaway Fernet key + shell: bash -el {0} + run: python -c "from cryptography.fernet import Fernet; print('FERNET_KEY=' + Fernet.generate_key().decode())" >> "$GITHUB_ENV" + - name: Run the suite on both branches and compare + shell: bash -el {0} + run: bash head/.github/scripts/no_new_test_failures.sh base head