diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8d8d347d..9e6f180b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,12 +25,38 @@ env: 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: name: Tests runs-on: ubuntu-latest + needs: [lint] # Wait for lint to pass before running tests services: redis: image: redis:7 @@ -91,30 +117,10 @@ jobs: junit.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: name: Mypy runs-on: ubuntu-latest + needs: [lint] # Wait for lint to pass before running type checks steps: - name: Checkout Code uses: actions/checkout@v4 @@ -133,7 +139,7 @@ jobs: 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: @@ -196,7 +202,7 @@ jobs: 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: diff --git a/.github/workflows/ruff-auto-fix.yml b/.github/workflows/ruff-auto-fix.yml new file mode 100644 index 00000000..a834b523 --- /dev/null +++ b/.github/workflows/ruff-auto-fix.yml @@ -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] " + 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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c92cb148..7d51243a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,13 +151,13 @@ DocuElevate uses [semantic-release](https://github.com/semantic-release/semantic 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 +- [ ] Pre-commit hooks installed and passing (see below) - [ ] Tests added/updated for new functionality - [ ] Documentation updated if user-facing changes - [ ] No manual edits to `VERSION` or `CHANGELOG.md` - [ ] All tests pass locally -- [ ] Pre-commit hooks pass - [ ] Security scan passes (if applicable) ## Development Environment @@ -176,8 +176,33 @@ source venv/bin/activate # On Windows: venv\Scripts\activate # Install dependencies pip install -r requirements.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 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 -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 | |--------|--------|--------------------------------------| | `test` | pytest | Unit/integration tests + coverage | -| `flake8`| flake8 | PEP 8 style | -| `black` | black | Code formatting | | `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 -We use: -- Black for Python code formatting (line length: 120) -- Flake8 for linting -- isort for import sorting (Black-compatible profile) +DocuElevate uses **Ruff** for all Python code quality checks: + +- **Linting** - PEP 8 style, code quality, and security checks +- **Formatting** - Consistent code formatting (120 character line length) +- **Import sorting** - Organized imports ```bash -# Format code -black . +# Check for linting issues +ruff check app/ tests/ -# Check linting -flake8 +# Auto-fix linting issues +ruff check app/ tests/ --fix -# Sort imports -isort . +# Check formatting +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 ``` diff --git a/tests/test_api_azure_comprehensive.py b/tests/test_api_azure_comprehensive.py index 42ff8505..dd52f3c5 100644 --- a/tests/test_api_azure_comprehensive.py +++ b/tests/test_api_azure_comprehensive.py @@ -110,7 +110,9 @@ class TestAzureTestConnectionEndpoint: @patch("app.api.azure.AzureKeyCredential") @patch("app.api.azure.settings") @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.""" from app.api.azure import test_azure_connection @@ -196,7 +198,9 @@ class TestAzureTestConnectionEndpoint: @patch("app.api.azure.AzureKeyCredential") @patch("app.api.azure.settings") @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.""" from app.api.azure import test_azure_connection @@ -325,7 +329,9 @@ class TestAzureTestConnectionEndpoint: @patch("app.api.azure.AzureKeyCredential") @patch("app.api.azure.settings") @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.""" from app.api.azure import test_azure_connection diff --git a/tests/test_external_integrations.py b/tests/test_external_integrations.py index 30c0166d..5fab2016 100644 --- a/tests/test_external_integrations.py +++ b/tests/test_external_integrations.py @@ -244,9 +244,9 @@ class TestAzureDocumentIntelligenceIntegration: assert len(result.content) > 10, f"OCR text too short: {result.content[:50]}" # Verify the generated text is recognizable - assert ( - "Acme" in result.content or "Invoice" in result.content - ), f"OCR text does not contain expected keywords: {result.content[:200]}" + assert "Acme" in result.content or "Invoice" in result.content, ( + f"OCR text does not contain expected keywords: {result.content[:200]}" + ) # Retrieve the searchable PDF output operation_id = poller.details["operation_id"] @@ -602,9 +602,9 @@ class TestFullOCRMetadataPipeline: # The generated invoice should be classified reasonably doc_type = metadata["document_type"].lower() - assert any( - kw in doc_type for kw in ("invoice", "rechnung", "bill") - ), f"Unexpected document_type: {metadata['document_type']}" + assert any(kw in doc_type for kw in ("invoice", "rechnung", "bill")), ( + f"Unexpected document_type: {metadata['document_type']}" + ) finally: os.unlink(pdf_path) diff --git a/tests/test_file_splitting.py b/tests/test_file_splitting.py index f88460da..9682e6df 100644 --- a/tests/test_file_splitting.py +++ b/tests/test_file_splitting.py @@ -83,9 +83,9 @@ class TestSplitPdfBySize: # that can cause files to exceed the target size by ~20-50%. We allow 1.5x (50%) margin. PDF_OVERHEAD_MULTIPLIER = 1.5 for split_file in split_files: - assert ( - os.path.getsize(split_file) <= max_size * PDF_OVERHEAD_MULTIPLIER - ), f"Split file {split_file} should respect size limit (with PDF overhead allowance)" + assert os.path.getsize(split_file) <= max_size * PDF_OVERHEAD_MULTIPLIER, ( + f"Split file {split_file} should respect size limit (with PDF overhead allowance)" + ) # Cleanup split files for split_file in split_files: diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py index 4a22f306..93dc711a 100644 --- a/tests/test_security_headers.py +++ b/tests/test_security_headers.py @@ -144,9 +144,9 @@ def test_x_frame_options_valid_value(client): x_frame_value = response.headers["X-Frame-Options"] valid_values = ["DENY", "SAMEORIGIN"] # 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( - "ALLOW-FROM" - ), f"Invalid X-Frame-Options value: {x_frame_value}" + assert x_frame_value in valid_values or x_frame_value.startswith("ALLOW-FROM"), ( + f"Invalid X-Frame-Options value: {x_frame_value}" + ) @pytest.mark.integration diff --git a/tests/test_upload_ftp_additional.py b/tests/test_upload_ftp_additional.py index 92102078..cdcafdf3 100644 --- a/tests/test_upload_ftp_additional.py +++ b/tests/test_upload_ftp_additional.py @@ -24,9 +24,7 @@ class TestUploadToFtp: @patch("app.tasks.upload_to_ftp.os.path.exists") @patch("app.tasks.upload_to_ftp.settings") @patch("builtins.open", create=True) - def test_uploads_file_with_ftps( - self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename - ): + def test_uploads_file_with_ftps(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename): """Test uploads file using FTPS (FTP with TLS).""" mock_exists.return_value = True mock_basename.return_value = "test.pdf" diff --git a/tests/test_upload_webdav_integration.py b/tests/test_upload_webdav_integration.py index e0dfc55f..dd7fa627 100644 --- a/tests/test_upload_webdav_integration.py +++ b/tests/test_upload_webdav_integration.py @@ -305,9 +305,9 @@ class TestWebDAVIntegration: response = requests.get(file_url, auth=(webdav_server["username"], webdav_server["password"]), timeout=10) assert response.status_code == 200 - assert ( - len(response.content) == 1024 * 1024 - ), f"File size mismatch: expected 1MB, got {len(response.content)} bytes" + assert len(response.content) == 1024 * 1024, ( + f"File size mismatch: expected 1MB, got {len(response.content)} bytes" + ) @pytest.mark.integration