fix(tests): improve signal handling and error reporting in lock tests (#622)

* fix(tests): improve signal handling and error reporting in lock tests

* fix(tests): prevent parallel execution in interrupt signal tests

* fix(tests): enhance signal handling by adding support for SIGTERM

Author: creativeprojects

lock/lock_test.go

@@ -3,6 +3,7 @@ package lock
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -269,7 +270,7 @@ func TestLockIsRemovedAfterInterruptSignal(t *testing.T) {
 	if platform.IsWindows() {
 		t.Skip("cannot send a signal to a child process in Windows")
 	}
-	t.Parallel()
+	// don't run in parallel
 	lockfile := getTempfile(t)
 
 	var err error
@@ -285,24 +286,23 @@ func TestLockIsRemovedAfterInterruptSignal(t *testing.T) {
 	require.NoError(t, err, "starting child process")
 
 	time.Sleep(time.Second)
-	err = cmd.Process.Signal(syscall.SIGINT)
+	err = cmd.Process.Signal(os.Interrupt)
 	require.NoError(t, err, "sending interrupt signal to child process")
 
 	err = cmd.Wait()
-	if err != nil {
-		assert.NoError(t, err, "waiting for child process to finish")
-	}
-	if buffer.Len() == 0 {
-		t.Skip(`test inconclusive: command sometimes not starting on macOS ¯\_(ツ)_/¯`)
+	if isSignalError(err) {
+		t.Skip("inconclusive test: command failed to run properly - flaky test on macOS only")
 	}
+	assert.NoError(t, err, "waiting for child process to finish")
+
 	assert.Equal(t, "started\nlock acquired\ntask interrupted\nlock released\n", buffer.String())
 }
 
 func TestLockIsRemovedAfterInterruptSignalInsideShell(t *testing.T) {
 	if platform.IsWindows() {
 		t.Skip("cannot send a signal to a child process in Windows")
 	}
-	t.Parallel()
+	// don't run in parallel
 	lockfile := getTempfile(t)
 
 	var err error
@@ -318,15 +318,28 @@ func TestLockIsRemovedAfterInterruptSignalInsideShell(t *testing.T) {
 	require.NoError(t, err, "starting child process inside a shell")
 
 	time.Sleep(time.Second)
-	err = cmd.Process.Signal(syscall.SIGINT)
+	err = cmd.Process.Signal(os.Interrupt)
 	require.NoError(t, err, "sending interrupt signal to child process")
 
 	err = cmd.Wait()
-	if err != nil {
-		assert.NoError(t, err, "waiting for child process to finish")
-	}
-	if buffer.Len() == 0 {
-		t.Skip(`test inconclusive: command sometimes not starting on macOS ¯\_(ツ)_/¯`)
+	if isSignalError(err) {
+		t.Skip("inconclusive test: command failed to run properly - flaky test on macOS only")
 	}
+	assert.NoError(t, err, "waiting for child process to finish")
+
 	assert.Equal(t, "started\nlock acquired\ntask interrupted\nlock released\n", buffer.String())
 }
+
+func isSignalError(err error) bool {
+	if err == nil {
+		return false
+	}
+	exitErr, ok := errors.AsType[*exec.ExitError](err)
+	if !ok {
+		return false
+	}
+	if status, ok := exitErr.Sys().(syscall.WaitStatus); ok {
+		return status.Signaled()
+	}
+	return false
+}

lock/test/main.go

@@ -27,9 +27,9 @@ func main() {
 }
 
 func run(wait int, lockfile string) int {
-	// Catch CTR-C key
 	sigChan := make(chan os.Signal, 2)
-	signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM, syscall.SIGABRT)
+	signal.Ignore()
+	signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
 	// remove signal catch before leaving
 	defer signal.Stop(sigChan)
 

wrapper_test.go

@@ -15,6 +15,7 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"github.com/creativeprojects/clog"
@@ -914,38 +915,40 @@ func TestBackupWithError(t *testing.T) {
 func TestBackupWithResticLockFailureRetried(t *testing.T) {
 	t.Parallel()
 
-	lockWait := constants.MinResticLockRetryDelay + time.Second
-	lockMessage := "unable to create lock in backend: repository is already locked exclusively by PID 60485 on VM by user (UID 503, GID 23)" + platform.LineSeparator +
-		"lock was created at 2023-09-24 15:29:57 (69.406ms ago)" + platform.LineSeparator +
-		"storage ID c8a44e77" + platform.LineSeparator +
-		"the `unlock` command can be used to remove stale locks" + platform.LineSeparator
-	tempfile := filepath.Join(t.TempDir(), "TestBackupWithResticLockFailureRetried.txt")
-	err := os.WriteFile(tempfile, []byte(lockMessage), 0o600)
-	require.NoError(t, err)
-	defer os.Remove(tempfile)
+	synctest.Test(t, func(t *testing.T) {
+		lockWait := constants.MinResticLockRetryDelay + time.Second
+		lockMessage := "unable to create lock in backend: repository is already locked exclusively by PID 60485 on VM by user (UID 503, GID 23)" + platform.LineSeparator +
+			"lock was created at 2023-09-24 15:29:57 (69.406ms ago)" + platform.LineSeparator +
+			"storage ID c8a44e77" + platform.LineSeparator +
+			"the `unlock` command can be used to remove stale locks" + platform.LineSeparator
+		tempfile := filepath.Join(t.TempDir(), "TestBackupWithResticLockFailureRetried.txt")
+		err := os.WriteFile(tempfile, []byte(lockMessage), 0o600)
+		require.NoError(t, err)
+		defer os.Remove(tempfile)
 
-	sigChan := make(chan os.Signal, 1)
-	global := &config.Global{
-		ResticLockRetryAfter: lockWait,
-	}
-	profile := config.NewProfile(nil, "name")
-	profile.Backup = &config.BackupSection{}
-	ctx := &Context{
-		global:   global,
-		binary:   mockBinary,
-		profile:  profile,
-		command:  "",
-		request:  Request{arguments: []string{"--stderr", "@" + tempfile, "--exit", "1"}},
-		sigChan:  sigChan,
-		terminal: term.NewTerminal(),
-	}
-	wrapper := newResticWrapper(ctx)
-	wrapper.lockWait = &lockWait
-	wrapper.startTime = time.Now()
+		sigChan := make(chan os.Signal, 1)
+		global := &config.Global{
+			ResticLockRetryAfter: lockWait,
+		}
+		profile := config.NewProfile(nil, "name")
+		profile.Backup = &config.BackupSection{}
+		ctx := &Context{
+			global:   global,
+			binary:   mockBinary,
+			profile:  profile,
+			command:  "",
+			request:  Request{arguments: []string{"--stderr", "@" + tempfile, "--exit", "1"}},
+			sigChan:  sigChan,
+			terminal: term.NewTerminal(),
+		}
+		wrapper := newResticWrapper(ctx)
+		wrapper.lockWait = &lockWait
+		wrapper.startTime = time.Now()
 
-	err = wrapper.runCommand("backup")
-	assert.Error(t, err)
-	assert.NotErrorIs(t, err, errInterrupt)
+		err = wrapper.runCommand("backup")
+		assert.Error(t, err)
+		assert.NotErrorIs(t, err, errInterrupt)
+	})
 }
 
 func TestBackupWithResticLockFailureCancelled(t *testing.T) {