-
Notifications
You must be signed in to change notification settings - Fork 2
Improve README, fix shebang bug, update Python to 3.13, add test infrastructure #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: awcook97 <8891546+awcook97@users.noreply.github.com>
Co-authored-by: awcook97 <8891546+awcook97@users.noreply.github.com>
Co-authored-by: awcook97 <8891546+awcook97@users.noreply.github.com>
…dencies Co-authored-by: awcook97 <8891546+awcook97@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Improves project maintainability by updating documentation, fixing an execution bug, bumping the pinned Python version, and introducing a pytest-based test setup.
Changes:
- Fixed
BackSEOAgencies.pyshebang for portable execution and updated.python-versionto Python 3.13. - Expanded
README.mdwith installation/usage/testing guidance. - Added initial pytest suite and configuration, plus test dependencies in
requirements.txtandpyproject.toml.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
BackSEOAgencies.py |
Fixes shebang to a portable /usr/bin/env python3. |
.python-version |
Pins development Python version to 3.13. |
README.md |
Adds detailed docs for features, install, usage, troubleshooting, and tests. |
pytest.ini |
Introduces pytest config and attempts coverage configuration. |
requirements.txt |
Adds pytest-related dependencies. |
pyproject.toml |
Updates project description and adds optional test extras. |
.gitignore |
Ignores pytest/coverage artifacts. |
tests/__init__.py |
Initializes tests package. |
tests/test_integration.py |
Adds project-structure and import integration tests. |
tests/test_backseo_settings.py |
Adds unit tests for BackSEOSettings. |
tests/test_backseo_datahandler.py |
Adds unit tests for BackSEODataHandler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Add project root to path to ensure imports work | ||
| sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | ||
|
|
||
|
|
||
| class TestProjectStructure: | ||
| """Test that the project structure is correct.""" | ||
|
|
||
| def test_main_modules_exist(self): | ||
| """Test that main modules exist.""" | ||
| modules = [ | ||
| "BackSEOAgencies.py", | ||
| "BackSEOSettings.py", | ||
| "BackSEODataHandler.py", | ||
| "BackSEOApplicationManager.py", | ||
| "PluginController.py", | ||
| "FlaskApp.py", | ||
| "core.py", | ||
| "__main__.py", | ||
| ] | ||
|
|
||
| for module in modules: | ||
| assert os.path.exists(module), f"Module {module} should exist" | ||
|
|
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These structure checks use os.path.exists() with relative paths, but the test never chdirs to the repo root. If pytest is invoked from a different working directory (common in some CI/tooling), these will fail even though the files exist. Use the computed project root (from __file__) to build absolute paths (e.g., root / module) for existence checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Needs to go back and fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit efbf2de. Tests now compute PROJECT_ROOT from __file__ and use absolute paths (e.g., PROJECT_ROOT / module) for all existence checks.
| @pytest.fixture(autouse=True) | ||
| def setup_and_teardown(self): | ||
| """Set up test fixtures and clean up after tests.""" | ||
| # Clear singleton instance before each test | ||
| BackSEOSettings._instances.clear() | ||
|
|
||
| # Create temporary directory for test outputs | ||
| self.test_dir = tempfile.mkdtemp() | ||
| self.original_cwd = os.getcwd() | ||
| os.chdir(self.test_dir) | ||
|
|
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fixture changes CWD to a temp dir, but BackSEOSettings() still creates settingsOutpath under the real user home directory via user_settings_dir() and writes settings.bseo there. This pollutes a developer/CI machine and can make tests flaky across runs. Patch user_settings_dir() (and any other home-based paths) to point into the temp directory before instantiating BackSEOSettings, or refactor BackSEOSettings to allow injecting the settings directory for tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit efbf2de. The fixture now patches BackSEOSettings.user_settings_dir to return a temporary directory, preventing pollution of the developer/CI machine.
tests/test_backseo_datahandler.py
Outdated
| import tempfile | ||
| import shutil | ||
| from multiprocessing import Queue | ||
| from unittest.mock import Mock, patch, MagicMock |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Mock' is not used.
Import of 'patch' is not used.
Import of 'MagicMock' is not used.
| from unittest.mock import Mock, patch, MagicMock |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
awcook97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've left comments on what needs to be done
Co-authored-by: awcook97 <8891546+awcook97@users.noreply.github.com>
Implementation Complete ✅
All requirements from the problem statement have been successfully implemented and review comments addressed:
✅ Bug Fixes
#!/usr/bin python3.12→#!/usr/bin/env python3)✅ Python Version Update
✅ README Improvements
✅ Testing Infrastructure
✅ Review Comments Addressed
Test Results
Security Summary
✅ No security vulnerabilities detected by CodeQL scanner.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.