gmakstutis/openrag
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add branch protection guide and PR CI workflow

Browse source 
Introduces .github/BRANCH_PROTECTION_GUIDE.md to document branch protection and CI requirements. Adds .github/workflows/pr-checks.yml for PR validation, including unit tests and code quality checks. Updates TESTING_QUICKSTART.md and tests/TEST_SUMMARY.md to use consistent status check notation and clarify test suite details.
This commit is contained in:
Edwin Jose 2025-10-07 04:46:54 -04:00
parent 3881c50ad5
commit d66a6284cd
4 changed files with 469 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

291
.github/BRANCH_PROTECTION_GUIDE.md vendored Normal file
View file

@ -0,0 +1,291 @@
# Branch Protection Configuration Guide
This guide explains how to configure branch protection rules for the `main` branch to ensure all tests pass before merging.
## Quick Setup
### 1. Navigate to Branch Protection Settings
1. Go to your repository on GitHub
2. Click **Settings** → **Branches**
3. Click **Add rule** or **Edit** for the `main` branch
### 2. Configure Required Settings
Enable the following options:
#### Required Status Checks
[x] **Require status checks to pass before merging**
- [x] Require branches to be up to date before merging
- Required checks:
- `Test Validation` (from pr-checks.yml)
- `Run Tests` (from tests.yml)
- `test-validation` (job name)
#### Additional Protections (Recommended)
- [x] Require a pull request before merging
- Required approvals: 1 (recommended)
- Dismiss stale reviews when new commits are pushed
- [x] Require conversation resolution before merging
- [x] Do not allow bypassing the above settings
## CI Workflows Overview
### 1. `tests.yml` - Main Test Workflow
**Triggers:**
- Push to `main` branch
- Pull requests to `main` branch
**Jobs:**
- `test`: Runs unit tests (required to pass)
- `integration-test`: Runs integration tests with OpenSearch (optional)
- `lint`: Code quality checks (optional)
- `test-summary`: Aggregates results
**Required for merge:** [x] Yes (test job must pass)
### 2. `pr-checks.yml` - PR-specific Validation
**Triggers:**
- Pull request opened/updated to `main`
**Jobs:**
- `test-validation`: Strict unit test validation
- `code-quality`: Linting and code quality
- `pr-validation`: Final validation check
**Required for merge:** [x] Yes (test-validation job must pass)
## What Tests Must Pass?
### Required [x]
- All unit tests (77+ tests)
- Runtime: ~2 seconds
- No external dependencies
### Optional [INFO]
- Integration tests (require OpenSearch)
- Linting checks
- Code quality checks
## Test Commands Run in CI
```bash
# Unit tests (REQUIRED - must pass for merge)
uv run pytest tests/ -v \
-m "not requires_opensearch and not requires_langflow" \
--cov=src \
--cov-report=xml \
--cov-fail-under=1
# Integration tests (OPTIONAL - informational)
uv run pytest tests/ -v \
-m "integration and requires_opensearch" \
--cov=src \
--cov-report=xml
```
## PR Workflow
### Contributor Flow
1. **Create feature branch**
```bash
git checkout -b feature/my-feature
```
2. **Make changes and run tests locally**
```bash
make test-unit
```
3. **Push changes**
```bash
git push origin feature/my-feature
```
4. **Create Pull Request**
- CI automatically runs
- Status checks appear on PR
5. **Review CI Results**
- [x] Green checks = ready to merge
- [FAIL] Red checks = fix issues
6. **Merge when tests pass**
- All required checks must be green
- PR can be merged to main
### Status Check Results
#### [x] Success
```
[x] Test Validation
[x] Run Tests
[x] All required tests passed
```
**Action:** PR can be merged
#### [FAIL] Failure
```
[FAIL] Test Validation
[FAIL] Some tests failed
```
**Action:** Fix failing tests and push changes
## Local Testing Before Push
Ensure tests pass locally before pushing:
```bash
# Quick check (recommended)
make test-unit
# Full test suite
make test
# With coverage
make test-coverage
# Specific tests
make test-api
make test-service
```
## Troubleshooting CI Failures
### Tests fail in CI but pass locally
1. **Check Python version**
```bash
python --version # Should be 3.13
```
2. **Ensure dependencies are in sync**
```bash
uv sync --extra dev
```
3. **Run tests in CI mode**
```bash
uv run pytest tests/ -v -m "not requires_opensearch and not requires_langflow"
```
### Permission errors in CI
- Ensure workflow has necessary permissions
- Check GitHub Actions settings
### Flaky tests
- Integration tests may be flaky (that's why they're optional)
- Unit tests should be stable
- Report flaky tests as issues
## Bypassing Branch Protection (Emergency Only)
[WARN] **Not recommended for normal workflow**
If you're a repository admin and need to bypass:
1. Go to Settings → Branches
2. Temporarily uncheck protection rules
3. Merge PR
4. **Immediately re-enable protection**
[WARN] **Always fix tests instead of bypassing!**
## Configuration Examples
### Minimal Configuration (Recommended)
```yaml
Branch Protection Rules for main:
☑ Require status checks to pass before merging
☑ Require branches to be up to date
Required checks:
- test-validation
- Run Tests
☑ Require a pull request before merging
```
### Strict Configuration (Team Environment)
```yaml
Branch Protection Rules for main:
☑ Require status checks to pass before merging
☑ Require branches to be up to date
Required checks:
- test-validation
- Run Tests
- code-quality
☑ Require a pull request before merging
Number of approvals: 2
☑ Dismiss stale reviews
☑ Require conversation resolution
☑ Require signed commits
☑ Include administrators
```
## Monitoring Test Health
### View Test Results
1. **In Pull Request:**
- Check "Checks" tab
- View detailed logs
- See test output
2. **In Actions Tab:**
- View all workflow runs
- Check historical pass rates
- Monitor performance
### Test Metrics
- **Success Rate:** Should be 100% for unit tests
- **Runtime:** ~2 seconds for unit tests
- **Coverage:** Growing toward 70%+
## FAQ
**Q: Can I merge if integration tests fail?**
A: Yes, integration tests are optional. Only unit tests are required.
**Q: How do I skip CI for a commit?**
A: You can't skip required checks. All PRs must pass tests.
**Q: What if tests are flaky?**
A: Fix the flaky tests. Unit tests should never be flaky.
**Q: Can I run CI locally?**
A: Yes, use `make test-unit` to run the same tests CI runs.
**Q: How do I add new required checks?**
A: Update branch protection settings and add job name to required checks.
## Support
- Tests failing? Check test logs in GitHub Actions
- Need help? See `tests/README.md` for testing guide
- Issues? Open a GitHub issue
## Summary
[x] **Required for merge to main:**
1. All unit tests pass (77+ tests)
2. No test failures
3. Code builds successfully
[FAIL] **Blocks merge:**
- Any unit test failure
- Build failures
- Missing required checks
**Optional:**
- Integration tests
- Linting checks
- Code coverage improvements
Enable branch protection to enforce these rules automatically!

122
.github/workflows/pr-checks.yml vendored Normal file
View file

@ -0,0 +1,122 @@
name: PR Checks
on:
pull_request:
branches: [main]
types: [opened, synchronize, reopened]
jobs:
test-validation:
name: Test Validation
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Set up Python 3.13
uses: actions/setup-python@v5
with:
python-version: "3.13"
- name: Install uv
uses: astral-sh/setup-uv@v4
with:
enable-cache: true
- name: Install dependencies
run: |
uv sync --extra dev
- name: Run unit tests with strict requirements
id: unit_tests
run: |
uv run pytest tests/ -v \
-m "not requires_opensearch and not requires_langflow" \
--cov=src \
--cov-report=term-missing \
--cov-report=xml \
--cov-fail-under=1 \
--tb=short
- name: Test Summary
if: always()
run: |
if [ "${{ steps.unit_tests.outcome }}" == "success" ]; then
echo "✅ All unit tests passed"
echo "PR is ready for review"
else
echo "❌ Tests failed"
echo "Please fix failing tests before merging"
exit 1
fi
- name: Comment PR with results
if: always() && github.event_name == 'pull_request'
uses: actions/github-script@v7
with:
script: |
const outcome = '${{ steps.unit_tests.outcome }}';
const icon = outcome === 'success' ? '✅' : '❌';
const status = outcome === 'success' ? 'passed' : 'failed';
const body = `## ${icon} Test Results
Unit tests ${status}!
- Python Version: 3.13
- Tests Run: Unit tests (excluding integration)
- Status: ${status}
${outcome === 'success' ? '✅ Ready for merge' : '❌ Please fix failing tests'}
`;
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: body
});
code-quality:
name: Code Quality
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Set up Python 3.13
uses: actions/setup-python@v5
with:
python-version: "3.13"
- name: Install uv
uses: astral-sh/setup-uv@v4
- name: Install dependencies
run: |
uv sync --extra dev
uv pip install ruff
- name: Run ruff linter
run: |
uv run ruff check src/ tests/ --output-format=github
continue-on-error: true
pr-validation:
name: PR Validation
runs-on: ubuntu-latest
needs: [test-validation, code-quality]
if: always()
steps:
- name: Check all jobs
run: |
if [ "${{ needs.test-validation.result }}" != "success" ]; then
echo "❌ Test validation failed"
exit 1
fi
echo "✅ All PR checks passed"
echo "This PR is ready to be merged to main"

8
TESTING_QUICKSTART.md
View file

@ -39,10 +39,10 @@ tests/
## Current Status
✅ **77 passing unit tests**
✅ **~2 second runtime**
✅ **No mocks - using real fixtures**
✅ **Ready for CI/CD**
[x] **77 passing unit tests**
[x] **~2 second runtime**
[x] **No mocks - using real fixtures**
[x] **Ready for CI/CD**
## Quick Commands

104
tests/TEST_SUMMARY.md
View file

@ -1,53 +1,53 @@
# OpenRAG Backend Test Suite Summary
## ✅ Implementation Complete
## [x] Implementation Complete
### Test Coverage Created
#### 1. **Utils Tests** (41 tests)
- ✅ `test_embeddings.py` - Embedding dimension handling and index body creation (15 tests)
- ✅ `test_hash_utils.py` - Hashing utilities for document IDs (26 tests)
- [x] `test_embeddings.py` - Embedding dimension handling and index body creation (15 tests)
- [x] `test_hash_utils.py` - Hashing utilities for document IDs (26 tests)
#### 2. **API Tests** (15 tests)
- ✅ `test_health.py` - Health check and basic API functionality (5 tests)
- ✅ `test_documents.py` - Document API endpoints (5 tests)
- ✅ `test_search.py` - Search API endpoints (5 tests)
- [x] `test_health.py` - Health check and basic API functionality (5 tests)
- [x] `test_documents.py` - Document API endpoints (5 tests)
- [x] `test_search.py` - Search API endpoints (5 tests)
#### 3. **Service Tests** (8 tests)
- ✅ `test_document_service.py` - Document service operations (4 tests)
- ✅ `test_search_service.py` - Search service operations (4 tests)
- [x] `test_document_service.py` - Document service operations (4 tests)
- [x] `test_search_service.py` - Search service operations (4 tests)
#### 4. **Connector Tests** (8 tests)
- ✅ `test_base.py` - Connector initialization and configuration (8 tests)
- [x] `test_base.py` - Connector initialization and configuration (8 tests)
#### 5. **Config Tests** (5 tests)
- ✅ `test_settings.py` - Configuration and environment variables (5 tests)
- [x] `test_settings.py` - Configuration and environment variables (5 tests)
### Test Infrastructure
#### Pytest Configuration
- ✅ `pytest.ini` - Test discovery, markers, coverage settings
- ✅ `conftest.py` - Root fixtures and configuration
- ✅ Coverage reporting (HTML, XML, terminal)
- [x] `pytest.ini` - Test discovery, markers, coverage settings
- [x] `conftest.py` - Root fixtures and configuration
- [x] Coverage reporting (HTML, XML, terminal)
#### Fixture System (No Mocks!)
- ✅ `fixtures/opensearch_fixtures.py` - Real OpenSearch test fixtures
- ✅ `fixtures/service_fixtures.py` - Service instance fixtures
- ✅ `fixtures/connector_fixtures.py` - Connector fixtures
- ✅ `fixtures/app_fixtures.py` - Application-level fixtures
- [x] `fixtures/opensearch_fixtures.py` - Real OpenSearch test fixtures
- [x] `fixtures/service_fixtures.py` - Service instance fixtures
- [x] `fixtures/connector_fixtures.py` - Connector fixtures
- [x] `fixtures/app_fixtures.py` - Application-level fixtures
#### Makefile Commands
- ✅ `make test` - Run all tests
- ✅ `make test-unit` - Unit tests only
- ✅ `make test-integration` - Integration tests only
- ✅ `make test-api` - API tests
- ✅ `make test-service` - Service tests
- ✅ `make test-connector` - Connector tests
- ✅ `make test-coverage` - Tests with coverage report
- ✅ `make test-verbose` - Verbose output
- ✅ `make test-failed` - Re-run failed tests
- ✅ `make test-quick` - Quick unit tests
- ✅ `make test-specific TEST=path` - Run specific test
- [x] `make test` - Run all tests
- [x] `make test-unit` - Unit tests only
- [x] `make test-integration` - Integration tests only
- [x] `make test-api` - API tests
- [x] `make test-service` - Service tests
- [x] `make test-connector` - Connector tests
- [x] `make test-coverage` - Tests with coverage report
- [x] `make test-verbose` - Verbose output
- [x] `make test-failed` - Re-run failed tests
- [x] `make test-quick` - Quick unit tests
- [x] `make test-specific TEST=path` - Run specific test
### Dependencies Added
```toml
@ -60,16 +60,16 @@ dev = [
]
```
## 📊 Test Results
## Test Results
```
Total Tests: 97 (77 unit, 20 integration)
Passing: 77/77 unit tests (100%)
Runtime: ~2 seconds (unit tests)
Status: ✅ ALL PASSING
Status: [x] ALL PASSING
```
## 🎯 Test Categories
## Test Categories
Tests are organized with pytest markers:
@ -81,7 +81,7 @@ Tests are organized with pytest markers:
- `@pytest.mark.requires_opensearch` - Requires OpenSearch
- `@pytest.mark.requires_langflow` - Requires Langflow
## 📁 Test Structure
## Test Structure
```
tests/
@ -102,7 +102,7 @@ tests/
└── app_fixtures.py
```
## 🚀 Quick Start
## Quick Start
```bash
# Install test dependencies
@ -118,7 +118,7 @@ make test-coverage
make test-api
```
## 🧪 Key Features
## Key Features
### 1. Fixture-Based Testing (No Mocks!)
- Real OpenSearch clients for integration tests
@ -148,19 +148,19 @@ make test-api
- Coverage enforcement
- Configurable markers
## 📈 Coverage Goals
## Coverage Goals
Current: Growing from 1.44% (utils only)
Target: 70%+ overall coverage
Tested modules:
- ✅ utils/embeddings.py - 100%
- ✅ utils/hash_utils.py - 88%
- [x] utils/embeddings.py - 100%
- [x] utils/hash_utils.py - 88%
- ⏳ services/* - To be expanded
- ⏳ api/* - To be expanded
- ⏳ connectors/* - To be expanded
## 🔧 Integration Tests
## Integration Tests
Integration tests require external services:
@ -175,7 +175,7 @@ make test-integration
pytest -m "not requires_opensearch and not requires_langflow"
```
## 📝 Sample Test
## Sample Test
```python
import pytest
@ -187,19 +187,19 @@ class TestEmbeddingDimensions:
assert get_embedding_dimensions("text-embedding-ada-002") > 0
```
## 🎓 Best Practices Implemented
## Best Practices Implemented
1. ✅ Use fixtures instead of mocks
2. ✅ Organize tests by category with markers
3. ✅ Keep unit tests fast
4. ✅ Proper resource cleanup
5. ✅ Test one thing per test
6. ✅ Descriptive test names
7. ✅ Follow AAA pattern (Arrange, Act, Assert)
8. ✅ Independent tests
9. ✅ Clear documentation
1. [x] Use fixtures instead of mocks
2. [x] Organize tests by category with markers
3. [x] Keep unit tests fast
4. [x] Proper resource cleanup
5. [x] Test one thing per test
6. [x] Descriptive test names
7. [x] Follow AAA pattern (Arrange, Act, Assert)
8. [x] Independent tests
9. [x] Clear documentation
## 🔄 Next Steps
## Next Steps
To expand test coverage:
@ -210,13 +210,13 @@ To expand test coverage:
5. Add flow management tests
6. Increase coverage to 70%+
## 📚 Documentation
## Documentation
- `tests/README.md` - Comprehensive testing guide
- `pytest.ini` - Configuration reference
- `Makefile` - Available commands
## ✨ Highlights
## Highlights
- **No mocks used** - Real fixtures for better integration testing
- **77 passing tests** - All unit tests green