Fail loudly on set-but-unparseable env config values (closes #80) (#92)
Some checks failed
check / check (push) Has been cancelled

Defaults now apply only to unset or empty environment variables; a set-but-
unparseable value aborts startup with an error naming the key and the value.
envInt is gone, envBool parses with strconv.ParseBool, and PORT is bounded.
This commit was merged in pull request #92.
This commit is contained in:
2026-08-10 16:06:12 +02:00
parent 0ce8565f51
commit 45890d4f82
5 changed files with 596 additions and 48 deletions

409
internal/config/env_test.go Normal file
View File

@@ -0,0 +1,409 @@
package config_test
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/logger"
)
// testEnvKey is a throwaway variable name used only by the helper
// tables below, so they cannot disturb real configuration.
const testEnvKey = "WEBHOOKER_TEST_VALUE"
// Real configuration variables exercised by the config.New tests.
const (
envKeyPort = "PORT"
envKeyDebug = "DEBUG"
envKeyMaintenanceMode = "MAINTENANCE_MODE"
)
// envBoolCase is one row of the envBool table.
type envBoolCase struct {
name string
set bool
value string
defaultValue bool
expectError bool
expected bool
}
// envBoolCases is the envBool table, kept out of the test body so
// the test itself stays readable.
func envBoolCases() []envBoolCase {
return []envBoolCase{
{
name: "unset uses default false",
defaultValue: false,
expected: false,
},
{
name: "unset uses default true",
defaultValue: true,
expected: true,
},
{
name: "empty uses default true",
set: true,
value: "",
defaultValue: true,
expected: true,
},
{
name: "true is parsed",
set: true,
value: "true",
expected: true,
},
{
name: "one is parsed",
set: true,
value: "1",
expected: true,
},
{
name: "False is parsed",
set: true,
value: "False",
defaultValue: true,
expected: false,
},
{
name: "zero is parsed",
set: true,
value: "0",
defaultValue: true,
expected: false,
},
{
name: "yes is rejected",
set: true,
value: "yes",
expectError: true,
},
{
name: "on is rejected",
set: true,
value: "on",
expectError: true,
},
{
name: "typo is rejected",
set: true,
value: "ture",
expectError: true,
},
}
}
func TestEnvBool(t *testing.T) {
for _, tt := range envBoolCases() {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvBoolForTest(
testEnvKey, tt.defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
assert.Contains(t, err.Error(), tt.value)
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
func TestEnvPositiveInt(t *testing.T) {
const defaultValue = 7
tests := []struct {
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{
name: "unset returns the default integer",
expected: defaultValue,
},
{
name: "empty returns the default integer",
set: true,
value: "",
expected: defaultValue,
},
{
name: "positive value is parsed",
set: true,
value: "42",
expected: 42,
},
{
name: "unparseable value is rejected",
set: true,
value: "not-a-number",
expectError: true,
},
{
name: "zero is rejected",
set: true,
value: "0",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
{
name: "negative is rejected",
set: true,
value: "-5",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvPositiveIntForTest(
testEnvKey, defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
assert.Contains(t, err.Error(), tt.value)
if tt.errIs != nil {
require.ErrorIs(t, err, tt.errIs)
}
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
func TestEnvPort(t *testing.T) {
const defaultValue = 8080
tests := []struct {
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{
name: "unset returns the default port",
expected: defaultValue,
},
{
name: "valid port is parsed",
set: true,
value: "9000",
expected: 9000,
},
{
name: "highest port is accepted",
set: true,
value: "65535",
expected: 65535,
},
{
name: "unparseable value is rejected",
set: true,
value: "not-a-port",
expectError: true,
},
{
name: "zero is rejected",
set: true,
value: "0",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
{
name: "above the port range is rejected",
set: true,
value: "65536",
expectError: true,
errIs: config.ErrInvalidPort,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvPortForTest(
testEnvKey, defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
if tt.errIs != nil {
require.ErrorIs(t, err, tt.errIs)
}
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
// buildConfig constructs a Config through fx exactly as the
// application does, returning the config and any construction error.
func buildConfig(t *testing.T) (*config.Config, error) {
t.Helper()
var cfg *config.Config
app := fx.New(
fx.NopLogger,
fx.Provide(
globals.New,
logger.New,
config.New,
),
fx.Populate(&cfg),
)
return cfg, app.Err()
}
func TestNewRejectsBadEnvValues(t *testing.T) {
tests := []struct {
name string
key string
value string
expectError bool
check func(t *testing.T, cfg *config.Config)
}{
{
name: "valid PORT is used",
key: envKeyPort,
value: "9001",
check: func(t *testing.T, cfg *config.Config) {
t.Helper()
assert.Equal(t, 9001, cfg.Port)
},
},
{
name: "unparseable PORT aborts startup",
key: envKeyPort,
value: "eighty-eighty",
expectError: true,
},
{
name: "out-of-range PORT aborts startup",
key: envKeyPort,
value: "70000",
expectError: true,
},
{
name: "valid DEBUG is used",
key: envKeyDebug,
value: "true",
check: func(t *testing.T, cfg *config.Config) {
t.Helper()
assert.True(t, cfg.Debug)
},
},
{
name: "unparseable DEBUG aborts startup",
key: envKeyDebug,
value: "ture",
expectError: true,
},
{
name: "unparseable MAINTENANCE_MODE aborts startup",
key: envKeyMaintenanceMode,
value: "sometimes",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
t.Setenv(tt.key, tt.value)
cfg, err := buildConfig(t)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.key)
assert.Contains(t, err.Error(), tt.value)
return
}
require.NoError(t, err)
require.NotNil(t, cfg)
tt.check(t, cfg)
})
}
}
// TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
// break the legitimate unset case: absent variables still get their
// documented defaults.
func TestNewUsesDefaultsWhenUnset(t *testing.T) {
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
for _, key := range []string{
envKeyPort, envKeyDebug, envKeyMaintenanceMode,
} {
require.NoError(t, os.Unsetenv(key))
}
cfg, err := buildConfig(t)
require.NoError(t, err)
require.NotNil(t, cfg)
assert.Equal(t, 8080, cfg.Port)
assert.False(t, cfg.Debug)
assert.False(t, cfg.MaintenanceMode)
}