Porting tests from archived repository of OpenPAYGO-Token #27
Conversation
dmohns
left a comment
dmohns
left a comment
There was a problem hiding this comment.
Looks really good already! Left a couple of comments
openpaygo/metrics_shared.py
Outdated
| if isinstance(secret_key, (bytes, bytearray)): | ||
| secret_key_bytes = bytes(secret_key) | ||
| if len(secret_key_bytes) != 16: | ||
| raise ValueError( | ||
| "The secret key provided is not correctly formatted, it should be " | ||
| "16 " | ||
| "bytes. " | ||
| ) | ||
| return secret_key_bytes | ||
|
|
There was a problem hiding this comment.
Why do you need to change code in the metrics part? Your changes are only touching Token tests, right?
openpaygo/token_decode.py
Outdated
| if not past_used_counts: | ||
| if past_used_counts is None: |
There was a problem hiding this comment.
Why is this change required? It seem less "pythonic". Looks like you want to continue processing, when past_used_counts is an empty array, but exit the processing when None. Can you find a more descriptive way of achieving this?
|
Hey @CoderOMaster another comment: I have updated the library to use a more modern tooling Also, probably need to resolve merge conflicts. Apologies for the additional effort 🙏 |
dmohns
left a comment
dmohns
left a comment
There was a problem hiding this comment.
Hey @CoderOMaster
left some more comments.
| ## Issues | ||
|
|
||
| Issues are created [here](https://github.com/EnAccess/OpenPAYGO-python/issues/new/choose) | ||
| Create new issues in the |
There was a problem hiding this comment.
The file still shows changes. Please fully revert 😉
| To run the test, first, create and activate a virtual environment. Then install the module and run the test: | ||
|
|
||
| ```bash | ||
| cd tests | ||
| python -m venv env | ||
| source env/bin/activate | ||
| python -m pip install -e .. | ||
| python simple_scenario_test.py | ||
| ``` | ||
|
|
||
| You can do it for full test procedure as well: | ||
|
|
||
| ```bash | ||
| python full_test_procedure.py | ||
| ``` |
There was a problem hiding this comment.
This is much simpler in the new repo. Just
uv run python test/simple_scenario_test.py
uv run python test/full_test_procedure.pyCan you adapt the README ?
| if used_counts is None: | ||
| used_counts = [] |
There was a problem hiding this comment.
Please don't modify existing code, yet. Unless there is a very specific reason to do so!
| if isinstance(secret_key, (bytes, bytearray)): | ||
| secret_key_bytes = bytes(secret_key) | ||
| if len(secret_key_bytes) != 16: | ||
| raise ValueError( | ||
| "The secret key provided is not correctly formatted, it should be " | ||
| "16 " | ||
| "bytes. " | ||
| ) | ||
| return secret_key_bytes |
There was a problem hiding this comment.
Can you explain again, why this is required?
3 participants
Brief summary of the change made
Copied tests cases from OpenPAYGO-Token Repository (Archived), refactored them based on relevant imports, added error handling during hex decoding, copied and refactor device and service simulators and improved token encoding and decoding script to handle changes.
Link-> https://github.com/EnAccess/OpenPAYGO-Token.git
closes: #13
Are there any other side effects of this change that we should be aware of?
I do not think so
Describe how you tested your changes?
I have followed the readme.md to run my changes as well as pytest for original test to make sure it works
Pull Request checklist
Please confirm you have completed any of the necessary steps below.