-
Notifications
You must be signed in to change notification settings - Fork 107
test: add unit test for subscription_handle #1028
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
…est running instructions Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
tech0priyanshu
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.
hi @Giannis-dot Thanks for time and effort. some change needs to address
exploreriii
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.
Hi @Giannis-dot
Our tests will use pytest automatically when we trigger the existing workflows - I don't think there is a need to add your own workflow to test a particular file
All of them, even new ones added, should be added to the pytest test
Will you need ci.yml?
exploreriii
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.
Also please don't forget to add a changelog entry :)
Changing this to draft while you apply changes
Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
|
@exploreriii i think i completed the changes, please check my new commits |
Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
|
i made a branch mistake before. now the last commit-push is ready for review |
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 comprehensive unit tests for the SubscriptionHandle class, testing its cancellation state management, thread lifecycle, and join operations. The PR also includes necessary test infrastructure improvements, including test shims for protobuf modules and a minimal mypy configuration file, along with documentation updates for local test execution.
- Adds 4 unit tests covering the core functionality of
SubscriptionHandle - Introduces test shims for protobuf/gRPC modules to enable isolated unit testing
- Updates documentation with instructions for running tests locally
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_subscription_handle.py |
Adds unit tests for SubscriptionHandle covering default state, cancellation, thread management, and join operations |
tools/_mypy_minimal.ini |
Adds minimal mypy configuration for ignoring missing imports during testing |
tests/_shims/hiero_sdk_python/hapi/services/timestamp_pb2.py |
Provides minimal shim for Timestamp protobuf class used in tests |
tests/_shims/hiero_sdk_python/hapi/services/basic_types_pb2.py |
Provides minimal shim for basic protobuf types including TransactionID |
tests/_shims/hiero_sdk_python/hapi/services/__init__.py |
Package initialization for services shims |
tests/_shims/hiero_sdk_python/hapi/mirror/consensus_service_pb2_grpc.py |
Provides stub for gRPC consensus service used in tests |
tests/_shims/hiero_sdk_python/hapi/mirror/__init__.py |
Package initialization for mirror shims |
tests/_shims/hiero_sdk_python/hapi/__init__.py |
Package initialization documenting purpose of test shims |
CONTRIBUTING.md |
Adds section on running tests locally with PowerShell example |
CHANGELOG.md |
Documents addition of SubscriptionHandle unit tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
872db0d to
5fb50a8
Compare
Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
Signed-off-by: Giannis <giannisgeorgiadiis@gmail.com>
|
is everything ok?do i have to do something more? |
Signed-off-by: GiannisGeo <giannisgeorgiadiis@gmail.com>
|
@exploreriii hey, can you please review this to check if everything is ok? |
|
congratulations @Giannis-dot ! |
|
Thank you my friend, that was actually my first contribution. it was a pleasure |
…est running instructions
Description:
This PR adds a missing unit test for the subscription_handle method.
Ensures correct behavior when handling subscription IDs and validates expected output.
Related issue(s):
Fixes #
Notes for reviewer:
4 tests passed in 0.13s
Checklist