From ba35cc6bd6acf1359667d589d9755e5f2f367e58 Mon Sep 17 00:00:00 2001 From: Onsi Fakhouri Date: Wed, 26 Oct 2022 08:54:55 -0600 Subject: [PATCH] Allow ctx to be passed in as a leading parameter for Eventually and Consistently --- docs/index.md | 9 +++-- gomega_dsl.go | 20 ++++----- internal/async_assertion_test.go | 25 +++++++++++- internal/duration_bundle.go | 16 ++++---- internal/duration_bundle_test.go | 16 ++++---- internal/gomega.go | 69 ++++++++++++++++++-------------- types/types.go | 8 ++-- 7 files changed, 99 insertions(+), 64 deletions(-) diff --git a/docs/index.md b/docs/index.md index c99fe260c..be511a611 100644 --- a/docs/index.md +++ b/docs/index.md @@ -254,10 +254,10 @@ Gomega has support for making *asynchronous* assertions. There are two function `Eventually` checks that an assertion *eventually* passes. `Eventually` blocks when called and attempts an assertion periodically until it passes or a timeout occurs. Both the timeout and polling interval are configurable as optional arguments: ```go -Eventually(ACTUAL, (TIMEOUT), (POLLING_INTERVAL), (context.Context).Should(MATCHER) +Eventually(ACTUAL, (TIMEOUT), (POLLING_INTERVAL), (context.Context)).Should(MATCHER) ``` -The first optional argument is the timeout (which defaults to 1s), the second is the polling interval (which defaults to 10ms). Both intervals can be specified as time.Duration, parsable duration strings (e.g. "100ms") or `float64` (in which case they are interpreted as seconds). You can also provide a `context.Context` which - when cancelled - will instruct `Eventually` to stop and exit with a failure message. +The first optional argument is the timeout (which defaults to 1s), the second is the polling interval (which defaults to 10ms). Both intervals can be specified as time.Duration, parsable duration strings (e.g. "100ms") or `float64` (in which case they are interpreted as seconds). You can also provide a `context.Context` which - when cancelled - will instruct `Eventually` to stop and exit with a failure message. You are also allowed to pass in the `context.Context` _first_ as `Eventually(ctx, ACTUAL)`. > As with synchronous assertions, you can annotate asynchronous assertions by passing either a format string and optional inputs or a function of type `func() string` after the `GomegaMatcher`. @@ -394,9 +394,10 @@ It("adds a few books and checks the count", func(ctx SpecContext) { intialCount := client.FetchCount(ctx, "/items") client.AddItem(ctx, "foo") client.AddItem(ctx, "bar") - Eventually(client.FetchCount).WithContext(ctx).WithArguments("/items").Should(BeNumerically("==", initialCount + 2)) - Eventually(client.FetchItems).WithContext(ctx).Should(ContainElement("foo")) + //note that there are several supported ways to pass in the context. All are equivalent: + Eventually(ctx, client.FetchCount).WithArguments("/items").Should(BeNumerically("==", initialCount + 2)) Eventually(client.FetchItems).WithContext(ctx).Should(ContainElement("foo")) + Eventually(client.FetchItems, ctx).Should(ContainElement("foo")) }, SpecTimeout(time.Second * 5)) ``` diff --git a/gomega_dsl.go b/gomega_dsl.go index 2208a37ed..5dbb5f34b 100644 --- a/gomega_dsl.go +++ b/gomega_dsl.go @@ -295,9 +295,9 @@ You can poll this function like so: It is important to note that the function passed into Eventually is invoked *synchronously* when polled. Eventually does not (in fact, it cannot) kill the function if it takes longer to return than Eventually's configured timeout. A common practice here is to use a context. Here's an example that combines Ginkgo's spec timeout support with Eventually: It("fetches the correct count", func(ctx SpecContext) { - Eventually(func() int { + Eventually(ctx, func() int { return client.FetchCount(ctx, "/users") - }, ctx).Should(BeNumerically(">=", 17)) + }).Should(BeNumerically(">=", 17)) }, SpecTimeout(time.Second)) you an also use Eventually().WithContext(ctx) to pass in the context. Passed-in contexts play nicely with paseed-in arguments as long as the context appears first. You can rewrite the above example as: @@ -355,9 +355,9 @@ is equivalent to Eventually(...).WithTimeout(time.Second).WithPolling(2*time.Second).WithContext(ctx).Should(...) */ -func Eventually(actual interface{}, args ...interface{}) AsyncAssertion { +func Eventually(args ...interface{}) AsyncAssertion { ensureDefaultGomegaIsConfigured() - return Default.Eventually(actual, args...) + return Default.Eventually(args...) } // EventuallyWithOffset operates like Eventually but takes an additional @@ -369,9 +369,9 @@ func Eventually(actual interface{}, args ...interface{}) AsyncAssertion { // `EventuallyWithOffset` specifying a timeout interval (and an optional polling interval) are // the same as `Eventually(...).WithOffset(...).WithTimeout` or // `Eventually(...).WithOffset(...).WithTimeout(...).WithPolling`. -func EventuallyWithOffset(offset int, actual interface{}, args ...interface{}) AsyncAssertion { +func EventuallyWithOffset(offset int, args ...interface{}) AsyncAssertion { ensureDefaultGomegaIsConfigured() - return Default.EventuallyWithOffset(offset, actual, args...) + return Default.EventuallyWithOffset(offset, args...) } /* @@ -389,9 +389,9 @@ Consistently is useful in cases where you want to assert that something *does no This will block for 200 milliseconds and repeatedly check the channel and ensure nothing has been received. */ -func Consistently(actual interface{}, args ...interface{}) AsyncAssertion { +func Consistently(args ...interface{}) AsyncAssertion { ensureDefaultGomegaIsConfigured() - return Default.Consistently(actual, args...) + return Default.Consistently(args...) } // ConsistentlyWithOffset operates like Consistently but takes an additional @@ -400,9 +400,9 @@ func Consistently(actual interface{}, args ...interface{}) AsyncAssertion { // // `ConsistentlyWithOffset` is the same as `Consistently(...).WithOffset` and // optional `WithTimeout` and `WithPolling`. -func ConsistentlyWithOffset(offset int, actual interface{}, args ...interface{}) AsyncAssertion { +func ConsistentlyWithOffset(offset int, args ...interface{}) AsyncAssertion { ensureDefaultGomegaIsConfigured() - return Default.ConsistentlyWithOffset(offset, actual, args...) + return Default.ConsistentlyWithOffset(offset, args...) } /* diff --git a/internal/async_assertion_test.go b/internal/async_assertion_test.go index 82423a725..b5e996dc9 100644 --- a/internal/async_assertion_test.go +++ b/internal/async_assertion_test.go @@ -236,6 +236,29 @@ var _ = Describe("Asynchronous Assertions", func() { Ω(ig.FailureMessage).Should(ContainSubstring("positive: no match")) }) + It("can also be configured with the context up front", func() { + ctx, cancel := context.WithCancel(context.Background()) + counter := 0 + ig.G.Eventually(ctx, func() string { + counter++ + if counter == 2 { + cancel() + } else if counter == 10 { + return MATCH + } + return NO_MATCH + }, time.Hour).Should(SpecMatch()) + Ω(ig.FailureMessage).Should(ContainSubstring("Context was cancelled after")) + Ω(ig.FailureMessage).Should(ContainSubstring("positive: no match")) + }) + + It("treats a leading context as an actual, even if valid durations are passed in", func() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + Eventually(ctx).Should(Equal(ctx)) + Eventually(ctx, 0.1).Should(Equal(ctx)) + }) + It("counts as a failure for Consistently", func() { ctx, cancel := context.WithCancel(context.Background()) counter := 0 @@ -1228,7 +1251,7 @@ sprocket: widget: : bob sprocket: - : 17`)) + : 17`)) }) }) diff --git a/internal/duration_bundle.go b/internal/duration_bundle.go index af8d989fa..6e0d90d3a 100644 --- a/internal/duration_bundle.go +++ b/internal/duration_bundle.go @@ -44,28 +44,28 @@ func durationFromEnv(key string, defaultDuration time.Duration) time.Duration { return duration } -func toDuration(input interface{}) time.Duration { +func toDuration(input interface{}) (time.Duration, error) { duration, ok := input.(time.Duration) if ok { - return duration + return duration, nil } value := reflect.ValueOf(input) kind := reflect.TypeOf(input).Kind() if reflect.Int <= kind && kind <= reflect.Int64 { - return time.Duration(value.Int()) * time.Second + return time.Duration(value.Int()) * time.Second, nil } else if reflect.Uint <= kind && kind <= reflect.Uint64 { - return time.Duration(value.Uint()) * time.Second + return time.Duration(value.Uint()) * time.Second, nil } else if reflect.Float32 <= kind && kind <= reflect.Float64 { - return time.Duration(value.Float() * float64(time.Second)) + return time.Duration(value.Float() * float64(time.Second)), nil } else if reflect.String == kind { duration, err := time.ParseDuration(value.String()) if err != nil { - panic(fmt.Sprintf("%#v is not a valid parsable duration string.", input)) + return 0, fmt.Errorf("%#v is not a valid parsable duration string: %w", input, err) } - return duration + return duration, nil } - panic(fmt.Sprintf("%v is not a valid interval. Must be time.Duration, parsable duration string or a number.", input)) + return 0, fmt.Errorf("%#v is not a valid interval. Must be a time.Duration, a parsable duration string, or a number.", input) } diff --git a/internal/duration_bundle_test.go b/internal/duration_bundle_test.go index 7ed4f9066..aceeb043d 100644 --- a/internal/duration_bundle_test.go +++ b/internal/duration_bundle_test.go @@ -139,16 +139,16 @@ var _ = Describe("DurationBundle and Duration Support", func() { Ω(time.Since(t)).Should(BeNumerically("~", 50*time.Millisecond, 30*time.Millisecond)) }) - It("panics when the duration string can't be parsed", func() { - Ω(func() { - Consistently(true, "fries").Should(BeTrue()) - }).Should(PanicWith(`"fries" is not a valid parsable duration string.`)) + It("fails when the duration string can't be parsed", func() { + ig := NewInstrumentedGomega() + ig.G.Consistently(true, "fries").Should(BeTrue()) + Ω(ig.FailureMessage).Should(Equal(`"fries" is not a valid parsable duration string: time: invalid duration "fries"`)) }) - It("panics if anything else is passed in", func() { - Ω(func() { - Consistently(true, true).Should(BeTrue()) - }).Should(PanicWith("true is not a valid interval. Must be time.Duration, parsable duration string or a number.")) + It("fails when the duration is the wrong type", func() { + ig := NewInstrumentedGomega() + ig.G.Consistently(true, true).Should(BeTrue()) + Ω(ig.FailureMessage).Should(Equal(`true is not a valid interval. Must be a time.Duration, a parsable duration string, or a number.`)) }) }) }) diff --git a/internal/gomega.go b/internal/gomega.go index 2d67f7f1c..e75d2626a 100644 --- a/internal/gomega.go +++ b/internal/gomega.go @@ -2,6 +2,7 @@ package internal import ( "context" + "fmt" "time" "github.com/onsi/gomega/types" @@ -52,43 +53,46 @@ func (g *Gomega) ExpectWithOffset(offset int, actual interface{}, extra ...inter return NewAssertion(actual, g, offset, extra...) } -func (g *Gomega) Eventually(actual interface{}, intervals ...interface{}) types.AsyncAssertion { - return g.EventuallyWithOffset(0, actual, intervals...) +func (g *Gomega) Eventually(args ...interface{}) types.AsyncAssertion { + return g.makeAsyncAssertion(AsyncAssertionTypeEventually, 0, args...) } -func (g *Gomega) EventuallyWithOffset(offset int, actual interface{}, args ...interface{}) types.AsyncAssertion { - timeoutInterval := -time.Duration(1) - pollingInterval := -time.Duration(1) - intervals := []interface{}{} - var ctx context.Context - for _, arg := range args { - switch v := arg.(type) { - case context.Context: - ctx = v - default: - intervals = append(intervals, arg) - } - } - if len(intervals) > 0 { - timeoutInterval = toDuration(intervals[0]) - } - if len(intervals) > 1 { - pollingInterval = toDuration(intervals[1]) - } +func (g *Gomega) EventuallyWithOffset(offset int, args ...interface{}) types.AsyncAssertion { + return g.makeAsyncAssertion(AsyncAssertionTypeEventually, offset, args...) +} - return NewAsyncAssertion(AsyncAssertionTypeEventually, actual, g, timeoutInterval, pollingInterval, ctx, offset) +func (g *Gomega) Consistently(args ...interface{}) types.AsyncAssertion { + return g.makeAsyncAssertion(AsyncAssertionTypeConsistently, 0, args...) } -func (g *Gomega) Consistently(actual interface{}, intervals ...interface{}) types.AsyncAssertion { - return g.ConsistentlyWithOffset(0, actual, intervals...) +func (g *Gomega) ConsistentlyWithOffset(offset int, args ...interface{}) types.AsyncAssertion { + return g.makeAsyncAssertion(AsyncAssertionTypeConsistently, offset, args...) } -func (g *Gomega) ConsistentlyWithOffset(offset int, actual interface{}, args ...interface{}) types.AsyncAssertion { +func (g *Gomega) makeAsyncAssertion(asyncAssertionType AsyncAssertionType, offset int, args ...interface{}) types.AsyncAssertion { + baseOffset := 3 timeoutInterval := -time.Duration(1) pollingInterval := -time.Duration(1) intervals := []interface{}{} var ctx context.Context - for _, arg := range args { + if len(args) == 0 { + g.Fail(fmt.Sprintf("Call to %s is missing a value or function to poll", asyncAssertionType), offset+baseOffset) + return nil + } + + actual := args[0] + startingIndex := 1 + if _, isCtx := args[0].(context.Context); isCtx && len(args) > 1 { + // the first argument is a context, we should accept it as the context _only if_ it is **not** the only argumnent **and** the second argument is not a parseable duration + // this is due to an unfortunate ambiguity in early version of Gomega in which multi-type durations are allowed after the actual + if _, err := toDuration(args[1]); err != nil { + ctx = args[0].(context.Context) + actual = args[1] + startingIndex = 2 + } + } + + for _, arg := range args[startingIndex:] { switch v := arg.(type) { case context.Context: ctx = v @@ -96,14 +100,21 @@ func (g *Gomega) ConsistentlyWithOffset(offset int, actual interface{}, args ... intervals = append(intervals, arg) } } + var err error if len(intervals) > 0 { - timeoutInterval = toDuration(intervals[0]) + timeoutInterval, err = toDuration(intervals[0]) + if err != nil { + g.Fail(err.Error(), offset+baseOffset) + } } if len(intervals) > 1 { - pollingInterval = toDuration(intervals[1]) + pollingInterval, err = toDuration(intervals[1]) + if err != nil { + g.Fail(err.Error(), offset+baseOffset) + } } - return NewAsyncAssertion(AsyncAssertionTypeConsistently, actual, g, timeoutInterval, pollingInterval, ctx, offset) + return NewAsyncAssertion(asyncAssertionType, actual, g, timeoutInterval, pollingInterval, ctx, offset) } func (g *Gomega) SetDefaultEventuallyTimeout(t time.Duration) { diff --git a/types/types.go b/types/types.go index b479e2e85..089505a4b 100644 --- a/types/types.go +++ b/types/types.go @@ -19,11 +19,11 @@ type Gomega interface { Expect(actual interface{}, extra ...interface{}) Assertion ExpectWithOffset(offset int, actual interface{}, extra ...interface{}) Assertion - Eventually(actual interface{}, intervals ...interface{}) AsyncAssertion - EventuallyWithOffset(offset int, actual interface{}, intervals ...interface{}) AsyncAssertion + Eventually(args ...interface{}) AsyncAssertion + EventuallyWithOffset(offset int, args ...interface{}) AsyncAssertion - Consistently(actual interface{}, intervals ...interface{}) AsyncAssertion - ConsistentlyWithOffset(offset int, actual interface{}, intervals ...interface{}) AsyncAssertion + Consistently(args ...interface{}) AsyncAssertion + ConsistentlyWithOffset(offset int, args ...interface{}) AsyncAssertion SetDefaultEventuallyTimeout(time.Duration) SetDefaultEventuallyPollingInterval(time.Duration)