clean cmd: migrate to use new lifecycle attribs #148

Merged
merged 2 commits into from Dec 3, 2015

Conversation

Projects
None yet
2 participants
Collaborator

sergiusens commented Dec 3, 2015

This also migrates clean to use real elements instead of
over mocking.

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

snapcraft/commands/clean.py
@@ -62,7 +57,8 @@ def main(argv=None):
not os.listdir(common.get_partsdir())):
os.rmdir(common.get_partsdir())
- clean_stage = not args['PART'] or part_names == set(args['PART'])
+ parts_match = set(config.part_names) == set(args['PART'])
+ clean_stage = not args['PART'] or parts_match
if clean_stage and os.path.exists(common.get_stagedir()):
@elopio

elopio Dec 3, 2015

Member

This deserves a comment. "Delete the stage dir only if all the parts were cleaned"

did I get it right?

@sergiusens

sergiusens Dec 3, 2015

Collaborator

That is correct, I'll add a comment.

-class _IO(io.StringIO):
-
- def fileno(self):
- return 1
@elopio

elopio Dec 3, 2015

Member

this looks crazy. Good delete.

@sergiusens

sergiusens Dec 3, 2015

Collaborator

It was important when trying to mock stdout for subprocess, that code was killed, so this is not used anymore.

snapcraft/tests/test_commands_clean.py
+ 'Expected for {!r} to be wiped'.format(part['part_dir']))
+ self.assertFalse(
+ os.path.exists(part['state_file']),
+ 'Expected for {!r} to be wiped'.format(part['state_file']))
@elopio

elopio Dec 3, 2015

Member

This might be unnecessary, isn't it enough to check that partsdir doesn't exist?
Feel free to leave the extra checks if you think they might be useful.

@sergiusens

sergiusens Dec 3, 2015

Collaborator

True that!

snapcraft/tests/test_commands_clean.py
+ 'Expected for {!r} to be wiped'.format(part['part_dir']))
+ self.assertFalse(
+ os.path.exists(part['state_file']),
+ 'Expected for {!r} to be wiped'.format(part['state_file']))
@elopio

elopio Dec 3, 2015

Member

same comment as above.

@sergiusens

sergiusens Dec 3, 2015

Collaborator

ditto.

Member

elopio commented Dec 3, 2015

💯 I like it a lot better without the mocks.
I just left a couple of comments because.

sergiusens added some commits Dec 3, 2015

clean cmd: migrate to use new lifecycle attribs
This also migrates clean to use real elements instead of
over mocking.

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Removing dead test code
Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Collaborator

sergiusens commented Dec 3, 2015

all good then?

Member

elopio commented Dec 3, 2015

all good.

sergiusens added a commit that referenced this pull request Dec 3, 2015

Merge pull request #148 from sergiusens/07-clean-cleanup
clean cmd: migrate to use new lifecycle attribs

@sergiusens sergiusens merged commit 3931b08 into snapcore:new-cli Dec 3, 2015

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.1%) to 80.178%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sergiusens sergiusens deleted the sergiusens:07-clean-cleanup branch Dec 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment