-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
refactor: Add unit testing and refactor code to follow Python standards. #31
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Reformat file with community standards black and ruff, with basic config - Format all variable namings using PEP8. - Add unit tests for two of the auxiliary methods. - Add testing on CI using github actions and pytest - Improve code clarity by using f-strings, available since python 3.6
cybcon
requested changes
Feb 6, 2024
Hi @bvanelli |
cybcon
requested changes
Feb 14, 2024
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.
Python test pipeline still fail.
In step pytest we get following error:
============================= test session starts ==============================
platform linux -- Python 3.9.18, pytest-8.0.0, pluggy-1.4.0
rootdir: /home/runner/work/modbus-server/modbus-server
collected 0 items / 2 errors
==================================== ERRORS ====================================
____________________ ERROR collecting tests/test_server.py _____________________
ImportError while importing test module '/home/runner/work/modbus-server/modbus-server/tests/test_server.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_server.py:2: in <module>
from src.app.modbus_server import prepare_register
src/app/modbus_server.py:17: in <module>
from pymodbus.datastore import (
E ModuleNotFoundError: No module named 'pymodbus'
_____________________ ERROR collecting tests/test_utils.py _____________________
ImportError while importing test module '/home/runner/work/modbus-server/modbus-server/tests/test_utils.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_utils.py:2: in <module>
from src.app.modbus_server import get_ip_address
src/app/modbus_server.py:17: in <module>
from pymodbus.datastore import (
E ModuleNotFoundError: No module named 'pymodbus'
=========================== short test summary info ============================
ERROR tests/test_server.py
ERROR tests/test_utils.py
!!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!!
============================== 2 errors in 0.10s ===============================
I had the conditional wrong, could you try again? |
cybcon
approved these changes
Feb 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello @cybcon , I started writing this code on the last Hackergarten, but did not had the time to push it yet.
You wrote the integration test with the other part of the code. This tests how the applications work together, but it's too broad to test small functions inside the code. This PR adds the basic structure for unit testing too.
I also included other changes, here is the full changelog:
I had to change some of the logic to make sure the code was testable by reducing the functions complexity, so some functions might be slightly different.
Let me know what you think and what you suggest.