From 28724e27a799a398376a9e33e06167dae0379254 Mon Sep 17 00:00:00 2001 From: Adrien CABARBAYE Date: Wed, 15 Oct 2025 11:44:20 +0100 Subject: [PATCH 1/3] :gear: `[parallelisation]` Report the context cancellation cause in the related error to provide more context --- changes/20251015114355.bugfix | 1 + utils/parallelisation/contextual.go | 6 +++- utils/parallelisation/contextual_test.go | 37 ++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 changes/20251015114355.bugfix diff --git a/changes/20251015114355.bugfix b/changes/20251015114355.bugfix new file mode 100644 index 0000000000..6d5ad5eab7 --- /dev/null +++ b/changes/20251015114355.bugfix @@ -0,0 +1 @@ +:gear: `[parallelisation]` Report the context cancellation cause in the related error to provide more context diff --git a/utils/parallelisation/contextual.go b/utils/parallelisation/contextual.go index 8e64186713..f5b559d735 100644 --- a/utils/parallelisation/contextual.go +++ b/utils/parallelisation/contextual.go @@ -8,7 +8,11 @@ import ( // DetermineContextError determines what the context error is if any. func DetermineContextError(ctx context.Context) error { - return commonerrors.ConvertContextError(ctx.Err()) + err := commonerrors.ConvertContextError(ctx.Err()) + if commonerrors.Any(err, nil) { + return err + } + return commonerrors.WrapError(err, context.Cause(ctx), "") } type ContextualFunc func(ctx context.Context) error diff --git a/utils/parallelisation/contextual_test.go b/utils/parallelisation/contextual_test.go index dcbe4b8e00..d2b0942519 100644 --- a/utils/parallelisation/contextual_test.go +++ b/utils/parallelisation/contextual_test.go @@ -2,8 +2,11 @@ package parallelisation import ( "context" + "errors" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ARM-software/golang-utils/utils/commonerrors" @@ -49,3 +52,37 @@ func TestForEach(t *testing.T) { require.NoError(t, ForEach(context.Background(), WithOptions(Workers(5), JoinErrors), WrapCancelToContextualFunc(cancelFunc), WrapCancelToContextualFunc(cancelFunc), WrapCancelToContextualFunc(cancelFunc))) }) } + +func TestDetermineContextError(t *testing.T) { + t.Run("normal", func(t *testing.T) { + require.NoError(t, DetermineContextError(context.Background())) + }) + t.Run("cancellation", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + require.NoError(t, DetermineContextError(ctx)) + cancel() + err := DetermineContextError(ctx) + errortest.AssertError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + }) + t.Run("cancellation with cause", func(t *testing.T) { + cause := errors.New("a cause") + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(cause) + require.NoError(t, DetermineContextError(ctx)) + cancel(cause) + err := DetermineContextError(ctx) + errortest.AssertError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + errortest.AssertErrorDescription(t, err, cause.Error()) + }) + t.Run("cancellation with timeout cause", func(t *testing.T) { + cause := errors.New("a cause") + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, cause) + defer cancel() + require.NoError(t, DetermineContextError(ctx)) + cancel() + err := DetermineContextError(ctx) + errortest.RequireError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + assert.NotContains(t, err.Error(), cause.Error()) // the timeout did not take effect and a cancellation was performed instead so the cause is not passed through + }) +} From c64b76065a3efc44b791a275023d397f7f41b6f0 Mon Sep 17 00:00:00 2001 From: Adrien CABARBAYE Date: Wed, 15 Oct 2025 13:36:41 +0100 Subject: [PATCH 2/3] address review comments --- utils/parallelisation/contextual_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/parallelisation/contextual_test.go b/utils/parallelisation/contextual_test.go index d2b0942519..cfcc62afdb 100644 --- a/utils/parallelisation/contextual_test.go +++ b/utils/parallelisation/contextual_test.go @@ -63,7 +63,7 @@ func TestDetermineContextError(t *testing.T) { require.NoError(t, DetermineContextError(ctx)) cancel() err := DetermineContextError(ctx) - errortest.AssertError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + errortest.AssertError(t, err, commonerrors.ErrCancelled) }) t.Run("cancellation with cause", func(t *testing.T) { cause := errors.New("a cause") @@ -72,7 +72,7 @@ func TestDetermineContextError(t *testing.T) { require.NoError(t, DetermineContextError(ctx)) cancel(cause) err := DetermineContextError(ctx) - errortest.AssertError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + errortest.AssertError(t, err, commonerrors.ErrCancelled) errortest.AssertErrorDescription(t, err, cause.Error()) }) t.Run("cancellation with timeout cause", func(t *testing.T) { @@ -82,7 +82,7 @@ func TestDetermineContextError(t *testing.T) { require.NoError(t, DetermineContextError(ctx)) cancel() err := DetermineContextError(ctx) - errortest.RequireError(t, err, commonerrors.ErrTimeout, commonerrors.ErrCancelled) + errortest.RequireError(t, err, commonerrors.ErrCancelled) assert.NotContains(t, err.Error(), cause.Error()) // the timeout did not take effect and a cancellation was performed instead so the cause is not passed through }) } From 9f7a804f00ecd9737a6b465e7f59815655499d30 Mon Sep 17 00:00:00 2001 From: Adrien CABARBAYE Date: Wed, 15 Oct 2025 14:54:23 +0100 Subject: [PATCH 3/3] :bug: test --- utils/filesystem/files_test.go | 37 ---------------- utils/filesystem/files_unix_test.go | 68 +++++++++++++++++++++++++++++ utils/filesystem/lockfile_test.go | 9 ++-- 3 files changed, 73 insertions(+), 41 deletions(-) create mode 100644 utils/filesystem/files_unix_test.go diff --git a/utils/filesystem/files_test.go b/utils/filesystem/files_test.go index f3e320a095..c11753e90a 100644 --- a/utils/filesystem/files_test.go +++ b/utils/filesystem/files_test.go @@ -9,7 +9,6 @@ import ( "context" "fmt" "io" - "net" "os" "path/filepath" "reflect" @@ -23,7 +22,6 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/sys/unix" "github.com/ARM-software/golang-utils/utils/collection" "github.com/ARM-software/golang-utils/utils/commonerrors" @@ -549,9 +547,6 @@ func TestConvertPaths(t *testing.T) { } func TestIsFile(t *testing.T) { - if platform.IsWindows() { - t.Skip("Windows doesn't have features such as named pipes or unix sockets") - } for _, fsType := range FileSystemTypes { t.Run(fmt.Sprint(fsType), func(t *testing.T) { fs := NewFs(fsType) @@ -567,38 +562,6 @@ func TestIsFile(t *testing.T) { assert.True(t, b) }) - t.Run("special file", func(t *testing.T) { - if fsType == InMemoryFS { - t.Skip("In-memory file system won't have hardware devices or special files") - } - - b, err := fs.IsFile("/dev/null") - require.NoError(t, err) - assert.True(t, b) - - fifoPath := filepath.Join(tmpDir, faker.Word()) - require.NoError(t, err) - defer func() { _ = fs.Rm(fifoPath) }() - err = unix.Mkfifo(fifoPath, 0666) - require.NoError(t, err) - b, err = fs.IsFile(fifoPath) - require.NoError(t, err) - assert.True(t, b) - err = fs.Rm(fifoPath) - require.NoError(t, err) - - socketPath := filepath.Join(tmpDir, faker.Word()) - require.NoError(t, err) - defer func() { _ = fs.Rm(socketPath) }() - l, err := net.Listen("unix", socketPath) - require.NoError(t, err) - defer func() { _ = l.Close() }() - b, err = fs.IsFile(socketPath) - require.NoError(t, err) - assert.True(t, b) - err = fs.Rm(socketPath) - require.NoError(t, err) - }) }) } } diff --git a/utils/filesystem/files_unix_test.go b/utils/filesystem/files_unix_test.go new file mode 100644 index 0000000000..c45988ef9e --- /dev/null +++ b/utils/filesystem/files_unix_test.go @@ -0,0 +1,68 @@ +//go:build !windows +// +build !windows + +package filesystem + +import ( + "fmt" + "net" + "path/filepath" + "testing" + + "github.com/go-faker/faker/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +func TestIsUnixFile(t *testing.T) { + for _, fsType := range FileSystemTypes { + t.Run(fmt.Sprint(fsType), func(t *testing.T) { + fs := NewFs(fsType) + + tmpDir := t.TempDir() + + t.Run("normal file", func(t *testing.T) { + filePath := filepath.Join(tmpDir, faker.Word()) + err := fs.Touch(filePath) + require.NoError(t, err) + b, err := fs.IsFile(filePath) + require.NoError(t, err) + assert.True(t, b) + }) + + t.Run("special file", func(t *testing.T) { + if fsType == InMemoryFS { + t.Skip("In-memory file system won't have hardware devices or special files") + } + + b, err := fs.IsFile("/dev/null") + require.NoError(t, err) + assert.True(t, b) + + fifoPath := filepath.Join(tmpDir, faker.Word()) + require.NoError(t, err) + defer func() { _ = fs.Rm(fifoPath) }() + err = unix.Mkfifo(fifoPath, 0666) + require.NoError(t, err) + b, err = fs.IsFile(fifoPath) + require.NoError(t, err) + assert.True(t, b) + err = fs.Rm(fifoPath) + require.NoError(t, err) + + socketPath := filepath.Join(tmpDir, faker.Word()) + require.NoError(t, err) + defer func() { _ = fs.Rm(socketPath) }() + l, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer func() { _ = l.Close() }() + b, err = fs.IsFile(socketPath) + require.NoError(t, err) + assert.True(t, b) + err = fs.Rm(socketPath) + require.NoError(t, err) + }) + }) + } +} diff --git a/utils/filesystem/lockfile_test.go b/utils/filesystem/lockfile_test.go index d9c665e781..cc601294b7 100644 --- a/utils/filesystem/lockfile_test.go +++ b/utils/filesystem/lockfile_test.go @@ -19,6 +19,7 @@ import ( "go.uber.org/goleak" "github.com/ARM-software/golang-utils/utils/commonerrors" + "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" ) var ( @@ -361,18 +362,18 @@ func TestLockConcurrentSafeguard(t *testing.T) { // FIXME it was noticed that there could be some race conditions happening in the in-memory file system // see https://github.com/spf13/afero/issues/298 if err1 != nil { - require.Equal(t0, expectedError, err1) + errortest.RequireError(t0, err1, expectedError) } if err2 != nil { - require.Equal(t0, expectedError, err2) + errortest.RequireError(t0, err2, expectedError) } } else { require.NotEqual(t0, err1, err2) if err1 == nil { - require.Equal(t0, expectedError, err2) + errortest.RequireError(t0, err2, expectedError) } if err2 == nil { - require.Equal(t0, expectedError, err1) + errortest.RequireError(t0, err1, expectedError) } } }