nunnatsa / ginkgolinter

golang linter for ginkgo and gomega
MIT License
25 stars 6 forks source link

[Enhancement] verify context handling #117

Open pohly opened 11 months ago

pohly commented 11 months ago

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

nunnatsa commented 11 months ago

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

pohly commented 11 months ago

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

thediveo commented 8 months ago

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 commented 8 months ago

@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 :cry: