corecfg: mock "systemctl" in all corecfg tests #3854

Merged
merged 2 commits into from Sep 7, 2017

Conversation

Projects
None yet
5 participants
Collaborator

mvo5 commented Sep 5, 2017

This is needed to ensure the tests can run on a system that does
not have systemctl (like ubuntu 14.04).

corecfg: mock "systemctl" in all corecfg tests
This is needed to ensure the tests can run on a system that does
not have systemctl (like ubuntu 14.04).

@mvo5 mvo5 added this to the 2.28 milestone Sep 5, 2017

codecov-io commented Sep 5, 2017

Codecov Report

Merging #3854 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3854      +/-   ##
==========================================
+ Coverage   75.71%   75.78%   +0.06%     
==========================================
  Files         413      414       +1     
  Lines       35555    35742     +187     
==========================================
+ Hits        26921    27086     +165     
- Misses       6730     6745      +15     
- Partials     1904     1911       +7
Impacted Files Coverage Δ
overlord/configstate/config/transaction.go 80.4% <0%> (-4.78%) ⬇️
interfaces/sorting.go 98.71% <0%> (-1.29%) ⬇️
image/image.go 67.06% <0%> (-0.3%) ⬇️
overlord/snapstate/snapstate.go 80.15% <0%> (-0.26%) ⬇️
daemon/api.go 72.39% <0%> (-0.06%) ⬇️
store/store.go 79.82% <0%> (ø) ⬆️
overlord/devicestate/handlers.go 63.31% <0%> (ø) ⬆️
overlord/snapstate/storehelpers.go 80% <0%> (ø) ⬆️
overlord/snapstate/handlers.go 65.1% <0%> (ø) ⬆️
overlord/storestate/storestate.go 100% <0%> (ø)
... and 7 more

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 39c2132...9043ba5. Read the comment docs.

zyga approved these changes Sep 6, 2017

Hmm, I was under the impression that this would never ever touch the real systemctl. I recall seeing a bug (race?) where this test would sometimes fail to figure out something and then proceeded into the "real code" and call systemctl.

While I don't mind the explicit mocking I feel we are missing something here.

Collaborator

mvo5 commented Sep 6, 2017

We do touch the real systemd because corecfg runs systemctl --version to detect if the core-support plug is connected. This is where this issue from from. Sorry for not explaining that in code better.

looks ok, but a doubt

corecfg/services_test.go
systemctlArgs [][]string
}
var _ = Suite(&servicesSuite{})
func (s *servicesSuite) SetUpSuite(c *C) {
+ s.coreCfgSuite.SetUpSuite(c)
+
systemd.SystemctlCmd = func(args ...string) ([]byte, error) {
@pedronis

pedronis Sep 6, 2017

Contributor

this looks strange, are we calling here systemctl both directly and through the systemd pkg? or we don't need coreCfgSuite here?

@mvo5

mvo5 Sep 6, 2017

Collaborator

Indeed, excellent point. This mixing is a mistake that I will fix

@mvo5 mvo5 merged commit 7495d55 into snapcore:master Sep 7, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
artful-i386 autopkgtest finished (success)
Details
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
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment