Skip to content

Conversation

@vtnphan
Copy link
Collaborator

@vtnphan vtnphan commented Dec 17, 2025

Pull Request

SBP-232 Enable BindFlow Workflow Execution from Input Workflow Page

Summary

This PR introduces comprehensive testing infrastructure and automated quality checks to the SBP Portal Backend, establishing a solid foundation for maintainability and code quality. The changes include 93% test coverage (exceeding the 90% requirement), modern Python 3.10+ type hints, and CI/CD pipelines for automated testing and linting.

Changes

Testing Infrastructure (70 tests, 93.14% coverage)

  • Added comprehensive test suite covering all service layers:
    • test_main.py - FastAPI app initialization, CORS, health endpoints (8 tests)
    • test_schemas.py - Pydantic schema validation and transformations (17 tests)
    • test_routes_workflows.py - API endpoint integration tests (12 tests)
    • test_services_seqera.py - Seqera Platform API integration (11 tests)
    • test_services_datasets.py - Dataset creation/upload service (20 tests)
    • test_additional_coverage.py - Edge case coverage (2 tests)
  • Test configuration:
    • conftest.py - Shared fixtures, mock clients, environment setup
    • pytest.ini - Pytest configuration with async mode
    • requirements-dev.txt - Development dependencies (pytest, coverage, mocking)

CI/CD Workflows

  • test-coverage.yml:
    • Runs tests on Python 3.10, 3.11, 3.12
    • Enforces 90% coverage threshold (currently at 93.14%)
    • Uploads coverage reports to Codecov
    • Generates coverage summary with badges
  • lint.yml:
    • Runs ruff (linter), black (formatter), mypy (type checker)
    • Ensures code quality and consistency
  • release.yml & release-simple.yml:
    • Automated release creation on version tags
    • Optional changelog generation

Project Configuration

  • pyproject.toml:
    • Coverage configuration (90% threshold, branch coverage)
    • Ruff linting rules and exclusions
    • Black formatting settings (line-length 100, Python 3.10-3.12)
    • MyPy type checking configuration
  • .pre-commit-config.yaml:
    • Pre-commit hooks for ruff, black, and mypy
    • Automated code quality checks before commits
  • README.md:
    • Added CI/CD badges (Coverage, Lint, Tests)
    • Updated Python version requirement to >=3.10
    • Documented testing and linting procedures

Coverage by Module

  • main.py: 93.94%
  • workflows.py: 97.92%
  • datasets.py: 94.92%
  • seqera.py: 100%
  • workflows.py: 75.38%
  • Overall: 93.14%

How to Test

Running Tests Locally

# Install development dependencies
pip install -r requirements-dev.txt

# Run all tests with coverage
pytest --cov=app --cov-report=term-missing

# Run specific test file
pytest tests/test_services_seqera.py -v

# Generate HTML coverage report
pytest --cov=app --cov-report=html
open htmlcov/index.html

Running Linters

# Run all linters
ruff check app tests
black --check app tests
mypy app --ignore-missing-imports

# Auto-fix linting issues
ruff check app tests --fix
black app tests

Pre-commit Hooks

# Install pre-commit hooks
pip install pre-commit
pre-commit install

# Run manually
pre-commit run --all-files

CI/CD Validation

  • Push to branch triggers:
    • Test coverage workflow (Python 3.10, 3.11, 3.12)
    • Lint workflow (ruff, black, mypy)
  • Both workflows must pass before merge

Type of change

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Checklist

  • I have added tests that prove my fix is effective or that my feature works (70 tests, 93% coverage)
  • I have added or updated documentation where necessary (README.md, inline docstrings)
  • I have run linting and unit tests locally (all passing)
  • The code follows the project's style guidelines (ruff + black + mypy compliant)

@vtnphan vtnphan marked this pull request as ready for review December 18, 2025 03:42
@vtnphan vtnphan requested a review from Copilot December 18, 2025 03:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces comprehensive testing infrastructure and linting workflows to the SBP Portal Backend. The changes modernize the codebase with Python 3.10+ type hints, add extensive test coverage across all service layers, and establish CI/CD pipelines for automated quality checks.

Key Changes:

  • Added comprehensive test suite with 90% coverage requirement across services, routes, schemas, and main application
  • Introduced GitHub Actions workflows for linting (ruff, black, mypy) and test coverage with Codecov integration
  • Modernized type hints from Optional[X], Dict, List to Python 3.10+ syntax (X | None, dict, list)

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
tests/conftest.py Adds shared test fixtures including mock clients, sample data, and environment setup for all tests
tests/test_main.py Tests FastAPI app creation, health endpoint, CORS configuration, and router inclusion
tests/test_schemas.py Validates Pydantic schema validation, field requirements, and data transformations
tests/test_services_seqera.py Tests Seqera Platform integration including workflow launches, error handling, and API interactions
tests/test_services_datasets.py Tests dataset creation, upload, CSV conversion, and form data handling
tests/test_routes_workflows.py Tests workflow API endpoints including launch, cancel, list, logs, and details
tests/test_additional_coverage.py Provides additional tests to meet coverage thresholds for edge cases
tests/init.py Marks the tests directory as a Python package
pytest.ini Configures pytest with async mode, test paths, and marker definitions
pyproject.toml Defines project metadata and configuration for coverage, ruff, black, and mypy
requirements-dev.txt Specifies development dependencies for testing and linting
.pre-commit-config.yaml Configures pre-commit hooks for automated code quality checks
.github/workflows/lint.yml CI workflow for running ruff, black, and mypy linters
.github/workflows/test-coverage.yml CI workflow for running tests with coverage reporting to Codecov
.github/PULL_REQUEST_TEMPLATE.md Template to standardize pull request descriptions
app/main.py Updates type hints to modern Python 3.10+ syntax
app/services/seqera.py Modernizes type hints and improves code formatting consistency
app/services/datasets.py Updates type hints to Python 3.10+ syntax and improves readability
app/schemas/workflows.py Converts all type hints to modern syntax (pipe union notation)
app/routes/workflows.py Updates type hints and adds proper exception chaining with from exc
README.md Adds CI badges, updates Python version requirement, and documents testing/linting procedures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vtnphan vtnphan requested review from amandazhuyilan and marius-mather and removed request for amandazhuyilan and marius-mather December 18, 2025 04:13
Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

The tests and CI checks look great, just a few suggestions on project and testing setup that will hopefully make things easier down the line.

You don't have to use the testing/mocking libraries like respx or polyfactory - just wanted to point them out as I find them useful for my own testing.

Comment on lines +5 to +7
branches: [main, workflows]
pull_request:
branches: [main, workflows]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is right - you generally only need CI to run on pushes or pull requests to main

run: |
python -m pip install --upgrade pip
python -m pip install -r requirements.txt
python -m pip install ruff black mypy
Copy link
Collaborator

Choose a reason for hiding this comment

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

These types of requirements should be in requirements-dev.txt so you can run the same checks locally (or in pyproject.toml as discussed below)

@@ -0,0 +1,7 @@
# Development dependencies
pytest==8.3.3
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd encourage you to look at uv for managing dependencies, instead of just using pip and requirements.txt. You can have dev dependencies as a dependency group.

A couple of tips:

  • Add any dependencies required for dev/testing with uv add --dev pytest
  • Don't pin exact versions, allow minor version upgrades by using version specifiers like pytest ~= 8.3. I find that if you pin dependencies to exact versions, it just creates a lot of work later when you need to update something, as you have to figure out which other versions need to be updated. Also, you want to regularly update packages for security updates.



@pytest.fixture
def mock_httpx_response():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have a look at the respx library for mocking httpx - it will handle a lot of this for you



@pytest.fixture
def sample_form_data():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also have a look at polyfactory for generating mock responses automatically from schemas - can save time if you need to specify some fields in a schema for a test but don't care about the rest.

from app.services.datasets import DatasetUploadResult


class TestUploadDataset:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I generally don't use test classes when using pytest - you can just use individual test functions and use fixtures for any setup that's required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants