onsi / gomega

Ginkgo's Preferred Matcher Library
http://onsi.github.io/gomega/
MIT License
2.15k stars 282 forks source link

improve context support #716

Open pohly opened 7 months ago

pohly commented 7 months ago

When a context is canceled, context.Cause may be able to provide a better explanation that just "context canceled" (the error returned by Context.Err). This can be useful to figure out which timeout triggered, in particular when Ginkgo also supports it (https://github.com/onsi/ginkgo/issues/1326).

https://github.com/onsi/gomega/blob/f1c8757f39be88577c525a74f3906c5f080eace0/internal/async_assertion.go#L556 could check for a cause and include that explanation. Bonus points for avoiding "Context was cancelled because context canceled" :grin:

When a Gomega async assertion has its own timeout and the callback function accepts a context, create a context that contains that timeout and pass that to the callback. Right now, the context given to gomega.Eventually is passed through (https://github.com/onsi/gomega/blob/f1c8757f39be88577c525a74f3906c5f080eace0/internal/async_assertion.go#L285), so if the callback blocks, it doesn't time out as requested via a per-assertion timeout.

pohly commented 7 months ago

It is a bit annoying that Go doesn't properly support "cause" in all variants of the context.With* calls. As it stands now (Go 1.21), one has to re-implement context.WithTimeout to get a cause both when the timeout occurs and for early cancelation.

test/utils/ktesting/contexthelper.go from https://github.com/kubernetes/kubernetes/pull/122481 is some code that I wrote for this, feel free to copy whatever you find useful.

onsi commented 7 months ago

hey there - the latest code on master now supports emiting the context cancellation reason. i started working on the second bit (passing in a context myself) but quickly ran into complexity so i'm bailing out in the interest of getting the context cancellation reason done.