cmd/snap, daemon, overlord/snapstate: tests and fixes for "snap refresh" of a classic snap #2612

Merged
merged 3 commits into from Jan 11, 2017

Projects

None yet

3 participants

@chipaca
Contributor
chipaca commented Jan 11, 2017

No description provided.

@chipaca chipaca cmd/snap, daemon, overlord/snapstate: tests and fixes for "snap refre…
…sh" of a classic snap
d3c5b66
@mvo5
mvo5 approved these changes Jan 11, 2017 View changes

Looks good, thank you! One question inline.

overlord/snapstate/snapstate.go
@@ -520,6 +520,11 @@ func Update(st *state.State, name, channel string, revision snap.Revision, userI
channel = snapst.Channel
}
+ if !flags.JailMode {
@mvo5
mvo5 Jan 11, 2017 Collaborator

Do we need a test for this as well?

overlord/snapstate/snapstate.go
@@ -520,6 +520,11 @@ func Update(st *state.State, name, channel string, revision snap.Revision, userI
channel = snapst.Channel
}
+ if !flags.JailMode {
@pedronis
pedronis Jan 11, 2017 Contributor

is this tested? also seems obscure tbh, it will not be marked classic but it will be marked jailmode

overlord/snapstate/snapmgr_test.go
c.Assert(err, IsNil)
+ c.Check(snapsup.Flags.Classic, Equals, true)
@pedronis
pedronis Jan 11, 2017 Contributor

sort of feel a test where we actually run the tasks and look also at final snapstate would be even better

@pedronis
pedronis Jan 11, 2017 Contributor

to be clear I think such a test would show the problem @mvo5 hit, these were passing anyway

@pedronis pedronis added this to the 2.21 milestone Jan 11, 2017
@chipaca chipaca address review comments
82d607c
overlord/snapstate/snapstate.go
@@ -520,6 +520,11 @@ func Update(st *state.State, name, channel string, revision snap.Revision, userI
channel = snapst.Channel
}
+ if !(flags.JailMode || flags.DevMode) {
+ // adding a mode flag means add them all
@pedronis
pedronis Jan 11, 2017 Contributor

I don't understand the comment, the code seems ok without

@pedronis

thanks

@chipaca chipaca address review comments again
fa8b6cd
@mvo5 mvo5 merged commit 9279611 into snapcore:master Jan 11, 2017

6 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment