test(testutil): fix usage of reaper in exec_test.go

[benhoyt]

Currently TestFakeCommandWithReaper is half using the reaper: it's setting the withReaper flag to true, but not using reaper versions of the command-running functions. Do that. Also, only start/stop the reaper in the one test that needs it.

This should fix intermittent failures like this one:

START: exec_test.go:42: fakeCommandSuite.TestFakeCommand
using shellcheck: "/usr/bin/shellcheck"
exec_test.go:46:
    c.Assert(err, check.IsNil)
... value *os.SyscallError = &os.SyscallError{Syscall:"waitid", Err:0xa} ("waitid: no child processes")

FAIL: exec_test.go:42: fakeCommandSuite.TestFakeCommand

[flotter]

@benhoyt I'm responsible for introducing the regression here. That was a massive oversight, sorry.

I now have a much better understanding of why this reaper option exist, so let me just capture this here for the next person running into issues using FakeCommand:

I only understand now ... FakeCommand was written to be useful both in environments where the reaper exist (such as servstate) and also environmnents without the reaper (other unit tests where the reaper is not implicitly enabled). The fundamental issue is that the Run() command (from Go exec) calls the syscall wait, to reap the child process, but so does the reaper with a hook using SIGCHILD. So both cannot be used at the same time, otherwise one syscall wait will fail to see the child. To be compatible with these environments, the shellcheck script (secretly running behind the scenes) must follow the strategy used by the caller (Run vs. Reaper).

[flotter]

Perfect.