testcontainers-go icon indicating copy to clipboard operation
testcontainers-go copied to clipboard

[Enhancement]: Do not pass the Go context across all container methods

Open mdelapenya opened this issue 2 years ago • 2 comments

Proposal

Suggested by @lanwen

It would be of interest to support passing the context in the original container request, and from there, store the context and pass it downstream. Therefore, it's not needed to pass/carry out that context in all container method signatures, simplifying the API.

Of course, this change would come with a hard breaking change, as all the methods would be changed. In the light of a short-medium term v1 release, I'd postpone this change to that v1.0.0 release.

We could create a v1 branch and start adding there any breaking changes, and from there eventually merging main into it.

mdelapenya avatar Jun 15 '23 13:06 mdelapenya

Apologies, but this seems misguided. From the Go docs on the context package (https://pkg.go.dev/context):

Programs that use Contexts should follow these rules to keep interfaces consistent across packages and enable static analysis tools to check context propagation: Do not store Contexts inside a struct type; instead, pass a Context explicitly to each function that needs it. The Context should be the first parameter, typically named ctx:

This blog post further emphasises the risks associated with doing so: https://go.dev/blog/context-and-structs

strideynet avatar Aug 22 '23 20:08 strideynet

We don't have to be crazy about that as well as passing context inside of each method has a different purpose usually from what is expected from the testing lib. For instance often context which is closed passed to a stopping container method what does nothing at the end - that's not expected at all. My initial proposal was to include context into container creation call, not into structure. And get rid of the param from methods until it makes clear sense from the testing perspective

lanwen avatar Aug 23 '23 07:08 lanwen