From e34689502c46f6693006e259ed94e96ec3fa09b4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 6 Jul 2022 09:29:40 +0200 Subject: env: Rewrite tests to match modern best practices The tests for our environment helpers aren't quite matching our modern best practices. Furthermore, they're missing tests for the case where environment variables are not set at all. Refactor these tests to adapt. While at it, convert them to use the new `t.Setenv()` and `testhelper.Unsetenv()` helper functions. --- internal/helper/env/env_test.go | 310 +++++++++++++++++++++++++--------------- 1 file changed, 198 insertions(+), 112 deletions(-) diff --git a/internal/helper/env/env_test.go b/internal/helper/env/env_test.go index d38cf0362..5a83ebbe4 100644 --- a/internal/helper/env/env_test.go +++ b/internal/helper/env/env_test.go @@ -1,208 +1,288 @@ package env_test import ( - "errors" "fmt" "strconv" "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/env" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) func TestGetBool(t *testing.T) { + const envvar = "TEST_BOOL" + for _, tc := range []struct { - value string + desc string + setup func(t *testing.T) fallback bool - expected bool - expectedErrIs error + expectedValue bool + expectedErr error }{ { - value: "true", - expected: true, + desc: "explicitly true", + setup: setupEnv(envvar, "true"), + expectedValue: true, + }, + { + desc: "explicitly false", + setup: setupEnv(envvar, "false"), + expectedValue: false, }, { - value: "false", - expected: false, + desc: "explicitly 1", + setup: setupEnv(envvar, "1"), + expectedValue: true, }, { - value: "1", - expected: true, + desc: "explicitly 0", + setup: setupEnv(envvar, "0"), + expectedValue: false, + }, + { + desc: "missing value", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + expectedValue: false, }, { - value: "0", - expected: false, + desc: "missing value with fallback", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + fallback: true, + expectedValue: true, }, { - value: "", - expected: false, + desc: "empty value", + setup: setupEnv(envvar, ""), + expectedValue: false, }, { - value: "", - fallback: true, - expected: true, + desc: "empty value with fallback", + setup: setupEnv(envvar, ""), + fallback: true, + expectedValue: true, }, { - value: "bad", - expected: false, - expectedErrIs: strconv.ErrSyntax, + desc: "invalid value", + setup: setupEnv(envvar, "bad"), + expectedValue: false, + expectedErr: fmt.Errorf("get bool TEST_BOOL: %w", &strconv.NumError{ + Func: "ParseBool", + Num: "bad", + Err: strconv.ErrSyntax, + }), }, { - value: "bad", + desc: "invalid value with fallback", + setup: setupEnv(envvar, "bad"), fallback: true, - expected: true, - expectedErrIs: strconv.ErrSyntax, + expectedValue: true, + expectedErr: fmt.Errorf("get bool TEST_BOOL: %w", &strconv.NumError{ + Func: "ParseBool", + Num: "bad", + Err: strconv.ErrSyntax, + }), }, } { - t.Run(fmt.Sprintf("value=%s,fallback=%t", tc.value, tc.fallback), func(t *testing.T) { - testhelper.ModifyEnvironment(t, "TEST_BOOL", tc.value) - - result, err := env.GetBool("TEST_BOOL", tc.fallback) - - if tc.expectedErrIs != nil { - assert.Error(t, err) - assert.True(t, errors.Is(err, tc.expectedErrIs), err) - } else { - assert.NoError(t, err) - } + t.Run(tc.desc, func(t *testing.T) { + tc.setup(t) - assert.Equal(t, tc.expected, result) + value, err := env.GetBool(envvar, tc.fallback) + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.expectedValue, value) }) } } func TestGetInt(t *testing.T) { + const envvar = "TEST_INT" + for _, tc := range []struct { - value string + desc string + setup func(t *testing.T) fallback int - expected int - expectedErrIs error + expectedValue int + expectedErr error }{ { - value: "3", - expected: 3, + desc: "valid number", + setup: setupEnv(envvar, "3"), + expectedValue: 3, + }, + { + desc: "unset value", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + expectedValue: 0, + }, + { + desc: "unset value with fallback", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + fallback: 3, + expectedValue: 3, }, { - value: "", - expected: 0, + desc: "empty value", + setup: setupEnv(envvar, ""), + expectedValue: 0, }, { - value: "", - fallback: 3, - expected: 3, + desc: "empty value with fallback", + setup: setupEnv(envvar, ""), + fallback: 3, + expectedValue: 3, }, { - value: "bad", - expected: 0, - expectedErrIs: strconv.ErrSyntax, + desc: "invalid value", + setup: setupEnv(envvar, "bad"), + expectedValue: 0, + expectedErr: fmt.Errorf("get int TEST_INT: %w", &strconv.NumError{ + Func: "Atoi", + Num: "bad", + Err: strconv.ErrSyntax, + }), }, { - value: "bad", + desc: "invalid value with fallback", + setup: setupEnv(envvar, "bad"), fallback: 3, - expected: 3, - expectedErrIs: strconv.ErrSyntax, + expectedValue: 3, + expectedErr: fmt.Errorf("get int TEST_INT: %w", &strconv.NumError{ + Func: "Atoi", + Num: "bad", + Err: strconv.ErrSyntax, + }), }, } { - t.Run(fmt.Sprintf("value=%s,fallback=%d", tc.value, tc.fallback), func(t *testing.T) { - testhelper.ModifyEnvironment(t, "TEST_INT", tc.value) - - result, err := env.GetInt("TEST_INT", tc.fallback) - - if tc.expectedErrIs != nil { - assert.Error(t, err) - assert.True(t, errors.Is(err, tc.expectedErrIs), err) - } else { - assert.NoError(t, err) - } + t.Run(tc.desc, func(t *testing.T) { + tc.setup(t) - assert.Equal(t, tc.expected, result) + value, err := env.GetInt(envvar, tc.fallback) + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.expectedValue, value) }) } } func TestGetDuration(t *testing.T) { + const envvar = "TEST_DURATION" + for _, tc := range []struct { - value string - fallback time.Duration - expected time.Duration - expectedErr string + desc string + setup func(t *testing.T) + fallback time.Duration + expectedValue time.Duration + expectedErr error }{ { - value: "3m", - fallback: 0, - expected: 3 * time.Minute, + desc: "valid duration", + setup: setupEnv(envvar, "3m"), + fallback: 0, + expectedValue: 3 * time.Minute, + }, + { + desc: "unset value", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + expectedValue: 0, + }, + { + desc: "unset value with fallback", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + fallback: 3, + expectedValue: 3, }, { - value: "", - expected: 0, + desc: "empty value", + setup: setupEnv(envvar, ""), + expectedValue: 0, }, { - value: "", - fallback: 3, - expected: 3, + desc: "empty value with fallback", + setup: setupEnv(envvar, ""), + fallback: 3, + expectedValue: 3, }, { - value: "bad", - expected: 0, - expectedErr: `get duration TEST_DURATION: time: invalid duration "bad"`, + desc: "invalid value", + setup: setupEnv(envvar, "bad"), + expectedValue: 0, + expectedErr: fmt.Errorf("get duration TEST_DURATION: %w", fmt.Errorf("time: invalid duration \"bad\"")), }, { - value: "bad", - fallback: 3, - expected: 3, - expectedErr: `get duration TEST_DURATION: time: invalid duration "bad"`, + desc: "invalid value with fallback", + setup: setupEnv(envvar, "bad"), + fallback: 3, + expectedValue: 3, + expectedErr: fmt.Errorf("get duration TEST_DURATION: %w", fmt.Errorf("time: invalid duration \"bad\"")), }, } { - t.Run(fmt.Sprintf("value=%s,fallback=%d", tc.value, tc.fallback), func(t *testing.T) { - testhelper.ModifyEnvironment(t, "TEST_DURATION", tc.value) - - result, err := env.GetDuration("TEST_DURATION", tc.fallback) - - if tc.expectedErr != "" { - assert.Error(t, err) - assert.EqualError(t, err, tc.expectedErr) - } else { - assert.NoError(t, err) - } + t.Run(tc.desc, func(t *testing.T) { + tc.setup(t) - assert.Equal(t, tc.expected, result) + value, err := env.GetDuration(envvar, tc.fallback) + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.expectedValue, value) }) } } func TestGetString(t *testing.T) { + const envvar = "TEST_STRING" + for _, tc := range []struct { - value string - fallback string - expected string + desc string + setup func(t *testing.T) + fallback string + expectedValue string }{ { - value: "Hello", - expected: "Hello", + desc: "simple string", + setup: setupEnv(envvar, "Hello"), + expectedValue: "Hello", }, { - value: "hello ", - expected: "hello", + desc: "string with trailing whitespace", + setup: setupEnv(envvar, "hello "), + expectedValue: "hello", + }, + { + desc: "unset value", + setup: func(t *testing.T) { + testhelper.Unsetenv(t, envvar) + }, + fallback: "fallback value", + expectedValue: "fallback value", }, { - fallback: "fallback value", - expected: "fallback value", + desc: "empty value", + setup: setupEnv(envvar, ""), + fallback: "fallback value", + expectedValue: "fallback value", }, { - value: " ", - fallback: "fallback value", - expected: "", + desc: "whitespace only", + setup: setupEnv(envvar, " "), + fallback: "fallback value", + expectedValue: "", }, } { - t.Run(fmt.Sprintf("value=%s,fallback=%s", tc.value, tc.fallback), func(t *testing.T) { - testhelper.ModifyEnvironment(t, "TEST_STRING", tc.value) - - result := env.GetString("TEST_STRING", tc.fallback) + t.Run(tc.desc, func(t *testing.T) { + tc.setup(t) - assert.Equal(t, tc.expected, result) + value := env.GetString(envvar, tc.fallback) + require.Equal(t, tc.expectedValue, value) }) } } @@ -262,3 +342,9 @@ func TestExtractKey(t *testing.T) { }) } } + +func setupEnv(key, value string) func(t *testing.T) { + return func(t *testing.T) { + t.Setenv(key, value) + } +} -- cgit v1.2.3