Add CI configuration and update requirements for testing tools#82
Add CI configuration and update requirements for testing tools#82whatislavx wants to merge 1 commit into
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on the GitHub Actions workflow! Your implementation successfully meets all the functional and technical requirements of the task. I am approving this pull request.
As a minor suggestion for improvement, you can make the workflow more efficient by removing the redundant Run tests step in .github/ci.yml. The Generate and display coverage report step already runs the full test suite, so the initial test run is not needed. This is just an optimization and doesn't affect the correctness of your solution.
Great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| - name: Run tests | ||
| run: pytest |
There was a problem hiding this comment.
This step to run tests is redundant. The next step, 'Generate and display coverage report', already runs the full test suite using pytest. You can remove this step to make the workflow more efficient and avoid running all the tests twice.
There was a problem hiding this comment.
Pull request overview
This PR introduces a Python CI workflow and a dev requirements file intended to support automated linting, complexity checks, tests, and coverage reporting.
Changes:
- Added
requirements-dev.txtwith pinned versions of pytest/coverage tooling and flake8. - Added a GitHub Actions CI workflow to install dependencies, run flake8 checks, run tests, generate coverage, and upload an artifact.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
requirements-dev.txt |
Introduces dev/test dependencies for CI and local development. |
.github/ci.yml |
Adds a CI workflow definition intended to run linting, tests, and coverage in GitHub Actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: Python CI | ||
| run-name: User ${{ github.actor }} - Commit ${{ github.sha }} |
There was a problem hiding this comment.
GitHub Actions only runs workflows from .github/workflows/*.yml. With this file located at .github/ci.yml, the workflow will never trigger. Move/rename it to something like .github/workflows/ci.yml.
| - name: Install dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install -r requirements.txt -r requirements-dev.txt |
There was a problem hiding this comment.
This step installs -r requirements.txt, but the repo doesn’t have a root requirements.txt (dependencies live in src/requirements.txt). Update the install command to reference the correct requirements file (or add the missing root requirements file).
| pip install -r requirements.txt -r requirements-dev.txt | |
| pip install -r src/requirements.txt -r requirements-dev.txt |
| run: pytest | ||
|
|
||
| - name: Generate and display coverage report | ||
| run: pytest --cov=. --cov-report=term-missing |
There was a problem hiding this comment.
Running pytest from the repo root is unlikely to work for this project layout: the Django app and tests live under src/ (e.g., tests import accounts.*), so imports will fail unless src is on PYTHONPATH/installed, and Django settings aren’t configured. Consider running tests via python manage.py test from src/, or add pytest-django plus the needed DJANGO_SETTINGS_MODULE and Python path configuration.
| run: pytest | |
| - name: Generate and display coverage report | |
| run: pytest --cov=. --cov-report=term-missing | |
| run: pytest | |
| working-directory: ./src | |
| - name: Generate and display coverage report | |
| run: pytest --cov=. --cov-report=term-missing | |
| working-directory: ./src |
| run: pytest | ||
|
|
||
| - name: Generate and display coverage report | ||
| run: pytest --cov=. --cov-report=term-missing |
There was a problem hiding this comment.
Tests are executed twice (once in “Run tests”, again in the coverage step), which increases CI time and can lead to inconsistent results if tests are order-dependent. Consider running tests only once while collecting coverage, and keep a separate step just to display/report coverage output.
| run: pytest | |
| - name: Generate and display coverage report | |
| run: pytest --cov=. --cov-report=term-missing | |
| run: pytest --cov=. --cov-report=term-missing | |
| - name: Generate and display coverage report | |
| run: coverage report -m |
| - name: Upload Python code as artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: python-src-code | ||
| path: . No newline at end of file |
There was a problem hiding this comment.
Uploading the artifact with path: . will include everything in the workspace (including .git, CI metadata, and any large/non-source files), which can bloat artifacts and potentially capture unintended content. Limit the artifact to the Python source tree (e.g., src/**.py, plus any needed templates/static files) or explicitly exclude .git and other non-source paths.
No description provided.