Conversation
Add pytest as dev dependency and create test suite for find_site_packages, Resolver, apply_patch, and commit_changes functions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Run pytest across Python 3.9-3.12 on push/PR to main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
nomyfan
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive unit tests for the core functionality of the patch-package-py tool, enhancing code reliability and establishing a testing infrastructure. The PR configures pytest with version-specific dependencies for Python 3.9-3.12, sets up GitHub Actions for continuous testing, and refactors the module structure by removing star imports in favor of explicit imports.
Key Changes
- Added 271 lines of unit tests covering
find_site_packages,Resolver,apply_patch, andcommit_changesfunctions with 19 distinct test cases - Configured pytest as a development dependency with platform-specific version resolution (8.4.2 for Python <3.10, 9.0.1 for Python ≥3.10)
- Established CI/CD pipeline with GitHub Actions workflow testing across Python 3.9-3.12
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Added pytest and its dependencies (colorama, exceptiongroup, iniconfig, packaging, pluggy, pygments, tomli) with version-specific resolution for different Python versions |
| tests/test_core.py | New comprehensive test suite with 19 unit tests organized into 4 test classes, covering site-packages discovery, package resolution, patch application, and change committing |
| tests/init.py | Empty init file to mark tests directory as a Python package |
| pyproject.toml | Added pytest≥8.0 to dev dependencies and configured pytest test discovery settings |
| patch_package_py/cli.py | Updated imports to use explicit imports from patch_package_py.core instead of relative imports |
| patch_package_py/init.py | Removed star imports and re-exports, simplifying the module interface |
| .python-version | Changed default Python version from 3.12 to 3.9 to match minimum supported version |
| .github/workflows/test.yml | New GitHub Actions workflow for automated testing across Python 3.9-3.12 using uv package manager |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def test_commit_no_changes(self, tmp_path: Path, caplog, monkeypatch): | ||
| """Test that no patch is created when there are no changes.""" | ||
| import logging |
There was a problem hiding this comment.
The logging module should be imported at the module level (top of the file) rather than inside a test function. This follows Python best practices and improves code readability.
| site_packages = tmp_path / "lib" / "python3.9" / "site-packages" | ||
| site_packages.mkdir(parents=True) | ||
|
|
||
| with patch("os.name", "posix"): |
There was a problem hiding this comment.
The patch target should be "patch_package_py.core.os.name" instead of "os.name" to correctly mock the os.name attribute in the module where find_site_packages is defined. The current patch may not affect the actual code being tested.
| site_packages = tmp_path / "Lib" / "site-packages" | ||
| site_packages.mkdir(parents=True) | ||
|
|
||
| with patch("os.name", "nt"): |
There was a problem hiding this comment.
The patch target should be "patch_package_py.core.os.name" instead of "os.name" to correctly mock the os.name attribute in the module where find_site_packages is defined. The current patch may not affect the actual code being tested.
|
|
||
| def test_no_site_packages_raises(self, tmp_path: Path): | ||
| """Test that missing site-packages raises FileNotFoundError.""" | ||
| with patch("os.name", "posix"), pytest.raises(FileNotFoundError): |
There was a problem hiding this comment.
The patch target should be "patch_package_py.core.os.name" instead of "os.name" to correctly mock the os.name attribute in the module where find_site_packages is defined. The current patch may not affect the actual code being tested.
No description provided.