-
Notifications
You must be signed in to change notification settings - Fork 601
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
Conversation
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.
some comments, mostly about blank/fields in suites
daemon/api_test.go
Outdated
@@ -87,6 +87,7 @@ type apiBaseSuite struct { | |||
storeSigning *assertstest.StoreStack | |||
restoreRelease func() | |||
trustedRestorer func() | |||
systemctlRestorer func() | |||
|
|||
origSysctlCmd func(...string) ([]byte, error) |
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.
we don't need this anymore
daemon/api_test.go
Outdated
@@ -87,6 +87,7 @@ type apiBaseSuite struct { | |||
storeSigning *assertstest.StoreStack | |||
restoreRelease func() | |||
trustedRestorer func() | |||
systemctlRestorer func() |
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.
would have a blank before this and group with the rest as orgiSysctlCmd before
storeSigning *assertstest.StoreStack | ||
restoreTrusted func() | ||
restore func() | ||
restoreSystemctl func() |
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.
we don't need prevctlCmd anymore and this should go there in its place
} | ||
} | ||
|
||
func Available() error { |
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.
does this merit a tests?
Codecov Report
@@ 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
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.
thank you
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.
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() |
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.
Nice!
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.