Skip to content

Commit

Permalink
chore: simplify ReadConfig signature, not passing context (#1292)
Browse files Browse the repository at this point in the history
* chore: simplify ReadConfig signature, not passing context

It's not needed: we added it in an intermediate state of #1161

* fix: more usages
  • Loading branch information
mdelapenya authored Jun 15, 2023
1 parent b76e5fb commit f1c8cc3
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 37 deletions.
11 changes: 1 addition & 10 deletions config.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package testcontainers

import (
"context"

"github.com/testcontainers/testcontainers-go/internal/config"
)

Expand All @@ -18,15 +16,8 @@ type TestcontainersConfig struct {

// ReadConfig reads from testcontainers properties file, storing the result in a singleton instance
// of the TestcontainersConfig struct
// Deprecated use ReadConfigWithContext instead
func ReadConfig() TestcontainersConfig {
return ReadConfigWithContext(context.Background())
}

// ReadConfigWithContext reads from testcontainers properties file, storing the result in a singleton instance
// of the TestcontainersConfig struct
func ReadConfigWithContext(ctx context.Context) TestcontainersConfig {
cfg := config.Read(ctx)
cfg := config.Read()
return TestcontainersConfig{
Host: cfg.Host,
TLSVerify: cfg.TLSVerify,
Expand Down
3 changes: 1 addition & 2 deletions config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package testcontainers

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -22,7 +21,7 @@ func TestReadConfig(t *testing.T) {
t.Setenv("HOME", "")
t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")

cfg := ReadConfigWithContext(context.Background())
cfg := ReadConfig()

expected := TestcontainersConfig{
RyukDisabled: true,
Expand Down
15 changes: 10 additions & 5 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ func TestContainerReturnItsContainerID(t *testing.T) {
}

func TestContainerStartsWithoutTheReaper(t *testing.T) {
tcConfig := config.Read(context.Background()) // read the config using the internal method to avoid the sync.Once
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if !tcConfig.RyukDisabled {
t.Skip("Ryuk is enabled, skipping test")
}
Expand Down Expand Up @@ -349,7 +350,8 @@ func TestContainerStartsWithoutTheReaper(t *testing.T) {
}

func TestContainerStartsWithTheReaper(t *testing.T) {
tcConfig := config.Read(context.Background()) // read the config using the internal method to avoid the sync.Once
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if tcConfig.RyukDisabled {
t.Skip("Ryuk is disabled, skipping test")
}
Expand Down Expand Up @@ -481,7 +483,8 @@ func TestContainerStateAfterTermination(t *testing.T) {
}

func TestContainerStopWithReaper(t *testing.T) {
tcConfig := config.Read(context.Background()) // read the config using the internal method to avoid the sync.Once
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if tcConfig.RyukDisabled {
t.Skip("Ryuk is disabled, skipping test")
}
Expand Down Expand Up @@ -528,7 +531,8 @@ func TestContainerStopWithReaper(t *testing.T) {
}

func TestContainerTerminationWithReaper(t *testing.T) {
tcConfig := config.Read(context.Background()) // read the config using the internal method to avoid the sync.Once
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if tcConfig.RyukDisabled {
t.Skip("Ryuk is disabled, skipping test")
}
Expand Down Expand Up @@ -567,7 +571,8 @@ func TestContainerTerminationWithReaper(t *testing.T) {
}

func TestContainerTerminationWithoutReaper(t *testing.T) {
tcConfig := config.Read(context.Background()) // read the config using the internal method to avoid the sync.Once
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if !tcConfig.RyukDisabled {
t.Skip("Ryuk is enabled, skipping test")
}
Expand Down
4 changes: 2 additions & 2 deletions docs/features/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ _Testcontainers for Go_ provides a struct type to represent the configuration:
[Supported properties](../../internal/config/config.go) inside_block:testcontainersConfig
<!--/codeinclude-->

You can read it with the `ReadConfigWithContext(ctx)` function:
You can read it with the `ReadConfig()` function:

```go
cfg := testcontainers.ReadConfigWithContext(ctx)
cfg := testcontainers.ReadConfig()
```

For advanced users, the Docker host connection can be configured **via configuration** in `~/.testcontainers.properties`, but environment variables will take precedence.
Expand Down
7 changes: 3 additions & 4 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"context"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -29,9 +28,9 @@ type Config struct {

// Read reads from testcontainers properties file, if it exists
// it is possible that certain values get overridden when set as environment variables
func Read(ctx context.Context) Config {
func Read() Config {
tcConfigOnce.Do(func() {
tcConfig = read(ctx)
tcConfig = read()

if tcConfig.RyukDisabled {
ryukDisabledMessage := `
Expand All @@ -54,7 +53,7 @@ func Reset() {
tcConfigOnce = new(sync.Once)
}

func read(ctx context.Context) Config {
func read() Config {
config := Config{}

applyEnvironmentConfiguration := func(config Config) Config {
Expand Down
17 changes: 8 additions & 9 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"context"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -31,7 +30,7 @@ func TestReadConfig(t *testing.T) {
t.Setenv("DOCKER_HOST", "")
t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")

config := Read(context.Background())
config := Read()

expected := Config{
RyukDisabled: true,
Expand All @@ -42,7 +41,7 @@ func TestReadConfig(t *testing.T) {

t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "false")

config = Read(context.Background())
config = Read()
assert.Equal(t, expected, config)
})
}
Expand All @@ -53,7 +52,7 @@ func TestReadTCConfig(t *testing.T) {
t.Run("HOME is not set", func(t *testing.T) {
t.Setenv("HOME", "")

config := read(context.Background())
config := read()

expected := Config{}

Expand All @@ -65,7 +64,7 @@ func TestReadTCConfig(t *testing.T) {
t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")
t.Setenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED", "true")

config := read(context.Background())
config := read()

expected := Config{
RyukDisabled: true,
Expand All @@ -80,7 +79,7 @@ func TestReadTCConfig(t *testing.T) {
tmpDir := t.TempDir()
t.Setenv("HOME", tmpDir)

config := read(context.Background())
config := read()

expected := Config{}

Expand All @@ -92,7 +91,7 @@ func TestReadTCConfig(t *testing.T) {
t.Setenv("HOME", tmpDir)
t.Setenv("DOCKER_HOST", tcpDockerHost33293)

config := read(context.Background())
config := read()
expected := Config{} // the config does not read DOCKER_HOST, that's why it's empty

assert.Equal(t, expected, config)
Expand All @@ -104,7 +103,7 @@ func TestReadTCConfig(t *testing.T) {
t.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")
t.Setenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED", "true")

config := read(context.Background())
config := read()
expected := Config{
RyukDisabled: true,
RyukPrivileged: true,
Expand Down Expand Up @@ -402,7 +401,7 @@ func TestReadTCConfig(t *testing.T) {
}

//
config := read(context.Background())
config := read()

assert.Equal(t, tt.expected, config, "Configuration doesn't not match")
})
Expand Down
2 changes: 1 addition & 1 deletion internal/testcontainersdocker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// NewClient returns a new docker client extracting the docker host from the different alternatives
func NewClient(ctx context.Context, ops ...client.Opt) (*client.Client, error) {
tcConfig := config.Read(ctx)
tcConfig := config.Read()

dockerHost := ExtractDockerHost(ctx)

Expand Down
4 changes: 2 additions & 2 deletions internal/testcontainersdocker/docker_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func dockerHostFromContext(ctx context.Context) (string, error) {

// dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty
func dockerHostFromProperties(ctx context.Context) (string, error) {
cfg := config.Read(ctx)
cfg := config.Read()
socketPath := cfg.Host
if socketPath != "" {
parsed, err := parseURL(socketPath)
Expand Down Expand Up @@ -235,7 +235,7 @@ func dockerSocketPath(ctx context.Context) (string, error) {

// testcontainersHostFromProperties returns the testcontainers host from the ~/.testcontainers.properties file, if it's not empty
func testcontainersHostFromProperties(ctx context.Context) (string, error) {
cfg := config.Read(ctx)
cfg := config.Read()
testcontainersHost := cfg.TestcontainersHost
if testcontainersHost != "" {
parsed, err := parseURL(testcontainersHost)
Expand Down
2 changes: 1 addition & 1 deletion logconsumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func TestContainerLogWithErrClosed(t *testing.T) {

provider := &DockerProvider{
client: client,
config: ReadConfigWithContext(ctx),
config: ReadConfig(),
DockerProviderOptions: &DockerProviderOptions{
GenericProviderOptions: &GenericProviderOptions{
Logger: TestLogger(t),
Expand Down
2 changes: 1 addition & 1 deletion provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error
return nil, err
}

tcConfig := ReadConfigWithContext(context.Background())
tcConfig := ReadConfig()

dockerHost := testcontainersdocker.ExtractDockerHost(context.Background())

Expand Down

0 comments on commit f1c8cc3

Please sign in to comment.