Skip to content

Commit 9f87527

Browse files
committed
launchd: ignore error on already-stopped service
If a 'launchd' service is bootstrapped but not currently running, calling 'launchctl kill SIGINT' on the service will result in error code 3 ("No such process"). Stopping an already-stopped service isn't an error case, so update to ignore the error code. As part of adding a new test to cover this error code, reorganize 'TestLaunchd_Stop' into table-driven tests to reduce code duplication. Signed-off-by: Victoria Dye <[email protected]>
1 parent 65332ba commit 9f87527

File tree

2 files changed

+62
-40
lines changed

2 files changed

+62
-40
lines changed

internal/daemon/launchd.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,9 @@ func (l *launchd) Stop(label string) error {
259259
}
260260

261261
// Don't throw an error if the service hasn't been bootstrapped
262-
if exitCode != 0 && exitCode != LaunchdServiceNotFoundErrorCode {
262+
if exitCode != 0 &&
263+
exitCode != LaunchdServiceNotFoundErrorCode &&
264+
exitCode != LaunchdNoSuchProcessErrorCode {
263265
return fmt.Errorf("'launchctl kill' exited with status %d", exitCode)
264266
}
265267

internal/daemon/launchd_test.go

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,44 @@ func TestLaunchd_Start(t *testing.T) {
392392
})
393393
}
394394

395+
var launchdStopTests = []struct {
396+
title string
397+
398+
// Inputs
399+
label string
400+
401+
// Mocked responses
402+
launchctlKill *Pair[int, error]
403+
404+
// Expected values
405+
expectErr bool
406+
}{
407+
{
408+
"Running service is stopped successfully",
409+
"com.test.service",
410+
PtrTo(NewPair[int, error](0, nil)), // launchctl kill
411+
false,
412+
},
413+
{
414+
"Stopping service not yet bootstrapped returns no error",
415+
"com.test.service",
416+
PtrTo(NewPair[int, error](daemon.LaunchdServiceNotFoundErrorCode, nil)), // launchctl kill
417+
false,
418+
},
419+
{
420+
"Stopping service not running returns no error",
421+
"com.test.service",
422+
PtrTo(NewPair[int, error](daemon.LaunchdNoSuchProcessErrorCode, nil)), // launchctl kill
423+
false,
424+
},
425+
{
426+
"Unknown launchctl error throws error",
427+
"com.test.service",
428+
PtrTo(NewPair[int, error](-1, nil)), // launchctl kill
429+
true,
430+
},
431+
}
432+
395433
func TestLaunchd_Stop(t *testing.T) {
396434
// Set up mocks
397435
testUser := &user.User{
@@ -405,47 +443,29 @@ func TestLaunchd_Stop(t *testing.T) {
405443

406444
launchd := daemon.NewLaunchdProvider(testUserProvider, testCommandExecutor, nil)
407445

408-
// Test #1: launchctl succeeds
409-
t.Run("Calls correct launchctl command", func(t *testing.T) {
410-
testCommandExecutor.On("Run",
411-
"launchctl",
412-
[]string{"kill", "SIGINT", fmt.Sprintf("user/123/%s", basicDaemonConfig.Label)},
413-
).Return(0, nil).Once()
414-
415-
err := launchd.Stop(basicDaemonConfig.Label)
416-
assert.Nil(t, err)
417-
mock.AssertExpectationsForObjects(t, testCommandExecutor)
418-
})
419-
420-
// Reset the mock structure between tests
421-
testCommandExecutor.Mock = mock.Mock{}
422-
423-
// Test #2: launchctl fails with uncaught error
424-
t.Run("Returns error when launchctl fails", func(t *testing.T) {
425-
testCommandExecutor.On("Run",
426-
mock.AnythingOfType("string"),
427-
mock.AnythingOfType("[]string"),
428-
).Return(1, nil).Once()
429-
430-
err := launchd.Stop(basicDaemonConfig.Label)
431-
assert.NotNil(t, err)
432-
mock.AssertExpectationsForObjects(t, testCommandExecutor)
433-
})
434-
435-
// Reset the mock structure between tests
436-
testCommandExecutor.Mock = mock.Mock{}
446+
for _, tt := range launchdStopTests {
447+
t.Run(tt.title, func(t *testing.T) {
448+
// Mock responses
449+
if tt.launchctlKill != nil {
450+
testCommandExecutor.On("Run",
451+
"launchctl",
452+
[]string{"kill", "SIGINT", fmt.Sprintf("user/123/%s", tt.label)},
453+
).Return(tt.launchctlKill.First, tt.launchctlKill.Second).Once()
454+
}
437455

438-
// Test #3: launchctl fails with expected error
439-
t.Run("Exits without error if service not found", func(t *testing.T) {
440-
testCommandExecutor.On("Run",
441-
mock.AnythingOfType("string"),
442-
mock.AnythingOfType("[]string"),
443-
).Return(daemon.LaunchdServiceNotFoundErrorCode, nil).Once()
456+
// Call function
457+
err := launchd.Stop(tt.label)
458+
mock.AssertExpectationsForObjects(t, testCommandExecutor)
459+
if tt.expectErr {
460+
assert.NotNil(t, err)
461+
} else {
462+
assert.Nil(t, err)
463+
}
464+
})
444465

445-
err := launchd.Stop(basicDaemonConfig.Label)
446-
assert.Nil(t, err)
447-
mock.AssertExpectationsForObjects(t, testCommandExecutor)
448-
})
466+
// Reset the mocks between tests
467+
testCommandExecutor.Mock = mock.Mock{}
468+
}
449469
}
450470

451471
var launchdRemoveTests = []struct {

0 commit comments

Comments
 (0)