Increase code test coverage, adjust release pipeline#6
Increase code test coverage, adjust release pipeline#6devinslick wants to merge 2 commits intomainfrom
Conversation
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR improves code quality and test coverage through formatting standardization, tooling updates, and comprehensive test additions.
- Version bumped from 2.0.2 to 2.0.3
- Added pre-commit hooks configuration for automated code quality checks
- Reformatted test file using Black formatter for consistency
- Updated GitHub Actions workflow to publish to TestPyPI on pull requests
Reviewed Changes
Copilot reviewed 6 out of 12 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_coverage_improvements.py | Applied Black formatting (line wrapping, spacing, quote consistency) throughout the test file |
| pyproject.toml | Version bump to 2.0.3 and added pre-commit to dev dependencies |
| fmd_api/_version.py | Version string updated to match pyproject.toml |
| fmd_api/client.py | Minor formatting improvements (line wrapping) |
| .pre-commit-config.yaml | Added new pre-commit configuration with Black, flake8, mypy, and file cleanup hooks |
| .github/workflows/publish.yml | Modified workflow to publish to TestPyPI on PRs and PyPI on main branch pushes |
| docs/*.md | Trailing whitespace removed from documentation files |
| .gitignore | Trailing newline added |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Publish from pull requests to main -> TestPyPI | ||
| publish_testpypi: | ||
| name: Publish to TestPyPI (branches except main) | ||
| name: Publish to TestPyPI (PR to main) | ||
| runs-on: ubuntu-latest | ||
| needs: build_sdist_and_wheel | ||
| # Only run for pushes (not releases) and only for branches that are NOT main | ||
| if: | | ||
| github.event_name == 'push' && | ||
| startsWith(github.ref, 'refs/heads/') && | ||
| github.ref != 'refs/heads/main' | ||
| # Only run for pull request events targeting main | ||
| if: github.event_name == 'pull_request' && github.base_ref == 'main' |
There was a problem hiding this comment.
Publishing to TestPyPI on every pull request to main could create issues:
- Version conflicts: Multiple PRs will try to publish the same version (2.0.3), causing failures after the first PR
- Unnecessary builds: This will publish packages for PRs that might not be merged
- The
skip-existing: trueparameter on line 71 only partially addresses this
Consider one of these alternatives:
- Only publish to TestPyPI on manual workflow dispatch or specific labels
- Use unique version suffixes for PR builds (e.g.,
2.0.3.dev{pr_number}) - Remove PR publishing entirely and only publish from main/releases
| assert "pictures/manifest.json" in files | ||
| assert any("picture_" in f and f.endswith(".png") for f in files) | ||
|
|
||
| import os |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import os to the top-level imports section.
|
|
||
| try: | ||
| import tempfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import tempfile to the top-level imports section.
| await client.export_data_zip(output_path, include_pictures=True) | ||
|
|
||
| # Should complete despite error | ||
| import zipfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import zipfile to the top-level imports section.
|
|
||
| try: | ||
| import tempfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import tempfile to the top-level imports section.
| import tempfile | ||
| import zipfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move both import tempfile and import zipfile to the top-level imports section.
|
|
||
| try: | ||
| import tempfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import tempfile to the top-level imports section (around line 6-19). This applies to multiple test functions in this file.
|
|
||
| # Verify ZIP contains PNG | ||
| import zipfile |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import zipfile to the top-level imports section.
| assert "error" in manifest[0] | ||
|
|
||
| import os |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import os to the top-level imports section.
| assert any(".jpg" in name for name in names) | ||
|
|
||
| # Cleanup | ||
| import os |
There was a problem hiding this comment.
Imports should be placed at the top of the file, not within function bodies. Move import os to the top-level imports section.
No description provided.