chore(ci): improve CI/CD workflows and add dependabot #533
Conversation
Workflows: - Add test.yml for running backend pytest and frontend tests - Improve docker-publish.yml with caching and better tagging - Add proper OCI labels via metadata-action DevOps: - Add dependabot.yml for automated dependency updates - Update docker-compose.yml with test service configuration - Improve Dockerfile with better caching layers - Expand .gitignore with common patterns
|
@ankitpasayat is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 601489b..94c5fdb
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (6)
• .github/dependabot.yml
• .github/workflows/docker-publish.yml
• .github/workflows/test.yml
• .gitignore
• docker-compose.yml
• surfsense_backend/Dockerfile
There was a problem hiding this comment.
Pull request overview
This PR introduces CI/CD infrastructure improvements including automated testing workflows, enhanced Docker publishing, and dependency management automation. However, the changes reference test infrastructure that doesn't yet exist in the repository, which will cause several workflow and service failures.
Key Changes:
- Added GitHub Actions test workflow for backend (pytest) and frontend tests
- Enhanced docker-publish.yml with proper metadata, caching, and conditional triggers
- Added Dependabot configuration for automated dependency updates across Python, npm, Docker, and GitHub Actions
- Expanded .gitignore with comprehensive Python, IDE, and development patterns
- Updated Dockerfile to include test dependencies
- Added backend-test service to docker-compose.yml
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/test.yml |
New workflow for running automated tests on backend and frontend with path filtering and caching |
.github/workflows/docker-publish.yml |
Enhanced with workflow_dispatch inputs, GHA caching, proper OCI metadata, and conditional build triggers |
.github/dependabot.yml |
New configuration for automated weekly dependency updates across all package ecosystems |
surfsense_backend/Dockerfile |
Added pytest and coverage dependencies for testing support |
docker-compose.yml |
Removed deprecated version field, added test volumes to backend service, and new backend-test profile service |
.gitignore |
Comprehensive additions for Python artifacts, virtual environments, IDE files, coverage reports, and temporary files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - ./surfsense_backend/tests:/app/tests | ||
| - ./surfsense_backend/pytest.ini:/app/pytest.ini |
There was a problem hiding this comment.
The backend-test service mounts ./surfsense_backend/tests:/app/tests and ./surfsense_backend/pytest.ini:/app/pytest.ini, but these files/directories don't exist in the repository. This will cause the test service to fail when invoked. Remove these volume mounts until the test infrastructure is actually created.
| - ./surfsense_backend/tests:/app/tests | |
| - ./surfsense_backend/pytest.ini:/app/pytest.ini |
| docker compose build backend | ||
| # Run tests (pytest is baked into the image) | ||
| docker compose run --rm -e TESTING=true backend pytest tests/ -v --tb=short --cov=app --cov-report=xml |
There was a problem hiding this comment.
The test workflow attempts to run pytest tests/ but the tests/ directory doesn't exist in surfsense_backend/. This will cause the test job to fail. Either create the tests directory with actual tests, or remove/skip this step until tests are implemented.
| docker compose run --rm -e TESTING=true backend pytest tests/ -v --tb=short --cov=app --cov-report=xml | |
| # docker compose run --rm -e TESTING=true backend pytest tests/ -v --tb=short --cov=app --cov-report=xml | |
| # (Tests directory does not exist yet; re-enable this line when tests are implemented) |
| - name: Cache Docker layers | ||
| if: steps.backend-changes.outputs.backend == 'true' | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: /tmp/.buildx-cache | ||
| key: ${{ runner.os }}-buildx-${{ hashFiles('surfsense_backend/Dockerfile', 'surfsense_backend/pyproject.toml') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-buildx- |
There was a problem hiding this comment.
The Docker layer cache path /tmp/.buildx-cache is configured but never used in the build process. Since docker compose build doesn't utilize this cache configuration, this cache step will have no effect. Either use docker buildx build with proper cache configuration or remove this unused cache step.
| - name: Cache Docker layers | |
| if: steps.backend-changes.outputs.backend == 'true' | |
| uses: actions/cache@v4 | |
| with: | |
| path: /tmp/.buildx-cache | |
| key: ${{ runner.os }}-buildx-${{ hashFiles('surfsense_backend/Dockerfile', 'surfsense_backend/pyproject.toml') }} | |
| restore-keys: | | |
| ${{ runner.os }}-buildx- |
| with: | ||
| images: ${{ env.REGISTRY }}/${{ github.repository_owner }}/surfsense_backend | ||
| tags: | | ||
| type=sha,prefix= |
There was a problem hiding this comment.
The type=sha,prefix= configuration creates a tag with just the SHA and no prefix (e.g., abc123). This is unconventional and may conflict with semantic version tags. Consider using type=sha,prefix=sha- to create tags like sha-abc123 for better clarity and to avoid potential conflicts.
| type=sha,prefix= | |
| type=sha,prefix=sha- |
| with: | ||
| images: ${{ env.REGISTRY }}/${{ github.repository_owner }}/surfsense_web | ||
| tags: | | ||
| type=sha,prefix= |
There was a problem hiding this comment.
The type=sha,prefix= configuration creates a tag with just the SHA and no prefix (e.g., abc123). This is unconventional and may conflict with semantic version tags. Consider using type=sha,prefix=sha- to create tags like sha-abc123 for better clarity and to avoid potential conflicts.
| type=sha,prefix= | |
| type=sha,prefix=sha- |
| if: | | ||
| github.event_name == 'release' || | ||
| (github.event_name == 'workflow_dispatch' && inputs.push_frontend) || | ||
| github.event_name == 'push' |
There was a problem hiding this comment.
The frontend build job includes a push trigger but the backend build job doesn't. This creates an inconsistency - when Dockerfiles are modified via push to main, only the frontend will be built. Add github.event_name == 'push' to the backend job's condition to ensure both images are built when their Dockerfiles change.
| with: | ||
| file: surfsense_backend/coverage.xml | ||
| flags: backend | ||
| fail_ci_if_error: false |
There was a problem hiding this comment.
The codecov upload step doesn't include a token parameter. While this may work for public repositories, it's a best practice to explicitly configure the CODECOV_TOKEN secret for reliability. Consider adding token: ${{ secrets.CODECOV_TOKEN }} to ensure consistent uploads.
| fail_ci_if_error: false | |
| fail_ci_if_error: false | |
| token: ${{ secrets.CODECOV_TOKEN }} |
| - name: Run tests | ||
| if: steps.frontend-changes.outputs.frontend == 'true' | ||
| working-directory: surfsense_web | ||
| run: pnpm test |
There was a problem hiding this comment.
The frontend tests are configured to run pnpm test, but this script is not defined in surfsense_web/package.json. This will cause the test job to fail. Either add a test script to package.json or remove/skip this test step if tests don't exist yet.
| - name: Run tests | |
| if: steps.frontend-changes.outputs.frontend == 'true' | |
| working-directory: surfsense_web | |
| run: pnpm test | |
| # - name: Run tests | |
| # if: steps.frontend-changes.outputs.frontend == 'true' | |
| # working-directory: surfsense_web | |
| # run: pnpm test |
| uv pip install --system --no-cache-dir -e . && \ | ||
| uv pip install --system --no-cache-dir pytest pytest-asyncio pytest-cov |
There was a problem hiding this comment.
Installing test dependencies (pytest, pytest-asyncio, pytest-cov) in the production Docker image increases the image size unnecessarily. Consider using multi-stage builds to create separate production and test images, or add these dependencies only in a development/test-specific stage to keep production images lean.
| uv pip install --system --no-cache-dir -e . && \ | |
| uv pip install --system --no-cache-dir pytest pytest-asyncio pytest-cov | |
| uv pip install --system --no-cache-dir -e . |
| - ./surfsense_backend/tests:/app/tests | ||
| - ./surfsense_backend/pytest.ini:/app/pytest.ini |
There was a problem hiding this comment.
The backend service mounts ./surfsense_backend/tests:/app/tests and ./surfsense_backend/pytest.ini:/app/pytest.ini, but these files/directories don't exist in the repository. This will cause volume mount failures when starting the backend service. Remove these volume mounts until the test infrastructure is actually created.
| - ./surfsense_backend/tests:/app/tests | |
| - ./surfsense_backend/pytest.ini:/app/pytest.ini |
1 participant
Workflows:
DevOps:
Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR enhances the CI/CD infrastructure by adding automated testing workflows for both backend and frontend, implementing Dependabot for dependency management across multiple ecosystems (Python, npm, Docker, GitHub Actions), improving Docker image publishing with better caching and OCI metadata labels, and expanding the
.gitignorewith common development patterns. The changes also include a new test service configuration indocker-compose.ymland pytest installation in the backend Dockerfile to support the new test workflows.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
.github/dependabot.yml.github/workflows/test.yml.github/workflows/docker-publish.ymldocker-compose.ymlsurfsense_backend/Dockerfile.gitignore