From 7f47c04e079f313196692f93248a2203c5e96da4 Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:16:21 +0200 Subject: [PATCH 1/4] refactor: replace urso/sderr with stdlib errors Go 1.20 added support for wrapping multiple errors so we can migrate to native errors and drop the additional dependency on github.com/urso/sderr. This allows beats and other downstream users to drop sderr as well. --- go.mod | 2 -- go.sum | 7 ------- unison/taskgroup.go | 5 +++-- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 6a54e38..920ac46 100644 --- a/go.mod +++ b/go.mod @@ -4,14 +4,12 @@ go 1.20 require ( github.com/stretchr/testify v1.4.0 - github.com/urso/sderr v0.0.0-20210525210834-52b04e8f5c71 go.uber.org/goleak v1.0.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/urso/diag v0.0.0-20200210123136-21b3cc8eb797 // indirect golang.org/x/lint v0.0.0-20200130185559-910be7a94367 // indirect golang.org/x/tools v0.0.0-20200216192241-b320d3a0f5a2 // indirect gopkg.in/yaml.v2 v2.2.2 // indirect diff --git a/go.sum b/go.sum index 7ce8e2c..3004547 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,6 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= -github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -13,10 +11,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/urso/diag v0.0.0-20200210123136-21b3cc8eb797 h1:OHNw/6pXODJAB32NujjdQO/KIYQ3KAbHQfCzH81XdCs= -github.com/urso/diag v0.0.0-20200210123136-21b3cc8eb797/go.mod h1:pNWFTeQ+V1OYT/TzWpnWb6eQBdoXpdx+H+lrH97/Oyo= -github.com/urso/sderr v0.0.0-20210525210834-52b04e8f5c71 h1:CehQeKbysHV8J2V7AD0w8NL2x1h04kmmo/Ft5su4lU0= -github.com/urso/sderr v0.0.0-20210525210834-52b04e8f5c71/go.mod h1:Wp40HwmjM59FkDIVFfcCb9LzBbnc0XAMp8++hJuWvSU= go.uber.org/goleak v1.0.0 h1:qsup4IcBdlmsnGfqyLl4Ntn3C2XCCuKAE7DwHpScyUo= go.uber.org/goleak v1.0.0/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= @@ -39,7 +33,6 @@ golang.org/x/tools v0.0.0-20200216192241-b320d3a0f5a2 h1:0sfSpGSa544Fwnbot3Oxq/U golang.org/x/tools v0.0.0-20200216192241-b320d3a0f5a2/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/unison/taskgroup.go b/unison/taskgroup.go index 5262fd2..d8d8acb 100644 --- a/unison/taskgroup.go +++ b/unison/taskgroup.go @@ -19,10 +19,11 @@ package unison import ( "context" + "errors" + "fmt" "sync" "github.com/elastic/go-concert/ctxtool" - "github.com/urso/sderr" ) // Group interface, that can be used to start tasks. The tasks started will @@ -199,7 +200,7 @@ func (t *TaskGroup) Context() context.Context { func (t *TaskGroup) Wait() error { errs := t.waitErrors() if len(errs) > 0 { - return sderr.WrapAll(errs, "task failures") + return fmt.Errorf("task failures: %w", errors.Join(errs...)) } return nil } From 6b3c7d6ac0224021d15fba34329f3f79b218052f Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:34:06 +0200 Subject: [PATCH 2/4] ci: do not test on go 1.19 the go.mod uses a go 1.20 directive so go 1.19 is not supported --- .buildkite/pipeline.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 02d61e5..2fb117a 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -12,7 +12,6 @@ steps: matrix: setup: go_version: - - "1.19.5" - "1.20.5" command: - ".buildkite/scripts/test.sh linux" @@ -30,7 +29,6 @@ steps: matrix: setup: go_version: - - "1.19.5" - "1.20.5" command: - ".buildkite/scripts/test.ps1" @@ -47,7 +45,6 @@ steps: matrix: setup: go_version: - - "1.19.5" - "1.20.5" command: - ".buildkite/scripts/test.sh mac" From 7861f05f87f755b07ae82b567c9e72a58e92c364 Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Tue, 18 Jun 2024 00:58:30 +0200 Subject: [PATCH 3/4] test: fix race condition in TestWithFunc use atomic.Int64 to avoid race condition and make sure to increment the counter before closing the channel. --- ctxtool/func_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ctxtool/func_test.go b/ctxtool/func_test.go index d1660fd..4b8cb9d 100644 --- a/ctxtool/func_test.go +++ b/ctxtool/func_test.go @@ -20,6 +20,7 @@ package ctxtool import ( "context" "sync" + "sync/atomic" "testing" "github.com/stretchr/testify/assert" @@ -30,16 +31,16 @@ func TestWithFunc(t *testing.T) { t.Run("executed on cleanup 'cancel' call", func(t *testing.T) { defer goleak.VerifyNone(t) - count := 0 + var count atomic.Int64 wg := makeWaitGroup((1)) // func is always run asynchronously, wait ctx, cancel := WithFunc(context.Background(), func() { defer wg.Done() - count++ + count.Add(1) }) cancel() wg.Wait() assert.NotNil(t, ctx) - assert.Equal(t, 1, count) + assert.Equal(t, int64(1), count.Load()) }) t.Run("executed func on cancel", func(t *testing.T) { @@ -51,13 +52,16 @@ func TestWithFunc(t *testing.T) { ) done := make(chan struct{}) - count := 0 + var count atomic.Int64 ctx, cancel1 = context.WithCancel(context.Background()) - ctx, cancel2 = WithFunc(ctx, func() { close(done); count++ }) + ctx, cancel2 = WithFunc(ctx, func() { + count.Add(1) + close(done) + }) defer cancel2() cancel1() <-done - assert.Equal(t, 1, count) + assert.Equal(t, int64(1), count.Load()) }) t.Run("wait for other before we continue cancelling", func(t *testing.T) { From f272df735648436df6a66409af745887ba289f1c Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Tue, 18 Jun 2024 01:01:07 +0200 Subject: [PATCH 4/4] test: fix flaky TestRetryUntil use forever (~1h) duration to ensure the timeout/interval is ignored if fn does not returns an error or a canceled context is passed (test 1 and 3). use a large period to ensure the task doesn't stop waiting before the ctx expired. --- timed/timed_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/timed/timed_test.go b/timed/timed_test.go index b8dfbee..1d3080b 100644 --- a/timed/timed_test.go +++ b/timed/timed_test.go @@ -110,25 +110,25 @@ func TestPeriodic(t *testing.T) { } func TestRetryUntil(t *testing.T) { - duration := 250 * time.Millisecond - period := 50 * time.Millisecond + short := 50 * time.Millisecond + forever := 1 * time.Hour neverError := func(_ canceler) error { return nil } alwaysError := func(_ canceler) error { return errors.New("you will never get rid of me") } t.Run("retryuntil returns nil if fn no longer returns an error", func(t *testing.T) { - err := RetryUntil(context.Background(), duration, period, neverError) + err := RetryUntil(context.Background(), forever, forever, neverError) assert.NoError(t, err) }) t.Run("retryuntil returns deadline exceeded error", func(t *testing.T) { - err := RetryUntil(context.Background(), duration, period, alwaysError) + err := RetryUntil(context.Background(), short, forever, alwaysError) assert.Error(t, err) }) t.Run("retryuntil does not return error if context is canceled", func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - err := RetryUntil(ctx, duration, period, alwaysError) + err := RetryUntil(ctx, forever, forever, alwaysError) assert.NoError(t, err) }) }