From a6d58eb2d1f160a927802817744a31ff761e69a0 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Thu, 27 Apr 2023 15:46:30 -0700 Subject: [PATCH 1/5] Remove juju/errors dependency --- errors/compatibility_test.go | 16 ---------------- go.mod | 4 ---- go.sum | 9 --------- sfxclient/multitokensink_test.go | 21 ++++++++++----------- 4 files changed, 10 insertions(+), 40 deletions(-) diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index 1e66a08..4f4050a 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -6,7 +6,6 @@ import ( dropboxerrors "github.com/dropbox/godropbox/errors" facebookerrors "github.com/facebookgo/stackerr" - jujuerrors "github.com/juju/errors" . "github.com/smartystreets/goconvey/convey" ) @@ -34,21 +33,6 @@ func TestGoDropbox(t *testing.T) { }) } -func TestJujuErrors(t *testing.T) { - Convey("When the original error is jujuerror", t, func() { - root := jujuerrors.New("juju root error") - So(Tail(root), ShouldBeNil) - dropboxWrap := jujuerrors.Annotate(root, "Wrapped error") - So(Tail(dropboxWrap), ShouldEqual, root) - myAnnotation := Annotate(dropboxWrap, "I have annotated juju error") - So(Tail(myAnnotation), ShouldEqual, root) - So(Cause(myAnnotation), ShouldEqual, root) - So(Details(myAnnotation), ShouldContainSubstring, "juju root error") - So(Details(myAnnotation), ShouldContainSubstring, "I have annotated juju error") - So(jujuerrors.Cause(myAnnotation), ShouldEqual, Tail(myAnnotation)) - }) -} - func TestFacebookErrors(t *testing.T) { Convey("When the original error is fb", t, func() { root := facebookerrors.New("fb root error") diff --git a/go.mod b/go.mod index cd64e0f..331a2f7 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/go-stack/stack v1.8.1 github.com/gogo/protobuf v1.3.2 github.com/jaegertracing/jaeger v1.38.0 - github.com/juju/errors v0.0.0-20181012004132-a4583d0a56ea github.com/mailru/easyjson v0.7.7 github.com/opentracing/opentracing-go v1.2.0 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 @@ -35,8 +34,6 @@ require ( github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/jtolds/gls v4.20.0+incompatible // indirect - github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 // indirect - github.com/juju/testing v0.0.0-20191001232224-ce9dec17d28b // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect @@ -54,6 +51,5 @@ require ( golang.org/x/text v0.3.7 // indirect google.golang.org/genproto v0.0.0-20220519153652-3a47de7e79bd // indirect google.golang.org/protobuf v1.28.1 // indirect - gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 0bee2cf..bfa5dcb 100644 --- a/go.sum +++ b/go.sum @@ -76,12 +76,6 @@ github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8Hm github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo= github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= -github.com/juju/errors v0.0.0-20181012004132-a4583d0a56ea h1:g2k+8WR7cHch4g0tBDhfiEvAp7fXxTNBiD1oC1Oxj3E= -github.com/juju/errors v0.0.0-20181012004132-a4583d0a56ea/go.mod h1:W54LbzXuIE0boCoNJfwqpmkKJ1O4TCTZMetAt6jGk7Q= -github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 h1:UUHMLvzt/31azWTN/ifGWef4WUqvXk0iRqdhdy/2uzI= -github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8/go.mod h1:vgyd7OREkbtVEN/8IXZe5Ooef3LQePvuBm9UWj6ZL8U= -github.com/juju/testing v0.0.0-20191001232224-ce9dec17d28b h1:Rrp0ByJXEjhREMPGTt3aWYjoIsUGCbt21ekbeJcTWv0= -github.com/juju/testing v0.0.0-20191001232224-ce9dec17d28b/go.mod h1:63prj8cnj0tU0S9OHjGJn+b1h0ZghCndfnbQolrYTwA= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= @@ -245,11 +239,8 @@ google.golang.org/protobuf v1.28.1 h1:d0NfwRgPtno5B1Wa6L2DAG+KivqkdutMf1UhdNx175 google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 h1:VpOs+IwYnYBaFnrNAeB8UUWtL3vEUnzSCL1nVjPhqrw= -gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/sfxclient/multitokensink_test.go b/sfxclient/multitokensink_test.go index 6a981ed..989cdd3 100644 --- a/sfxclient/multitokensink_test.go +++ b/sfxclient/multitokensink_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/juju/errors" "github.com/signalfx/golib/v3/datapoint" "github.com/signalfx/golib/v3/event" "github.com/signalfx/golib/v3/log" @@ -41,9 +40,9 @@ func TestAddDataToAsyncMultitokenSink(t *testing.T) { spans := GoSpanSource.Spans() Convey("shouldn't accept dps and events with a context if a token isn't provided in the context", func() { - So(errors.Details(s.AddEvents(ctx, evs)), ShouldContainSubstring, "no value was found on the context with key") - So(errors.Details(s.AddDatapoints(ctx, dps)), ShouldContainSubstring, "no value was found on the context with key") - So(errors.Details(s.AddSpans(ctx, spans)), ShouldContainSubstring, "no value was found on the context with key") + So(s.AddEvents(ctx, evs).Error(), ShouldContainSubstring, "no value was found on the context with key") + So(s.AddDatapoints(ctx, dps).Error(), ShouldContainSubstring, "no value was found on the context with key") + So(s.AddSpans(ctx, spans).Error(), ShouldContainSubstring, "no value was found on the context with key") }) Convey("shouldn't accept dps and events if the sink has started, but the workers have shutdown", func() { @@ -52,10 +51,10 @@ func TestAddDataToAsyncMultitokenSink(t *testing.T) { So(s.Close(), ShouldBeNil) _ = s.AddEvents(ctx, evs) _ = s.AddDatapointsWithToken("HELLOOOOOO", dps) - So(errors.Details(s.AddEvents(ctx, evs)), ShouldContainSubstring, "unable to add events: the worker has been stopped") - So(errors.Details(s.AddDatapoints(ctx, dps)), ShouldContainSubstring, "unable to add datapoints: the worker has been stopped") - So(errors.Details(s.AddEventsWithToken("HELLOOOOO", evs)), ShouldContainSubstring, "unable to add events: the worker has been stopped") - So(errors.Details(s.AddDatapointsWithToken("HELLOOOOOO", dps)), ShouldContainSubstring, "unable to add datapoints: the worker has been stopped") + So(s.AddEvents(ctx, evs).Error(), ShouldContainSubstring, "unable to add events: the worker has been stopped") + So(s.AddDatapoints(ctx, dps).Error(), ShouldContainSubstring, "unable to add datapoints: the worker has been stopped") + So(s.AddEventsWithToken("HELLOOOOO", evs).Error(), ShouldContainSubstring, "unable to add events: the worker has been stopped") + So(s.AddDatapointsWithToken("HELLOOOOOO", dps).Error(), ShouldContainSubstring, "unable to add datapoints: the worker has been stopped") }) }) } @@ -282,7 +281,7 @@ func TestAsyncMultiTokenSinkShutdownDroppedDatapoints(t *testing.T) { for atomic.LoadInt64(&s.dpBuffered) <= int64(iterations) { runtime.Gosched() } - So(errors.Details(s.Close()), ShouldContainSubstring, "may have been dropped") + So(s.Close().Error(), ShouldContainSubstring, "may have been dropped") }) }) } @@ -313,7 +312,7 @@ func TestAsyncMultiTokenSinkShutdownDroppedEvents(t *testing.T) { for atomic.LoadInt64(&s.evBuffered) <= int64(iterations) { runtime.Gosched() } - So(errors.Details(s.Close()), ShouldContainSubstring, "may have been dropped") + So(s.Close().Error(), ShouldContainSubstring, "may have been dropped") }) }) } @@ -344,7 +343,7 @@ func TestAsyncMultiTokenSinkShutdownDroppedSpans(t *testing.T) { for atomic.LoadInt64(&s.spansBuffered) <= int64(iterations) { runtime.Gosched() } - So(errors.Details(s.Close()), ShouldContainSubstring, "may have been dropped") + So(s.Close().Error(), ShouldContainSubstring, "may have been dropped") }) }) } From a43f1f41113acbd2bbb27343e3280e97181dc3bb Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 28 Apr 2023 14:28:43 -0700 Subject: [PATCH 2/5] make sure we keep coverage --- errors/compatibility_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index 4f4050a..57e7b99 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -40,6 +40,7 @@ func TestFacebookErrors(t *testing.T) { fbWrap := facebookerrors.Wrap(root) So(Tail(fbWrap), ShouldEqual, root) myAnnotation := Annotate(fbWrap, "I have annotated fb error") + So(Cause(myAnnotation), ShouldEqual, root) So(Tail(myAnnotation), ShouldEqual, root) So(Cause(myAnnotation), ShouldEqual, root) From 2194d3f4277c759e0df195bfbddaac8e8b8efc92 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 28 Apr 2023 14:44:22 -0700 Subject: [PATCH 3/5] bring back tests and functionality to check causable, and add more coverage for hasMessage --- errors/compatibility_test.go | 48 +++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index 57e7b99..20165b5 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -2,6 +2,7 @@ package errors import ( "errors" + "fmt" "testing" dropboxerrors "github.com/dropbox/godropbox/errors" @@ -9,6 +10,52 @@ import ( . "github.com/smartystreets/goconvey/convey" ) +type causableErrorImpl struct { + root error + cause error +} + +func (e causableErrorImpl) Error() string { + return fmt.Sprintf("%v\n\tcause:%v", e.root, e.cause) +} + +func (e causableErrorImpl) Cause() error { + return e.cause +} + +type messageErrorImpl struct { + root error + message string +} + +func (e messageErrorImpl) Error() string { + return fmt.Sprintf("%v\n\tcause:%v", e.root, e.message) +} + +func (e messageErrorImpl) Message() string { + return e.message +} +func TestMessageErrors(t *testing.T) { + Convey("When the original error implements hasMessage", t, func() { + err := messageErrorImpl{ + root: errors.New("foo"), + message: "bar", + } + So(Message(err), ShouldEqual, "bar") + }) +} + +func TestCausableErrors(t *testing.T) { + Convey("When the original error implements causableError", t, func() { + cause := errors.New("cause") + err := causableErrorImpl{ + root: errors.New("foo"), + cause: cause, + } + So(Cause(err), ShouldEqual, cause) + }) +} + func TestGoDropbox(t *testing.T) { Convey("When the original error is godropbox", t, func() { root := dropboxerrors.New("dropbox root error") @@ -40,7 +87,6 @@ func TestFacebookErrors(t *testing.T) { fbWrap := facebookerrors.Wrap(root) So(Tail(fbWrap), ShouldEqual, root) myAnnotation := Annotate(fbWrap, "I have annotated fb error") - So(Cause(myAnnotation), ShouldEqual, root) So(Tail(myAnnotation), ShouldEqual, root) So(Cause(myAnnotation), ShouldEqual, root) From 5b033b3d2c6b3afb4cd9219227349437fcf41a2b Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 28 Apr 2023 15:00:09 -0700 Subject: [PATCH 4/5] add more coverage explicitly --- disco/disco_test.go | 3 +++ errors/compatibility_test.go | 16 ++++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/disco/disco_test.go b/disco/disco_test.go index a21abbc..d78baaa 100644 --- a/disco/disco_test.go +++ b/disco/disco_test.go @@ -287,6 +287,9 @@ func testAdvertise(t *testing.T, zkConnFunc ZkConnCreatorFunc, zkConnFunc2 ZkCon require.NotNil(t, d1) defer d1.Close() + err = d1.refreshAll() + require.NoError(t, err) + service, err := d1.Services("TestAdvertiseService") require.NoError(t, err) require.Equal(t, 0, len(service.ServiceInstances())) diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index 20165b5..d3d2016 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -10,34 +10,34 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -type causableErrorImpl struct { +type causableImpl struct { root error cause error } -func (e causableErrorImpl) Error() string { +func (e causableImpl) Error() string { return fmt.Sprintf("%v\n\tcause:%v", e.root, e.cause) } -func (e causableErrorImpl) Cause() error { +func (e causableImpl) Cause() error { return e.cause } -type messageErrorImpl struct { +type messageImpl struct { root error message string } -func (e messageErrorImpl) Error() string { +func (e messageImpl) Error() string { return fmt.Sprintf("%v\n\tcause:%v", e.root, e.message) } -func (e messageErrorImpl) Message() string { +func (e messageImpl) Message() string { return e.message } func TestMessageErrors(t *testing.T) { Convey("When the original error implements hasMessage", t, func() { - err := messageErrorImpl{ + err := messageImpl{ root: errors.New("foo"), message: "bar", } @@ -48,7 +48,7 @@ func TestMessageErrors(t *testing.T) { func TestCausableErrors(t *testing.T) { Convey("When the original error implements causableError", t, func() { cause := errors.New("cause") - err := causableErrorImpl{ + err := causableImpl{ root: errors.New("foo"), cause: cause, } From 792f04052d9cd0458428a098654a683a15a2a303 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 28 Apr 2023 15:12:36 -0700 Subject: [PATCH 5/5] gofumpt --- errors/compatibility_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index d3d2016..138acd3 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -10,34 +10,35 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -type causableImpl struct { +type causableImplError struct { root error cause error } -func (e causableImpl) Error() string { +func (e causableImplError) Error() string { return fmt.Sprintf("%v\n\tcause:%v", e.root, e.cause) } -func (e causableImpl) Cause() error { +func (e causableImplError) Cause() error { return e.cause } -type messageImpl struct { +type messageImplError struct { root error message string } -func (e messageImpl) Error() string { +func (e messageImplError) Error() string { return fmt.Sprintf("%v\n\tcause:%v", e.root, e.message) } -func (e messageImpl) Message() string { +func (e messageImplError) Message() string { return e.message } + func TestMessageErrors(t *testing.T) { Convey("When the original error implements hasMessage", t, func() { - err := messageImpl{ + err := messageImplError{ root: errors.New("foo"), message: "bar", } @@ -48,7 +49,7 @@ func TestMessageErrors(t *testing.T) { func TestCausableErrors(t *testing.T) { Convey("When the original error implements causableError", t, func() { cause := errors.New("cause") - err := causableImpl{ + err := causableImplError{ root: errors.New("foo"), cause: cause, }