-
Notifications
You must be signed in to change notification settings - Fork 86
refactor: remove verify_interface #470
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: remove verify_interface #470
Conversation
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.
I have 2 blocking comments.
I have not reviewed the files under test
.
If we re-implement verify_interface
in
aws_encryption_sdk/internal/utils/__init__.py
,
we may need to refactor the tests... not sure yet.
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.
LGTM
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.
Approve with Suggestion
test/unit/test_utils.py
Outdated
def test_prep_stream_data_passthrough(): | ||
test = aws_encryption_sdk.internal.utils.prep_stream_data(io.BytesIO(b"some data")) | ||
|
||
assert_prepped_stream_identity(test, io.BytesIO) | ||
|
||
|
||
def test_verify_interface(patch_ec): |
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.
Suggestion: Rename test_verify_ec_interface
def test_verify_interface(patch_ec): | |
def test_verify_ec_interface(patch_ec): |
Issue #, if available:
#464
Description of changes:
verify_interface
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: