testcontainers / testcontainers-go

Testcontainers for Go is a Go package that makes it simple to create and clean up container-based dependencies for automated integration/smoke tests. The clean, easy-to-use API enables developers to programmatically define containers that should be run as part of a test and clean up those resources when the test is done.
https://golang.testcontainers.org
MIT License
3.3k stars 450 forks source link

chore: remove most uses of TestcontainersConfig and deprecated TestcontainersConfig fields. #2614

Closed thaJeztah closed 1 day ago

thaJeztah commented 4 days ago

What does this PR do?

remove TestProviderHasConfig as it wasn't testing anything

This test was verifying that DockerProvider.Config() returned a non-nil value, but Config() does not return a pointer, so it would always pass (even with an empty struct).

TestContainerLogWithErrClosed: rename variable that shadowed import

NewDockerProvider: inline intermediate variables

modules/compose: NewDockerComposeWith: don't use deprecated RyukDisabled

The TestcontainersConfig.RyukDisabled field is deprecated. NewDockerProvider() uses ReadConfig() internally to load the config, which always copies its value from config.Config.RyukDisabled field, which is not deprecated.

This patch removes the extra redirection, and changes the code to use Config.RyukDisabled instead.

DockerProvider: don't use legacy TestcontainersConfig internally

All fields in TestcontainersConfig are deprecated, making it a wrapper around config.Config and DockerProvider internally only depends on config.Config. All deprecated fields contain a copy of values available in config.Config.

However, the exported Config() has TestcontainersConfig as return type; filling the deprecated fields is is currently handled by ReadConfig, but straightforward, so we can inline that code in the Config() method.

This patch:

After this patch, the only remaining use of ReadConfig in testcontainers-go itself is in tests (TestReadConfig); as a follow-up, we can consider deprecating ReadConfig and TestcontainersConfig, although DockerProvider may need a new Config() method with a different signature if its config must be accessible for others, which would require the ContainerProvider and ReaperProvider interfaces to be updated.

Why is it important?

Remove uses of deprecated fields to make it easier to spot where it's still used.

Follow-ups

consider deprecating ReadConfig and TestcontainersConfig, although DockerProvider may need a new Config() method with a different signature if its config must be accessible for others, which would require the ContainerProvider and ReaperProvider interfaces to be updated.

netlify[bot] commented 4 days ago

Deploy Preview for testcontainers-go ready!

Name Link
Latest commit b4af24f9d63968ef3be6779675c42c8f78da4d51
Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6683d3b662908c00088025b2
Deploy Preview https://deploy-preview-2614--testcontainers-go.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.