Skip to content

Commit

Permalink
daemon: FakeCommand usage requires reaper.Start()
Browse files Browse the repository at this point in the history
The usage of testutil.FakeCommand requires the test environment to start
and stop the reaper in the case where the reaper option is enabled.

For example, in internals/daemon/daemon_test.go:

:
cmd := testutil.FakeCommand(c, "shutdown", "", true)
:

However, in daemon_test.go, the code is currently relying on the service
manager, provided by the overlord, to start the reaper. This is not a
safe solution as not all test implementations may actually run the real
overlord code, and even if they do, we have a potential race condition.

daemon.Init() -> overlord.New() -> servstate.NewManager() -> reaper.Start()

The following changes are introduced:

- Add reaper.Start() and reaper.Stop() to the daemon test setup and teardown.

- Add a reaper based test for testutil.FakeCommand().
  • Loading branch information
flotter committed Aug 1, 2023
1 parent f3d036c commit 12f56f0
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
5 changes: 5 additions & 0 deletions internals/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/canonical/pebble/internals/overlord/restart"
"github.com/canonical/pebble/internals/overlord/standby"
"github.com/canonical/pebble/internals/overlord/state"
"github.com/canonical/pebble/internals/reaper"
"github.com/canonical/pebble/internals/systemd"
"github.com/canonical/pebble/internals/testutil"
)
Expand All @@ -58,6 +59,8 @@ type daemonSuite struct {
var _ = Suite(&daemonSuite{})

func (s *daemonSuite) SetUpTest(c *C) {
err := reaper.Start()
c.Assert(err, check.IsNil)

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.19

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.18

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.17

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.16

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.15

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.14

undefined: check

Check failure on line 63 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Root Tests

undefined: check
s.pebbleDir = c.MkDir()
s.statePath = filepath.Join(s.pebbleDir, ".pebble.state")
systemdSdNotify = func(notif string) error {
Expand All @@ -71,6 +74,8 @@ func (s *daemonSuite) TearDownTest(c *C) {
s.notified = nil
s.authorized = false
s.err = nil
err := reaper.Stop()
c.Assert(err, check.IsNil)

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.19

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.18

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.17

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.16

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.15

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Go 1.14

undefined: check

Check failure on line 78 in internals/daemon/daemon_test.go

View workflow job for this annotation

GitHub Actions / Root Tests

undefined: check
}

func (s *daemonSuite) newDaemon(c *C) *Daemon {
Expand Down
19 changes: 19 additions & 0 deletions internals/testutil/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,24 @@ import (
"path/filepath"

"gopkg.in/check.v1"

"github.com/canonical/pebble/internals/reaper"
)

type fakeCommandSuite struct{}

var _ = check.Suite(&fakeCommandSuite{})

func (s *fakeCommandSuite) SetUpSuite(c *check.C) {
err := reaper.Start()
c.Assert(err, check.IsNil)
}

func (s *fakeCommandSuite) TearDownSuite(c *check.C) {
err := reaper.Stop()
c.Assert(err, check.IsNil)
}

func (s *fakeCommandSuite) TestFakeCommand(c *check.C) {
fake := FakeCommand(c, "cmd", "true", false)
defer fake.Restore()
Expand All @@ -46,6 +58,13 @@ func (s *fakeCommandSuite) TestFakeCommand(c *check.C) {
})
}

func (s *fakeCommandSuite) TestFakeCommandWithReaper(c *check.C) {
fake := FakeCommand(c, "cmd", "true", true)
defer fake.Restore()
err := exec.Command("cmd", "").Run()
c.Assert(err, check.IsNil)
}

func (s *fakeCommandSuite) TestFakeCommandAlso(c *check.C) {
fake := FakeCommand(c, "fst", "", false)
also := fake.Also("snd", "")
Expand Down

0 comments on commit 12f56f0

Please sign in to comment.