Skip to content
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

many: provide systemd.MockSystemctl() helper #3865

Merged
merged 4 commits into from
Sep 11, 2017
Merged

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Sep 6, 2017

This branch allows to MockSystemctl() in similar ways as we do for other snapd packages, i.e. to provide a restore helper.

A similar branch for systemd.JournalctlCmd() will follow if this one gets acceptance.

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments, mostly about blank/fields in suites

@@ -87,6 +87,7 @@ type apiBaseSuite struct {
storeSigning *assertstest.StoreStack
restoreRelease func()
trustedRestorer func()
systemctlRestorer func()

origSysctlCmd func(...string) ([]byte, error)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this anymore

@@ -87,6 +87,7 @@ type apiBaseSuite struct {
storeSigning *assertstest.StoreStack
restoreRelease func()
trustedRestorer func()
systemctlRestorer func()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would have a blank before this and group with the rest as orgiSysctlCmd before

storeSigning *assertstest.StoreStack
restoreTrusted func()
restore func()
restoreSystemctl func()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need prevctlCmd anymore and this should go there in its place

}
}

func Available() error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this merit a tests?

@codecov-io
Copy link

codecov-io commented Sep 6, 2017

Codecov Report

Merging #3865 into master will increase coverage by <.01%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3865      +/-   ##
=========================================
+ Coverage    75.9%   75.9%   +<.01%     
=========================================
  Files         416     416              
  Lines       35988   35996       +8     
=========================================
+ Hits        27315   27323       +8     
  Misses       6753    6753              
  Partials     1920    1920
Impacted Files Coverage Δ
corecfg/corecfg.go 50% <100%> (-1.62%) ⬇️
systemd/systemd.go 82.53% <88.88%> (+0.71%) ⬆️
daemon/daemon.go 64.31% <0%> (ø) ⬆️
daemon/api.go 72.44% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e65e738...7d76cd2. Read the comment docs.

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really nice, thank you!

@@ -38,8 +38,7 @@ var (
// ensureSupportInterface checks that the system has the core-support
// interface. An error is returned if this is not the case
func ensureSupportInterface() error {
_, err := systemd.SystemctlCmd("--version")
return err
return systemd.Available()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@mvo5 mvo5 merged commit a6edba4 into canonical:master Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants