From fe65825510d584672a527de974507d8a9b24b7ee Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 8 Jun 2020 16:52:50 +0800 Subject: [PATCH] Use EnvStore to recover environment variables in testing --- exporters/trace/jaeger/env_test.go | 97 ++++++++++++++++----------- exporters/trace/jaeger/jaeger_test.go | 13 ++++ 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/exporters/trace/jaeger/env_test.go b/exporters/trace/jaeger/env_test.go index 5af9c1a23..5ce2ab1bf 100644 --- a/exporters/trace/jaeger/env_test.go +++ b/exporters/trace/jaeger/env_test.go @@ -23,10 +23,17 @@ import ( "go.opentelemetry.io/otel/api/kv" "go.opentelemetry.io/otel/api/kv/value" + ottest "go.opentelemetry.io/otel/internal/testing" ) func Test_parseTags(t *testing.T) { - require.NoError(t, os.Setenv("existing", "not-default")) + envStore, err := ottest.SetEnvVariables(map[string]string{ + "existing": "not-default", + }) + require.NoError(t, err) + defer func() { + require.NoError(t, envStore.Restore()) + }() tags := "key=value,k1=${nonExisting:default}, k2=${withSpace:default},k3=${existing:default},k4=true,k5=42,k6=-1.2" ts := parseTags(tags) @@ -127,19 +134,17 @@ func TestNewRawExporterWithEnv(t *testing.T) { tags = "key=value" ) - require.NoError(t, os.Setenv(envEndpoint, collectorEndpoint)) - require.NoError(t, os.Setenv(envUser, username)) - require.NoError(t, os.Setenv(envPassword, password)) - require.NoError(t, os.Setenv(envDisabled, disabled)) - require.NoError(t, os.Setenv(envServiceName, serviceName)) - require.NoError(t, os.Setenv(envTags, tags)) + envStore, err := ottest.SetEnvVariables(map[string]string{ + envEndpoint: collectorEndpoint, + envUser: username, + envPassword: password, + envDisabled: disabled, + envServiceName: serviceName, + envTags: tags, + }) + require.NoError(t, err) defer func() { - require.NoError(t, os.Unsetenv(envEndpoint)) - require.NoError(t, os.Unsetenv(envUser)) - require.NoError(t, os.Unsetenv(envPassword)) - require.NoError(t, os.Unsetenv(envDisabled)) - require.NoError(t, os.Unsetenv(envServiceName)) - require.NoError(t, os.Unsetenv(envTags)) + require.NoError(t, envStore.Restore()) }() // Create Jaeger Exporter with environment variables @@ -172,24 +177,22 @@ func TestNewRawExporterWithEnvImplicitly(t *testing.T) { tags = "key=value" ) - require.NoError(t, os.Setenv(envEndpoint, collectorEndpoint)) - require.NoError(t, os.Setenv(envUser, username)) - require.NoError(t, os.Setenv(envPassword, password)) - require.NoError(t, os.Setenv(envDisabled, disabled)) - require.NoError(t, os.Setenv(envServiceName, serviceName)) - require.NoError(t, os.Setenv(envTags, tags)) + envStore, err := ottest.SetEnvVariables(map[string]string{ + envEndpoint: collectorEndpoint, + envUser: username, + envPassword: password, + envDisabled: disabled, + envServiceName: serviceName, + envTags: tags, + }) + require.NoError(t, err) defer func() { - require.NoError(t, os.Unsetenv(envEndpoint)) - require.NoError(t, os.Unsetenv(envUser)) - require.NoError(t, os.Unsetenv(envPassword)) - require.NoError(t, os.Unsetenv(envDisabled)) - require.NoError(t, os.Unsetenv(envServiceName)) - require.NoError(t, os.Unsetenv(envTags)) + require.NoError(t, envStore.Restore()) }() // Create Jaeger Exporter with environment variables exp, err := NewRawExporter( - WithCollectorEndpoint(""), + WithCollectorEndpoint("should be overwritten"), WithDisabled(true), ) @@ -210,9 +213,12 @@ func TestCollectorEndpointFromEnv(t *testing.T) { collectorEndpoint = "http://localhost" ) - require.NoError(t, os.Setenv(envEndpoint, collectorEndpoint)) + envStore, err := ottest.SetEnvVariables(map[string]string{ + envEndpoint: collectorEndpoint, + }) + require.NoError(t, err) defer func() { - require.NoError(t, os.Unsetenv(envEndpoint)) + require.NoError(t, envStore.Restore()) }() assert.Equal(t, collectorEndpoint, CollectorEndpointFromEnv()) @@ -254,6 +260,12 @@ func TestWithCollectorEndpointOptionFromEnv(t *testing.T) { }, } + envStore := ottest.NewEnvStore() + envStore.Record(envUser) + envStore.Record(envPassword) + defer func() { + require.NoError(t, envStore.Restore()) + }() for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { require.NoError(t, os.Setenv(envUser, tc.envUsername)) @@ -265,9 +277,6 @@ func TestWithCollectorEndpointOptionFromEnv(t *testing.T) { assert.Equal(t, tc.expectedCollectorEndpointOptions, tc.collectorEndpointOptions) }) } - - require.NoError(t, os.Unsetenv(envUser)) - require.NoError(t, os.Unsetenv(envPassword)) } func TestWithDisabledFromEnv(t *testing.T) { @@ -291,6 +300,11 @@ func TestWithDisabledFromEnv(t *testing.T) { }, } + envStore := ottest.NewEnvStore() + envStore.Record(envDisabled) + defer func() { + require.NoError(t, envStore.Restore()) + }() for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { require.NoError(t, os.Setenv(envDisabled, tc.env)) @@ -301,8 +315,6 @@ func TestWithDisabledFromEnv(t *testing.T) { assert.Equal(t, tc.expectedOptions, tc.options) }) } - - require.NoError(t, os.Unsetenv(envDisabled)) } func TestProcessFromEnv(t *testing.T) { @@ -311,11 +323,13 @@ func TestProcessFromEnv(t *testing.T) { tags = "key=value,key2=123" ) - require.NoError(t, os.Setenv(envServiceName, serviceName)) - require.NoError(t, os.Setenv(envTags, tags)) + envStore, err := ottest.SetEnvVariables(map[string]string{ + envServiceName: serviceName, + envTags: tags, + }) + require.NoError(t, err) defer func() { - require.NoError(t, os.Unsetenv(envServiceName)) - require.NoError(t, os.Unsetenv(envTags)) + require.NoError(t, envStore.Restore()) }() p := ProcessFromEnv() @@ -381,6 +395,12 @@ func TestWithProcessFromEnv(t *testing.T) { }, } + envStore := ottest.NewEnvStore() + envStore.Record(envServiceName) + envStore.Record(envTags) + defer func() { + require.NoError(t, envStore.Restore()) + }() for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { require.NoError(t, os.Setenv(envServiceName, tc.envServiceName)) @@ -392,7 +412,4 @@ func TestWithProcessFromEnv(t *testing.T) { assert.Equal(t, tc.expectedOptions, tc.options) }) } - - require.NoError(t, os.Unsetenv(envServiceName)) - require.NoError(t, os.Unsetenv(envTags)) } diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index 34c97dfd2..01d4e1c75 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -17,18 +17,21 @@ package jaeger import ( "context" "encoding/binary" + "os" "sort" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "go.opentelemetry.io/otel/api/global" "go.opentelemetry.io/otel/api/kv" apitrace "go.opentelemetry.io/otel/api/trace" gen "go.opentelemetry.io/otel/exporters/trace/jaeger/internal/gen-go/jaeger" + ottest "go.opentelemetry.io/otel/internal/testing" export "go.opentelemetry.io/otel/sdk/export/trace" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -105,6 +108,16 @@ func TestNewRawExporter(t *testing.T) { } func TestNewRawExporterShouldFailIfCollectorEndpointEmpty(t *testing.T) { + // Record and restore env + envStore := ottest.NewEnvStore() + envStore.Record(envEndpoint) + defer func() { + require.NoError(t, envStore.Restore()) + }() + + // If the user sets the environment variable JAEGER_ENDPOINT, endpoint will always get a value. + require.NoError(t, os.Unsetenv(envEndpoint)) + _, err := NewRawExporter( WithCollectorEndpoint(""), )