Merge pull request #284 from christianlouis/copilot/fix-ruff-formatting-errors
Fix ruff formatting errors and implement CI auto-fix pipeline
This commit is contained in:
+30
-24
@@ -25,12 +25,38 @@ env:
|
|||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
# Stage 1: Quality Checks (run in parallel)
|
# Stage 1: Ruff Lint & Format (runs first to catch style issues early)
|
||||||
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
|
lint:
|
||||||
|
name: Ruff Lint & Format
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- name: Checkout Code
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
|
||||||
|
- name: Set up Python
|
||||||
|
uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: "3.11"
|
||||||
|
|
||||||
|
- name: Install Ruff
|
||||||
|
run: pip install ruff
|
||||||
|
|
||||||
|
- name: Run Ruff Check
|
||||||
|
run: ruff check app/ tests/
|
||||||
|
|
||||||
|
- name: Run Ruff Format
|
||||||
|
run: ruff format --check app/ tests/
|
||||||
|
|
||||||
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
|
# Stage 2: Tests & Type Checking (run in parallel after lint passes)
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
test:
|
test:
|
||||||
name: Tests
|
name: Tests
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
needs: [lint] # Wait for lint to pass before running tests
|
||||||
services:
|
services:
|
||||||
redis:
|
redis:
|
||||||
image: redis:7
|
image: redis:7
|
||||||
@@ -91,30 +117,10 @@ jobs:
|
|||||||
junit.xml
|
junit.xml
|
||||||
coverage.xml
|
coverage.xml
|
||||||
|
|
||||||
lint:
|
|
||||||
name: Ruff Lint & Format
|
|
||||||
runs-on: ubuntu-latest
|
|
||||||
steps:
|
|
||||||
- name: Checkout Code
|
|
||||||
uses: actions/checkout@v4
|
|
||||||
|
|
||||||
- name: Set up Python
|
|
||||||
uses: actions/setup-python@v5
|
|
||||||
with:
|
|
||||||
python-version: "3.11"
|
|
||||||
|
|
||||||
- name: Install Ruff
|
|
||||||
run: pip install ruff
|
|
||||||
|
|
||||||
- name: Run Ruff Check
|
|
||||||
run: ruff check app/ tests/
|
|
||||||
|
|
||||||
- name: Run Ruff Format
|
|
||||||
run: ruff format --check app/ tests/
|
|
||||||
|
|
||||||
mypy:
|
mypy:
|
||||||
name: Mypy
|
name: Mypy
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
needs: [lint] # Wait for lint to pass before running type checks
|
||||||
steps:
|
steps:
|
||||||
- name: Checkout Code
|
- name: Checkout Code
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
@@ -133,7 +139,7 @@ jobs:
|
|||||||
run: mypy app/
|
run: mypy app/
|
||||||
|
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
# Stage 2: Build & Push Docker Image (only after all Stage 1 jobs pass)
|
# Stage 3: Build & Push Docker Image (only after all Stage 2 jobs pass)
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
build:
|
build:
|
||||||
@@ -196,7 +202,7 @@ jobs:
|
|||||||
cache-to: type=gha,mode=max
|
cache-to: type=gha,mode=max
|
||||||
|
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
# Stage 3: Deploy (only after build succeeds, only on main branch)
|
# Stage 4: Deploy (only after build succeeds, only on main branch)
|
||||||
# ══════════════════════════════════════════════════════════════════════════
|
# ══════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
deploy:
|
deploy:
|
||||||
|
|||||||
@@ -0,0 +1,94 @@
|
|||||||
|
name: Ruff Auto-Fix
|
||||||
|
|
||||||
|
# This workflow automatically fixes ruff formatting and linting issues
|
||||||
|
# and commits them back to the PR branch when issues are detected.
|
||||||
|
|
||||||
|
on:
|
||||||
|
pull_request:
|
||||||
|
branches:
|
||||||
|
- main
|
||||||
|
- develop
|
||||||
|
paths:
|
||||||
|
- '**.py'
|
||||||
|
workflow_dispatch: # Allow manual triggering
|
||||||
|
|
||||||
|
permissions:
|
||||||
|
contents: write
|
||||||
|
pull-requests: write
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
ruff-auto-fix:
|
||||||
|
name: Auto-fix Ruff Issues
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
# Only run on PRs from the same repository (not forks) for security
|
||||||
|
if: github.event.pull_request.head.repo.full_name == github.repository
|
||||||
|
|
||||||
|
steps:
|
||||||
|
- name: Checkout PR branch
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
ref: ${{ github.head_ref }}
|
||||||
|
token: ${{ secrets.GITHUB_TOKEN }}
|
||||||
|
|
||||||
|
- name: Set up Python
|
||||||
|
uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: "3.11"
|
||||||
|
|
||||||
|
- name: Install Ruff
|
||||||
|
run: pip install ruff
|
||||||
|
|
||||||
|
- name: Run Ruff Check with Auto-fix
|
||||||
|
run: |
|
||||||
|
echo "Running ruff check with auto-fix..."
|
||||||
|
ruff check app/ tests/ --fix || true
|
||||||
|
|
||||||
|
- name: Run Ruff Format
|
||||||
|
run: |
|
||||||
|
echo "Running ruff format..."
|
||||||
|
ruff format app/ tests/
|
||||||
|
|
||||||
|
- name: Check for changes
|
||||||
|
id: check_changes
|
||||||
|
run: |
|
||||||
|
if [[ -n $(git status --porcelain) ]]; then
|
||||||
|
echo "changes=true" >> $GITHUB_OUTPUT
|
||||||
|
echo "Changes detected after running ruff auto-fix"
|
||||||
|
else
|
||||||
|
echo "changes=false" >> $GITHUB_OUTPUT
|
||||||
|
echo "No changes needed - code is already properly formatted"
|
||||||
|
fi
|
||||||
|
|
||||||
|
- name: Commit and push changes
|
||||||
|
if: steps.check_changes.outputs.changes == 'true'
|
||||||
|
run: |
|
||||||
|
git config --local user.email "github-actions[bot]@users.noreply.github.com"
|
||||||
|
git config --local user.name "github-actions[bot]"
|
||||||
|
git add app/ tests/
|
||||||
|
git commit -m "style: apply ruff auto-fix
|
||||||
|
|
||||||
|
- Auto-formatted code with ruff format
|
||||||
|
- Applied ruff linting fixes with --fix
|
||||||
|
|
||||||
|
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>"
|
||||||
|
git push
|
||||||
|
|
||||||
|
- name: Comment on PR
|
||||||
|
if: steps.check_changes.outputs.changes == 'true'
|
||||||
|
uses: actions/github-script@v7
|
||||||
|
with:
|
||||||
|
script: |
|
||||||
|
github.rest.issues.createComment({
|
||||||
|
issue_number: context.issue.number,
|
||||||
|
owner: context.repo.owner,
|
||||||
|
repo: context.repo.repo,
|
||||||
|
body: '✨ Ruff auto-fix applied! The code has been automatically formatted and linting issues have been fixed.\n\nPlease pull the latest changes:\n```bash\ngit pull\n```'
|
||||||
|
})
|
||||||
|
|
||||||
|
- name: Summary
|
||||||
|
run: |
|
||||||
|
if [[ "${{ steps.check_changes.outputs.changes }}" == "true" ]]; then
|
||||||
|
echo "✅ Ruff auto-fix completed and changes committed"
|
||||||
|
else
|
||||||
|
echo "✅ No changes needed - code is already properly formatted"
|
||||||
|
fi
|
||||||
+62
-18
@@ -151,13 +151,13 @@ DocuElevate uses [semantic-release](https://github.com/semantic-release/semantic
|
|||||||
|
|
||||||
Before submitting a pull request:
|
Before submitting a pull request:
|
||||||
|
|
||||||
- [ ] Code follows the project style guide (Black, isort, flake8)
|
- [ ] Code follows the project style guide (Ruff)
|
||||||
- [ ] Commit messages follow conventional commit format
|
- [ ] Commit messages follow conventional commit format
|
||||||
|
- [ ] Pre-commit hooks installed and passing (see below)
|
||||||
- [ ] Tests added/updated for new functionality
|
- [ ] Tests added/updated for new functionality
|
||||||
- [ ] Documentation updated if user-facing changes
|
- [ ] Documentation updated if user-facing changes
|
||||||
- [ ] No manual edits to `VERSION` or `CHANGELOG.md`
|
- [ ] No manual edits to `VERSION` or `CHANGELOG.md`
|
||||||
- [ ] All tests pass locally
|
- [ ] All tests pass locally
|
||||||
- [ ] Pre-commit hooks pass
|
|
||||||
- [ ] Security scan passes (if applicable)
|
- [ ] Security scan passes (if applicable)
|
||||||
|
|
||||||
## Development Environment
|
## Development Environment
|
||||||
@@ -176,8 +176,33 @@ source venv/bin/activate # On Windows: venv\Scripts\activate
|
|||||||
# Install dependencies
|
# Install dependencies
|
||||||
pip install -r requirements.txt
|
pip install -r requirements.txt
|
||||||
pip install -r requirements-dev.txt
|
pip install -r requirements-dev.txt
|
||||||
|
|
||||||
|
# Install pre-commit hooks (recommended)
|
||||||
|
pre-commit install
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### Pre-commit Hooks
|
||||||
|
|
||||||
|
Pre-commit hooks automatically check your code before each commit, catching issues early:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Install the hooks (one-time setup)
|
||||||
|
pre-commit install
|
||||||
|
|
||||||
|
# Run hooks manually on all files
|
||||||
|
pre-commit run --all-files
|
||||||
|
|
||||||
|
# Run hooks on staged files (happens automatically on commit)
|
||||||
|
pre-commit run
|
||||||
|
```
|
||||||
|
|
||||||
|
The pre-commit hooks include:
|
||||||
|
- **Ruff** - Linting and formatting (with auto-fix)
|
||||||
|
- **Mypy** - Type checking
|
||||||
|
- **detect-secrets** - Secret detection
|
||||||
|
- **Conventional commits** - Commit message validation
|
||||||
|
- File checks (trailing whitespace, large files, etc.)
|
||||||
|
|
||||||
### Running Tests
|
### Running Tests
|
||||||
|
|
||||||
DocuElevate has comprehensive test coverage including unit tests, integration tests, and end-to-end tests. Tests are automatically configured with the necessary environment variables.
|
DocuElevate has comprehensive test coverage including unit tests, integration tests, and end-to-end tests. Tests are automatically configured with the necessary environment variables.
|
||||||
@@ -264,37 +289,56 @@ Tests are organized using pytest markers:
|
|||||||
|
|
||||||
#### Running Tests in CI
|
#### Running Tests in CI
|
||||||
|
|
||||||
Tests run automatically in GitHub Actions for all pull requests. The CI workflow splits every check into an **independent parallel job** so that a failure in one tool never blocks the others:
|
Tests run automatically in GitHub Actions for all pull requests. The CI workflow is organized in stages:
|
||||||
|
|
||||||
|
**Stage 1: Ruff Lint & Format** (runs first)
|
||||||
|
- Checks code style, formatting, and basic security issues
|
||||||
|
- Must pass before tests run
|
||||||
|
|
||||||
|
**Stage 2: Tests & Type Checking** (runs after lint passes)
|
||||||
|
|
||||||
| Job | Tool | What it checks |
|
| Job | Tool | What it checks |
|
||||||
|--------|--------|--------------------------------------|
|
|--------|--------|--------------------------------------|
|
||||||
| `test` | pytest | Unit/integration tests + coverage |
|
| `test` | pytest | Unit/integration tests + coverage |
|
||||||
| `flake8`| flake8 | PEP 8 style |
|
|
||||||
| `black` | black | Code formatting |
|
|
||||||
| `mypy` | mypy | Static type checking |
|
| `mypy` | mypy | Static type checking |
|
||||||
| `pylint`| pylint | Code quality |
|
|
||||||
| `bandit`| bandit | Security vulnerabilities |
|
|
||||||
|
|
||||||
All jobs are enforced — failures in any linter will block the PR. For full details see [docs/CIWorkflow.md](docs/CIWorkflow.md).
|
**Stage 3: Docker Build** (runs after all checks pass)
|
||||||
|
- Builds and pushes Docker images
|
||||||
|
|
||||||
|
**Stage 4: Deploy** (only on main branch)
|
||||||
|
- Deploys to production
|
||||||
|
|
||||||
|
**Auto-fix Workflow:**
|
||||||
|
- A separate `ruff-auto-fix` workflow automatically fixes formatting issues on PRs
|
||||||
|
- Commits fixes back to the PR branch
|
||||||
|
- Only runs on PRs from the same repository (not forks)
|
||||||
|
|
||||||
|
For full details see [docs/CIWorkflow.md](docs/CIWorkflow.md) and [docs/CIToolsGuide.md](docs/CIToolsGuide.md).
|
||||||
|
|
||||||
### Code Style
|
### Code Style
|
||||||
|
|
||||||
We use:
|
DocuElevate uses **Ruff** for all Python code quality checks:
|
||||||
- Black for Python code formatting (line length: 120)
|
|
||||||
- Flake8 for linting
|
- **Linting** - PEP 8 style, code quality, and security checks
|
||||||
- isort for import sorting (Black-compatible profile)
|
- **Formatting** - Consistent code formatting (120 character line length)
|
||||||
|
- **Import sorting** - Organized imports
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Format code
|
# Check for linting issues
|
||||||
black .
|
ruff check app/ tests/
|
||||||
|
|
||||||
# Check linting
|
# Auto-fix linting issues
|
||||||
flake8
|
ruff check app/ tests/ --fix
|
||||||
|
|
||||||
# Sort imports
|
# Check formatting
|
||||||
isort .
|
ruff format --check app/ tests/
|
||||||
|
|
||||||
|
# Auto-format code
|
||||||
|
ruff format app/ tests/
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Note:** The pre-commit hooks and CI pipeline will automatically check (and optionally fix) these for you.
|
||||||
|
|
||||||
## Project Structure
|
## Project Structure
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|||||||
@@ -110,7 +110,9 @@ class TestAzureTestConnectionEndpoint:
|
|||||||
@patch("app.api.azure.AzureKeyCredential")
|
@patch("app.api.azure.AzureKeyCredential")
|
||||||
@patch("app.api.azure.settings")
|
@patch("app.api.azure.settings")
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_azure_connection_service_request_error(self, mock_settings, mock_credential, mock_admin_client_class):
|
async def test_azure_connection_service_request_error(
|
||||||
|
self, mock_settings, mock_credential, mock_admin_client_class
|
||||||
|
):
|
||||||
"""Test connection with service request error."""
|
"""Test connection with service request error."""
|
||||||
from app.api.azure import test_azure_connection
|
from app.api.azure import test_azure_connection
|
||||||
|
|
||||||
@@ -196,7 +198,9 @@ class TestAzureTestConnectionEndpoint:
|
|||||||
@patch("app.api.azure.AzureKeyCredential")
|
@patch("app.api.azure.AzureKeyCredential")
|
||||||
@patch("app.api.azure.settings")
|
@patch("app.api.azure.settings")
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_azure_connection_with_empty_operations(self, mock_settings, mock_credential, mock_admin_client_class):
|
async def test_azure_connection_with_empty_operations(
|
||||||
|
self, mock_settings, mock_credential, mock_admin_client_class
|
||||||
|
):
|
||||||
"""Test connection returning empty operations list."""
|
"""Test connection returning empty operations list."""
|
||||||
from app.api.azure import test_azure_connection
|
from app.api.azure import test_azure_connection
|
||||||
|
|
||||||
@@ -325,7 +329,9 @@ class TestAzureTestConnectionEndpoint:
|
|||||||
@patch("app.api.azure.AzureKeyCredential")
|
@patch("app.api.azure.AzureKeyCredential")
|
||||||
@patch("app.api.azure.settings")
|
@patch("app.api.azure.settings")
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_azure_connection_uses_credential(self, mock_settings, mock_credential_class, mock_admin_client_class):
|
async def test_azure_connection_uses_credential(
|
||||||
|
self, mock_settings, mock_credential_class, mock_admin_client_class
|
||||||
|
):
|
||||||
"""Test that AzureKeyCredential is used correctly."""
|
"""Test that AzureKeyCredential is used correctly."""
|
||||||
from app.api.azure import test_azure_connection
|
from app.api.azure import test_azure_connection
|
||||||
|
|
||||||
|
|||||||
@@ -244,9 +244,9 @@ class TestAzureDocumentIntelligenceIntegration:
|
|||||||
assert len(result.content) > 10, f"OCR text too short: {result.content[:50]}"
|
assert len(result.content) > 10, f"OCR text too short: {result.content[:50]}"
|
||||||
|
|
||||||
# Verify the generated text is recognizable
|
# Verify the generated text is recognizable
|
||||||
assert (
|
assert "Acme" in result.content or "Invoice" in result.content, (
|
||||||
"Acme" in result.content or "Invoice" in result.content
|
f"OCR text does not contain expected keywords: {result.content[:200]}"
|
||||||
), f"OCR text does not contain expected keywords: {result.content[:200]}"
|
)
|
||||||
|
|
||||||
# Retrieve the searchable PDF output
|
# Retrieve the searchable PDF output
|
||||||
operation_id = poller.details["operation_id"]
|
operation_id = poller.details["operation_id"]
|
||||||
@@ -602,9 +602,9 @@ class TestFullOCRMetadataPipeline:
|
|||||||
|
|
||||||
# The generated invoice should be classified reasonably
|
# The generated invoice should be classified reasonably
|
||||||
doc_type = metadata["document_type"].lower()
|
doc_type = metadata["document_type"].lower()
|
||||||
assert any(
|
assert any(kw in doc_type for kw in ("invoice", "rechnung", "bill")), (
|
||||||
kw in doc_type for kw in ("invoice", "rechnung", "bill")
|
f"Unexpected document_type: {metadata['document_type']}"
|
||||||
), f"Unexpected document_type: {metadata['document_type']}"
|
)
|
||||||
finally:
|
finally:
|
||||||
os.unlink(pdf_path)
|
os.unlink(pdf_path)
|
||||||
|
|
||||||
|
|||||||
@@ -83,9 +83,9 @@ class TestSplitPdfBySize:
|
|||||||
# that can cause files to exceed the target size by ~20-50%. We allow 1.5x (50%) margin.
|
# that can cause files to exceed the target size by ~20-50%. We allow 1.5x (50%) margin.
|
||||||
PDF_OVERHEAD_MULTIPLIER = 1.5
|
PDF_OVERHEAD_MULTIPLIER = 1.5
|
||||||
for split_file in split_files:
|
for split_file in split_files:
|
||||||
assert (
|
assert os.path.getsize(split_file) <= max_size * PDF_OVERHEAD_MULTIPLIER, (
|
||||||
os.path.getsize(split_file) <= max_size * PDF_OVERHEAD_MULTIPLIER
|
f"Split file {split_file} should respect size limit (with PDF overhead allowance)"
|
||||||
), f"Split file {split_file} should respect size limit (with PDF overhead allowance)"
|
)
|
||||||
|
|
||||||
# Cleanup split files
|
# Cleanup split files
|
||||||
for split_file in split_files:
|
for split_file in split_files:
|
||||||
|
|||||||
@@ -144,9 +144,9 @@ def test_x_frame_options_valid_value(client):
|
|||||||
x_frame_value = response.headers["X-Frame-Options"]
|
x_frame_value = response.headers["X-Frame-Options"]
|
||||||
valid_values = ["DENY", "SAMEORIGIN"]
|
valid_values = ["DENY", "SAMEORIGIN"]
|
||||||
# Note: ALLOW-FROM is deprecated in modern browsers; use CSP frame-ancestors instead
|
# Note: ALLOW-FROM is deprecated in modern browsers; use CSP frame-ancestors instead
|
||||||
assert x_frame_value in valid_values or x_frame_value.startswith(
|
assert x_frame_value in valid_values or x_frame_value.startswith("ALLOW-FROM"), (
|
||||||
"ALLOW-FROM"
|
f"Invalid X-Frame-Options value: {x_frame_value}"
|
||||||
), f"Invalid X-Frame-Options value: {x_frame_value}"
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
|
|||||||
@@ -24,9 +24,7 @@ class TestUploadToFtp:
|
|||||||
@patch("app.tasks.upload_to_ftp.os.path.exists")
|
@patch("app.tasks.upload_to_ftp.os.path.exists")
|
||||||
@patch("app.tasks.upload_to_ftp.settings")
|
@patch("app.tasks.upload_to_ftp.settings")
|
||||||
@patch("builtins.open", create=True)
|
@patch("builtins.open", create=True)
|
||||||
def test_uploads_file_with_ftps(
|
def test_uploads_file_with_ftps(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename):
|
||||||
self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
|
|
||||||
):
|
|
||||||
"""Test uploads file using FTPS (FTP with TLS)."""
|
"""Test uploads file using FTPS (FTP with TLS)."""
|
||||||
mock_exists.return_value = True
|
mock_exists.return_value = True
|
||||||
mock_basename.return_value = "test.pdf"
|
mock_basename.return_value = "test.pdf"
|
||||||
|
|||||||
@@ -305,9 +305,9 @@ class TestWebDAVIntegration:
|
|||||||
response = requests.get(file_url, auth=(webdav_server["username"], webdav_server["password"]), timeout=10)
|
response = requests.get(file_url, auth=(webdav_server["username"], webdav_server["password"]), timeout=10)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
assert (
|
assert len(response.content) == 1024 * 1024, (
|
||||||
len(response.content) == 1024 * 1024
|
f"File size mismatch: expected 1MB, got {len(response.content)} bytes"
|
||||||
), f"File size mismatch: expected 1MB, got {len(response.content)} bytes"
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
|
|||||||
Reference in New Issue
Block a user