From 519477c03fb7233ce8898315bbf063449f796101 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 c10d04d9e..bff34730c 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -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"
 )
@@ -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 {
@@ -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 {
diff --git a/internals/testutil/exec_test.go b/internals/testutil/exec_test.go
index 47c35a777..2bdffaf85 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()
@@ -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", "")