-
Notifications
You must be signed in to change notification settings - Fork 441
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
cli: allow snap-id for validate #3349
Conversation
Info API can only retrieve snap-id's for what is publicly available, workaround the issue by allowing for using the snap-id as a fallback to the snap-name. LP: #1902913 Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Thank you Sergio for working on this! I've reviewed the code and it looks good to me. I would just mention that I think there are no unit tests added for the newly added if branch, do you think that's something doable/worth it? |
Thanks @nessita. That code path is tested through the test_validate case we have which leverages our fake store servers. We have a long lasting tech debt to move this block of code closer to where the unit test lives, |
In case you want to try this out:
Or refresh
|
I've tested this and it's not working since the added variable |
that should be fixed and more thoroughly tested now |
I can confirm this now works as expected. |
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
Info API can only retrieve snap-id's for what is publicly available, workaround the issue by allowing for using the snap-id as a fallback to the snap-name. LP: #1902913 Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Info API can only retrieve snap-id's for what is publicly available,
workaround the issue by allowing for using the snap-id as a fallback
to the snap-name.
LP: #1902913
Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com
./runtests.sh static
?./runtests.sh tests/unit
?