Skip to content

ci: add pytest, ruff, mypy, and Docker build verification to CI #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LarytheLord wants to merge 1 commit into from
Closed

ci: add pytest, ruff, mypy, and Docker build verification to CI #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions .github/workflows/test-lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
name: Test & Lint

on:
push:
branches: ["main"]
pull_request:
branches: ["main"]
workflow_dispatch:

permissions:
contents: read

jobs:
test:
name: Python Tests
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6

- uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: 'pip'
cache-dependency-path: |
auth-proxy/requirements.txt
requirements-test.txt

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -r auth-proxy/requirements.txt
pip install -r requirements-test.txt

- name: Run tests
working-directory: .
run: |
python -m pytest tests/ -v --tb=short

lint:
name: Ruff Lint & Format
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6

- uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: 'pip'

- name: Install ruff
run: pip install ruff

- name: Run ruff check
run: ruff check auth-proxy/ scripts/ tests/

- name: Run ruff format check
run: ruff format --check auth-proxy/ scripts/ tests/

typecheck:
name: Mypy Type Check
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6

- uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: 'pip'
cache-dependency-path: |
auth-proxy/requirements.txt
requirements-test.txt

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -r auth-proxy/requirements.txt
pip install -r requirements-test.txt
pip install mypy

- name: Run mypy
run: |
echo "Mypy is advisory — errors are reported but do not fail CI."
echo "Fix errors incrementally as you touch files."
mypy auth-proxy/ scripts/ --ignore-missing-imports --check-untyped-defs --no-strict-optional || true

docker:
name: Docker Build Check
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v6

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

- name: Build Docker image
uses: docker/build-push-action@v6
with:
context: .
push: false
load: true
tags: privatemode-proxy:test
cache-from: type=gha
cache-to: type=gha,mode=max

- name: Verify container starts
run: |
docker run --rm -d --name test-container \
-e API_KEYS_FILE=/app/secrets/keys.json \
-e UPSTREAM_URL=http://localhost:8081 \
-e PORT=8080 \
-e PRIVATEMODE_API_KEY=test-key \
-e FORCE_HTTPS=false \
privatemode-proxy:test
sleep 5
docker logs test-container
docker stop test-container
Comment on lines +109 to +120
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for PBKDF2_SALT usage and the ValueError it raises
rg -n "PBKDF2_SALT" --type py -A 5 -B 2

Repository: Open-Paws/privatemode-proxy

Length of output: 2536

Docker smoke test may silently mask container startup failures.

Two compounding issues:

  1. Missing PBKDF2_SALT and ADMIN_PASSWORD: auth-proxy/admin.py raises ValueError at import time if PBKDF2_SALT is shorter than 16 bytes. The docker run command doesn't set these variables, so the container will crash immediately on startup. The test configuration in tests/conftest.py properly sets both to valid defaults.

  2. --rm + crash = misleading failure: With --rm -d, if the container exits within the 5-second window due to the startup error above, it is automatically removed. Subsequent docker logs test-container and docker stop test-container commands then fail with "no such container" — the CI step exits non-zero for the wrong reason and the actual crash logs are lost.

🔧 Proposed fix 
       - name: Verify container starts
         run: |
-          docker run --rm -d --name test-container \
+          docker run -d --name test-container \
             -e API_KEYS_FILE=/app/secrets/keys.json \
             -e UPSTREAM_URL=http://localhost:8081 \
             -e PORT=8080 \
             -e PRIVATEMODE_API_KEY=test-key \
             -e FORCE_HTTPS=false \
+            -e PBKDF2_SALT=test-salt-value-1234567890 \
+            -e ADMIN_PASSWORD=test-admin-password \
             privatemode-proxy:test
           sleep 5
           docker logs test-container
-          docker stop test-container
+          docker stop test-container || true
+          docker rm -f test-container || true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Verify container starts
run: |
docker run --rm -d --name test-container \
-e API_KEYS_FILE=/app/secrets/keys.json \
-e UPSTREAM_URL=http://localhost:8081 \
-e PORT=8080 \
-e PRIVATEMODE_API_KEY=test-key \
-e FORCE_HTTPS=false \
privatemode-proxy:test
sleep 5
docker logs test-container
docker stop test-container
- name: Verify container starts
run: |
docker run -d --name test-container \
-e API_KEYS_FILE=/app/secrets/keys.json \
-e UPSTREAM_URL=http://localhost:8081 \
-e PORT=8080 \
-e PRIVATEMODE_API_KEY=test-key \
-e FORCE_HTTPS=false \
-e PBKDF2_SALT=test-salt-value-1234567890 \
-e ADMIN_PASSWORD=test-admin-password \
privatemode-proxy:test
sleep 5
docker logs test-container
docker stop test-container || true
docker rm -f test-container || true
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test-lint.yml around lines 109 - 120, The workflow's
docker run smoke test must supply the env vars required by auth-proxy/admin.py
and avoid auto-removal so startup crashes aren't hidden; update the docker run
in the Verify container starts step to set PBKDF2_SALT and ADMIN_PASSWORD (use
the same safe defaults as tests/conftest.py), remove the --rm flag and keep the
container name, wait, then explicitly inspect container status and run docker
logs test-container before stopping and removing it so crash logs are preserved
for debugging.

Loading
Loading