-
Notifications
You must be signed in to change notification settings - Fork 41
Add initial unit tests to the proxy #216
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #216 +/- ##
==========================================
+ Coverage 27.29% 27.52% +0.23%
==========================================
Files 35 35
Lines 5400 5413 +13
Branches 779 752 -27
==========================================
+ Hits 1474 1490 +16
+ Misses 3856 3846 -10
- Partials 70 77 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Move the proxy to use a main() method so unit tests can load the file. - Remove Python 3.8 as it is no longer compatible with the proxy code.
f6ad690
to
1984b6c
Compare
@jasonacox This is ready for you to take a look at. |
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
This PR adds initial unit tests for the proxy component, focusing on the stats endpoints as a starting point. The changes also drop Python 3.8 support due to incompatibility with the removeprefix
method already used in the codebase.
- Add unit tests for
/stats
and/stats/clear
endpoints with mock-based testing infrastructure - Remove Python 3.8 from CI testing matrix due to existing incompatibility
- Refactor proxy server structure to support testing by adding a main function and fixing import paths
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pytest.ini | Adds proxy/tests to test discovery paths |
proxy/tests/test_do_get.py | New unit test file with comprehensive stats endpoint testing |
proxy/server.py | Refactors import path and adds main function for testability |
.github/workflows/test.yml | Removes Python 3.8 from CI matrix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@jasonacox Are you the only one who can call for the github copilot review? I looked around to try to find the command/UI option and I didn't see it. |
Yes, unfortunately. I do like to run it as it has found some non-obvious bugs. But also gets things worngs too. :) I made a minor change to this to accomodate my local test harness for the proxy. I suspect others may use it too (to run local vs via container). |
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
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Thanks for this @Nexarian ! |
You're welcome! As I've mentioned before, the long-term plan here is to have all of "do_GET" unit tested... So that I can then refactor it. Which is a prelude to adding the functionality for multiple inverters/gateways/etc. |
Summary
Initial unit test, as simple as I could get it. Unfortunately, it does require some modifications to the proxy to get working.
First unit test chosen because it's easy and closed-scope: The stats endpoints.
Biggest problem: The code in the proxy is no longer fully compatible with Python 3.8: https://indhumathychelliah.com/2020/10/12/new-string-methods-to-remove-prefixes-and-suffixes-in-python-3-9/
The
do_GET
handler usesremoveprefix
. This was already merged, so the proxy has been broken on 3.8 for a while. This tells me that we can justifiably drop it. Clearly nobody was actually using this with Python 3.8 any longer, otherwise we'd have gotten bug reports.