[Enhancement] verify context handling

[pohly]

Is your feature request related to a problem? Please describe.

In Kubernetes, we encourage writing tests so that each It callback gets a context from Ginkgo and then passed that context on to gomega.Eventually/Consistently. This is easy to get wrong, so we have instructions: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/writing-good-e2e-tests.md#interrupting-tests

I am not sure how many of those gotchas can be detected through static analysis.

Not mentioned there (but probably should be):

It(func(ctx context.Context) {
   // Should pass ctx!
   Eventually(func() {
      // Should not use ctx from It, but rather from func(ctx context.Context) { ... }
      ... Get(ctx, ...)
   }).Should(...)
})

Describe the solution you'd like

• Optional: require that Ginkgo callbacks take a context if the callback needs a context (i.e. has a context.TODO or context.Background or calls gomega.Eventually/Consistently)
• Optional: require that gomega.Eventually/Consistently are called with a context (whether it is as first parameter or through WithContext (I think there is such a thing...)
• Always: if gomega.Eventually/Consistently is called with a context, then it's callback should also accept a context if it needs one.

[nunnatsa]

Hi @pohly - I think this is too specific to K8s. Not sure it's a generic ginkgo & gomega issue.

[pohly]

Is only Kubernetes using the context feature in Ginkgo? All other projects using it will have the same challenges.

[thediveo]

Another one who likes to shoot his own context feet. Yes, checking that if a context is used inside a function passed to Eventually should take the context from the function, not the closure; cherry on top: kick the stupid dev forgetting to pass the context into Eventually using WithContext...

[pohly]

@thediveo: Eventually(ctx, func(...)) also works. I find that more readable than Eventually(func(...)).WithContext(ctx).

Doesn't make the linter's job any easier, of course 😢