From 374389a7edee59279be6fadfe6c5cf563e3847d6 Mon Sep 17 00:00:00 2001
From: Fred Lotter <fred.lotter@canonical.com>
Date: Mon, 26 Jun 2023 11:54:54 +0200
Subject: [PATCH] daemon: FakeCommand usage requires reaper.Start()

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().
---
 internals/daemon/daemon_test.go |  5 +++++
 internals/testutil/exec_test.go | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 1683b546..6a8e120f 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -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"
 )
@@ -58,6 +59,8 @@ type daemonSuite struct {
 var _ = Suite(&daemonSuite{})
 
 func (s *daemonSuite) SetUpTest(c *C) {
+	err := reaper.Start()
+	c.Assert(err, IsNil)
 	s.pebbleDir = c.MkDir()
 	s.statePath = filepath.Join(s.pebbleDir, ".pebble.state")
 	systemdSdNotify = func(notif string) error {
@@ -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, IsNil)
 }
 
 func (s *daemonSuite) newDaemon(c *C) *Daemon {
diff --git a/internals/testutil/exec_test.go b/internals/testutil/exec_test.go
index 18bf08d0..705bda1b 100644
--- a/internals/testutil/exec_test.go
+++ b/internals/testutil/exec_test.go
@@ -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()
@@ -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", "")