-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add unit tests for asyncio download methods #243
Conversation
Any chance we can also cover these lines? This would increase the coverage of this module to a good level. Not sure how we'd do it though 🤔 |
ae85ff6
to
a943f88
Compare
84796f2
to
225f8bb
Compare
Overall, great work 👍 Just left two minor comments. |
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!
We're still missing coverage in lines 110-113 and 113-139 because they proved quite tricky to test.
I believe the time we'd spend trying to get full coverage is probably not worth it, considering the main behavior is already covered.
STONEBLD-1356 Signed-off-by: Felipe de Almeida <[email protected]>
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 👍
2d17fea
STONEBLD-1356
Maintainers will complete the following section