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
lifecycle: don't clean priming area if the snap is being tried #2143
lifecycle: don't clean priming area if the snap is being tried #2143
Conversation
Currently, if one uses 'snap try' and then runs 'snapcraft clean', one must 'snap try' again once the snap has been re-primed. Be smarter about this: check to see if the priming area is being used by 'snap try', and if so, don't remove it when cleaning, just remove its contents. Then once the snap has been re-primed, it'll just start working again, no 'snap try' required. Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
Codecov Report
@@ Coverage Diff @@
## master #2143 +/- ##
==========================================
+ Coverage 91.32% 91.35% +0.02%
==========================================
Files 193 194 +1
Lines 12168 12244 +76
Branches 1814 1820 +6
==========================================
+ Hits 11113 11186 +73
- Misses 714 716 +2
- Partials 341 342 +1
Continue to review full report at Codecov.
|
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.
This is missing a test with a bad mountline in order to trigger a potential KeyError
from initializing Mount
.
Looks good otherwise.
snapcraft/internal/mountinfo.py
Outdated
with contextlib.suppress(FileNotFoundError): | ||
with open(mountinfo_file) as f: | ||
for row in csv.reader(f, delimiter=' '): | ||
mount = Mount(row) |
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.
this can raise a KeyError, if catched, warn and continue would be my choice.
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.
Good catch, thank you. Fixed.
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
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.
Thanks
We are missing tests for the actual error classes under test errors. I just noticed that, sorry about not noticing before. I leave it to you to add and merge at your convenience.
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
751a404
to
6222cd4
Compare
./runtests.sh static
?./runtests.sh unit
?Currently, if one uses 'snap try' and then runs 'snapcraft clean', one must 'snap try' again once the snap has been re-primed. This PR fixes LP: #1774015 by being smarter about this: checking to see if the priming area is being used by 'snap try', and if so, not removing it when cleaning, just its contents. Then once the snap has been re-primed, it'll just start working again, no 'snap try' required.