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 Jun 26, 2023
1 parent 8902cbe commit 519477c
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 @@ -41,6 +41,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 @@ -62,6 +63,8 @@ type daemonSuite struct {
var _ = check.Suite(&daemonSuite{})

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

func (s *daemonSuite) newDaemon(c *check.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 @@ -40,6 +52,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 519477c

Please sign in to comment.