Isolate config tests from the shell; any out-of-range PORT is ErrInvalidPort (closes #94)
check / check (push) Waiting to run

Config tests, and the first-boot debug log test that builds a Config,
start from an empty environment: config.ClearEnvForTest unsets every
variable the process has and restores each when the test ends, so
nothing exported in the developer's shell changes a result.
TestEnvPositiveInt and TestEnvPort share one table runner, and
TestEnvPort checks that the bad value appears in each error.

Every out-of-range PORT now wraps ErrInvalidPort: zero, negatives,
above 65535, and numbers too large or too small for an int.

The README configuration table and the Settings page say that a
RETENTION_SWEEP_INTERVAL that does not parse, or is zero or negative,
fails startup.

Model: opus-5-5
This commit is contained in:
2026-10-02 14:50:03 +00:00
parent debe588bba
commit e72d08db13
9 changed files with 160 additions and 172 deletions
+3 -2
View File
@@ -139,7 +139,7 @@ TTY detection, and security headers are always applied.
| `METRICS_USERNAME` | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""` | | `METRICS_USERNAME` | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN. Unset leaves error reporting off; a value the Sentry SDK cannot parse fails startup rather than serving with reporting silently off | `""` | | `SENTRY_DSN` | Sentry error reporting DSN. Unset leaves error reporting off; a value the Sentry SDK cannot parse fails startup rather than serving with reporting silently off | `""` |
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` | | `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive). A value that does not parse, or is zero or negative, fails startup | `1h` |
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted. A set value replaces the default. If any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set it to the proxy's address alone. See [Trusted proxies](#trusted-proxies) | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16` (RFC 1918) | | `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted. A set value replaces the default. If any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set it to the proxy's address alone. See [Trusted proxies](#trusted-proxies) | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16` (RFC 1918) |
@@ -2924,7 +2924,8 @@ webhooker/
│ ├── resetpw/ │ ├── resetpw/
│ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only │ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only
│ ├── config/ │ ├── config/
│ │ └── config.go # Configuration loading from environment variables │ │ ├── config.go # Configuration loading from environment variables
│ │ └── testing.go # ClearEnvForTest: an empty environment for one test
│ ├── database/ │ ├── database/
│ │ ├── base_model.go # BaseModel with UUID primary keys │ │ ├── base_model.go # BaseModel with UUID primary keys
│ │ ├── database.go # GORM connection, migrations, admin seed │ │ ├── database.go # GORM connection, migrations, admin seed
+19 -10
View File
@@ -80,8 +80,7 @@ const (
// process over a Docker network or a private LAN connects from. // process over a Docker network or a private LAN connects from.
defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
// maxPort is the highest valid TCP port number. The lower // maxPort is the highest valid TCP port number.
// bound (at least 1) is enforced by envPositiveInt.
maxPort = 65535 maxPort = 65535
// mappedV4Offset is the number of leading bits an IPv4-mapped // mappedV4Offset is the number of leading bits an IPv4-mapped
@@ -105,7 +104,7 @@ var ErrInvalidEnvironment = errors.New("invalid environment")
var ErrNonPositiveValue = errors.New("value must be positive") var ErrNonPositiveValue = errors.New("value must be positive")
// ErrInvalidPort is returned when an environment variable holding a // ErrInvalidPort is returned when an environment variable holding a
// TCP port number is set above the valid port range. // TCP port number is set to a number outside 1 to 65535.
var ErrInvalidPort = errors.New("invalid port") var ErrInvalidPort = errors.New("invalid port")
// ErrInvalidCIDR is returned when an environment variable holding a // ErrInvalidCIDR is returned when an environment variable holding a
@@ -363,17 +362,27 @@ func envPositiveInt(
// envPort returns the value of the named environment variable parsed // envPort returns the value of the named environment variable parsed
// as a TCP port number. Returns defaultValue if not set. A set value // as a TCP port number. Returns defaultValue if not set. A set value
// that is unparseable, below 1, or above maxPort is a hard error // that is unparseable, below 1, or above maxPort is a hard error
// naming the key and the bad value. // naming the key and the bad value; every out-of-range value wraps
// ErrInvalidPort, including one too large or too small for an int.
func envPort(key string, defaultValue int) (int, error) { func envPort(key string, defaultValue int) (int, error) {
port, err := envPositiveInt(key, defaultValue) v := os.Getenv(key)
if err != nil { if v == "" {
return 0, err return defaultValue, nil
} }
if port > maxPort { // strconv.ErrRange means a number too large or too small for an
// int, which is outside the port range as well.
port, err := strconv.Atoi(v)
if err != nil && !errors.Is(err, strconv.ErrRange) {
return 0, fmt.Errorf( return 0, fmt.Errorf(
"%w: %s must be at most %d, got %d", "invalid integer for %s: %q: %w", key, v, err,
ErrInvalidPort, key, maxPort, port, )
}
if err != nil || port < 1 || port > maxPort {
return 0, fmt.Errorf(
"%w: %s must be from 1 to %d, got %q",
ErrInvalidPort, key, maxPort, v,
) )
} }
+16 -45
View File
@@ -3,7 +3,6 @@ package config_test
import ( import (
"bytes" "bytes"
"log/slog" "log/slog"
"os"
"testing" "testing"
"time" "time"
@@ -71,14 +70,12 @@ func TestEnvironmentConfig(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if tt.envValue != "" { if tt.envValue != "" {
t.Setenv( t.Setenv(
"WEBHOOKER_ENVIRONMENT", tt.envValue, "WEBHOOKER_ENVIRONMENT", tt.envValue,
) )
} else {
require.NoError(t, os.Unsetenv(
"WEBHOOKER_ENVIRONMENT",
))
} }
for k, v := range tt.envVars { for k, v := range tt.envVars {
@@ -199,14 +196,11 @@ func TestRetentionSweepInterval(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set { if tt.set {
t.Setenv("RETENTION_SWEEP_INTERVAL", tt.value) t.Setenv("RETENTION_SWEEP_INTERVAL", tt.value)
} else {
require.NoError(t, os.Unsetenv(
"RETENTION_SWEEP_INTERVAL",
))
} }
if tt.expectError { if tt.expectError {
@@ -341,14 +335,11 @@ func TestSessionIdleTimeout(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set { if tt.set {
t.Setenv("SESSION_IDLE_TIMEOUT", tt.value) t.Setenv("SESSION_IDLE_TIMEOUT", tt.value)
} else {
require.NoError(t, os.Unsetenv(
"SESSION_IDLE_TIMEOUT",
))
} }
if tt.expectError { if tt.expectError {
@@ -397,16 +388,12 @@ func TestDefaultDataDir(t *testing.T) {
t.Run("env="+name, func(t *testing.T) { t.Run("env="+name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if env != "" { if env != "" {
t.Setenv("WEBHOOKER_ENVIRONMENT", env) t.Setenv("WEBHOOKER_ENVIRONMENT", env)
} else {
require.NoError(t, os.Unsetenv(
"WEBHOOKER_ENVIRONMENT",
))
} }
require.NoError(t, os.Unsetenv("DATA_DIR"))
var cfg *config.Config var cfg *config.Config
app := fxtest.New( app := fxtest.New(
@@ -446,9 +433,9 @@ func TestDataDirHelper(t *testing.T) {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
if set == "" { config.ClearEnvForTest(t)
require.NoError(t, os.Unsetenv("DATA_DIR"))
} else { if set != "" {
t.Setenv("DATA_DIR", set) t.Setenv("DATA_DIR", set)
} }
@@ -511,14 +498,11 @@ func TestReceiverRateLimit(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set { if tt.set {
t.Setenv("RECEIVER_RATE_LIMIT", tt.value) t.Setenv("RECEIVER_RATE_LIMIT", tt.value)
} else {
require.NoError(t, os.Unsetenv(
"RECEIVER_RATE_LIMIT",
))
} }
if tt.expectError { if tt.expectError {
@@ -630,12 +614,11 @@ func TestTrustedProxies(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set { if tt.set {
t.Setenv("TRUSTED_PROXIES", tt.value) t.Setenv("TRUSTED_PROXIES", tt.value)
} else {
require.NoError(t, os.Unsetenv("TRUSTED_PROXIES"))
} }
if tt.expectError { if tt.expectError {
@@ -742,14 +725,11 @@ func TestAllowedEgressCIDRs(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set { if tt.set {
t.Setenv("ALLOWED_EGRESS_CIDRS", tt.value) t.Setenv("ALLOWED_EGRESS_CIDRS", tt.value)
} else {
require.NoError(
t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"),
)
} }
if tt.expectError { if tt.expectError {
@@ -817,13 +797,10 @@ func TestEgressAllowlistWarning(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", config.EnvironmentDev) t.Setenv("WEBHOOKER_ENVIRONMENT", config.EnvironmentDev)
if tt.allowed == "" { if tt.allowed != "" {
require.NoError(
t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"),
)
} else {
t.Setenv("ALLOWED_EGRESS_CIDRS", tt.allowed) t.Setenv("ALLOWED_EGRESS_CIDRS", tt.allowed)
} }
@@ -956,20 +933,14 @@ func TestMetricsAuthConfig(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if tt.username.set { if tt.username.set {
t.Setenv("METRICS_USERNAME", tt.username.value) t.Setenv("METRICS_USERNAME", tt.username.value)
} else {
require.NoError(
t, os.Unsetenv("METRICS_USERNAME"),
)
} }
if tt.password.set { if tt.password.set {
t.Setenv("METRICS_PASSWORD", tt.password.value) t.Setenv("METRICS_PASSWORD", tt.password.value)
} else {
require.NoError(
t, os.Unsetenv("METRICS_PASSWORD"),
)
} }
if tt.expectError { if tt.expectError {
+7 -18
View File
@@ -22,17 +22,6 @@ const malformedDotEnv = "PORT 19615\n" +
"this is not = valid ! syntax\n" + "this is not = valid ! syntax\n" +
"\"unclosed\n" "\"unclosed\n"
// unsetDotEnvKey makes dotEnvKey genuinely absent for the duration of
// the test and restores it afterwards. t.Setenv registers the restore;
// the Unsetenv that follows is what the test actually needs, because a
// variable set to the empty string is still present in os.Environ and
// godotenv would refuse to overwrite it.
func unsetDotEnvKey(t *testing.T) {
t.Helper()
t.Setenv(dotEnvKey, "placeholder")
require.NoError(t, os.Unsetenv(dotEnvKey))
}
// writeDotEnv writes contents to a .env file in a fresh temporary // writeDotEnv writes contents to a .env file in a fresh temporary
// directory and returns its path. // directory and returns its path.
func writeDotEnv(t *testing.T, contents string) string { func writeDotEnv(t *testing.T, contents string) string {
@@ -50,9 +39,9 @@ func writeDotEnv(t *testing.T, contents string) string {
// normally rather than be refused for a file it was never meant to // normally rather than be refused for a file it was never meant to
// have. // have.
// //
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv. //nolint:paralleltest // ClearEnvForTest uses t.Setenv.
func TestLoadDotEnv_MissingFileIsFine(t *testing.T) { func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
unsetDotEnvKey(t) config.ClearEnvForTest(t)
absent := filepath.Join(t.TempDir(), config.DotEnvPath) absent := filepath.Join(t.TempDir(), config.DotEnvPath)
require.NoError(t, config.LoadDotEnvFileForTest(absent)) require.NoError(t, config.LoadDotEnvFileForTest(absent))
@@ -65,9 +54,9 @@ func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
// reaches the environment, which is the whole reason the file is read // reaches the environment, which is the whole reason the file is read
// at all. // at all.
// //
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv. //nolint:paralleltest // ClearEnvForTest uses t.Setenv.
func TestLoadDotEnv_AppliesValues(t *testing.T) { func TestLoadDotEnv_AppliesValues(t *testing.T) {
unsetDotEnvKey(t) config.ClearEnvForTest(t)
path := writeDotEnv(t, "# a comment\n"+dotEnvKey+"=from-dot-env\n") path := writeDotEnv(t, "# a comment\n"+dotEnvKey+"=from-dot-env\n")
@@ -93,9 +82,9 @@ func TestLoadDotEnv_RealEnvironmentWins(t *testing.T) {
// reverts to its default; the process used to start that way with no // reverts to its default; the process used to start that way with no
// log line naming the file at all. // log line naming the file at all.
// //
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv. //nolint:paralleltest // ClearEnvForTest uses t.Setenv.
func TestLoadDotEnv_MalformedFileAborts(t *testing.T) { func TestLoadDotEnv_MalformedFileAborts(t *testing.T) {
unsetDotEnvKey(t) config.ClearEnvForTest(t)
path := writeDotEnv( path := writeDotEnv(
t, malformedDotEnv+dotEnvKey+"=from-dot-env\n", t, malformedDotEnv+dotEnvKey+"=from-dot-env\n",
@@ -143,7 +132,7 @@ func TestLoadDotEnv_UnreadableFileAborts(t *testing.T) {
// //
//nolint:paralleltest // t.Chdir moves the whole process. //nolint:paralleltest // t.Chdir moves the whole process.
func TestLoadDotEnv_ReadsTheWorkingDirectory(t *testing.T) { func TestLoadDotEnv_ReadsTheWorkingDirectory(t *testing.T) {
unsetDotEnvKey(t) config.ClearEnvForTest(t)
dir := t.TempDir() dir := t.TempDir()
require.NoError(t, os.WriteFile( require.NoError(t, os.WriteFile(
+78 -91
View File
@@ -1,7 +1,6 @@
package config_test package config_test
import ( import (
"os"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -121,10 +120,10 @@ func TestEnvBool(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if tt.set { if tt.set {
t.Setenv(testEnvKey, tt.value) t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
} }
got, err := config.EnvBoolForTest( got, err := config.EnvBoolForTest(
@@ -145,17 +144,62 @@ func TestEnvBool(t *testing.T) {
} }
} }
// envIntCase is one row of the envPositiveInt and envPort tables.
type envIntCase struct {
name string
set bool
value string
expectError bool
errIs error
expected int
}
// runEnvIntCases runs each row through parse, which is
// envPositiveInt or envPort, with testEnvKey set to the row's value
// or left unset.
func runEnvIntCases(
t *testing.T,
parse func(key string, defaultValue int) (int, error),
defaultValue int,
tests []envIntCase,
) {
t.Helper()
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.
config.ClearEnvForTest(t)
if tt.set {
t.Setenv(testEnvKey, tt.value)
}
got, err := parse(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)
})
}
}
//nolint:paralleltest // runEnvIntCases uses t.Setenv.
func TestEnvPositiveInt(t *testing.T) { func TestEnvPositiveInt(t *testing.T) {
const defaultValue = 7 const defaultValue = 7
tests := []struct { runEnvIntCases(t, config.EnvPositiveIntForTest, defaultValue, []envIntCase{
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{ {
name: "unset returns the default integer", name: "unset returns the default integer",
expected: defaultValue, expected: defaultValue,
@@ -192,51 +236,14 @@ func TestEnvPositiveInt(t *testing.T) {
expectError: true, expectError: true,
errIs: config.ErrNonPositiveValue, 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)
})
}
} }
//nolint:paralleltest // runEnvIntCases uses t.Setenv.
func TestEnvPort(t *testing.T) { func TestEnvPort(t *testing.T) {
const defaultValue = 8080 const defaultValue = 8080
tests := []struct { runEnvIntCases(t, config.EnvPortForTest, defaultValue, []envIntCase{
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{ {
name: "unset returns the default port", name: "unset returns the default port",
expected: defaultValue, expected: defaultValue,
@@ -264,7 +271,14 @@ func TestEnvPort(t *testing.T) {
set: true, set: true,
value: "0", value: "0",
expectError: true, expectError: true,
errIs: config.ErrNonPositiveValue, errIs: config.ErrInvalidPort,
},
{
name: "negative is rejected",
set: true,
value: "-1",
expectError: true,
errIs: config.ErrInvalidPort,
}, },
{ {
name: "above the port range is rejected", name: "above the port range is rejected",
@@ -273,37 +287,14 @@ func TestEnvPort(t *testing.T) {
expectError: true, expectError: true,
errIs: config.ErrInvalidPort, errIs: config.ErrInvalidPort,
}, },
} {
name: "too large for an int is rejected",
for _, tt := range tests { set: true,
t.Run(tt.name, func(t *testing.T) { value: "99999999999999999999",
// Cannot use t.Parallel() here because t.Setenv expectError: true,
// is incompatible with parallel subtests. errIs: config.ErrInvalidPort,
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)
})
}
} }
// TestEnvBindAddress covers BIND_ADDRESS parsing. // TestEnvBindAddress covers BIND_ADDRESS parsing.
@@ -319,10 +310,10 @@ func TestEnvBindAddress(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if tt.set { if tt.set {
t.Setenv(testEnvKey, tt.value) t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
} }
got, err := config.EnvBindAddressForTest( got, err := config.EnvBindAddressForTest(
@@ -485,6 +476,7 @@ func TestNewRejectsBadEnvValues(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
t.Setenv(tt.key, tt.value) t.Setenv(tt.key, tt.value)
@@ -646,14 +638,9 @@ func sentryEnvValueCases() []badEnvValueCase {
// break the legitimate unset case: absent variables still get their // break the legitimate unset case: absent variables still get their
// documented defaults. // documented defaults.
func TestNewUsesDefaultsWhenUnset(t *testing.T) { func TestNewUsesDefaultsWhenUnset(t *testing.T) {
config.ClearEnvForTest(t)
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
for _, key := range []string{
envKeyPort, envKeyDebug, envKeyBindAddress, envKeySentryDSN,
} {
require.NoError(t, os.Unsetenv(key))
}
cfg, err := buildConfig(t) cfg, err := buildConfig(t)
require.NoError(t, err) require.NoError(t, err)
require.NotNil(t, cfg) require.NotNil(t, cfg)
+2 -3
View File
@@ -1,7 +1,6 @@
package config_test package config_test
import ( import (
"os"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -101,10 +100,10 @@ func TestEnvSentryDSN(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv // Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests. // is incompatible with parallel subtests.
config.ClearEnvForTest(t)
if tt.set { if tt.set {
t.Setenv(envKeySentryDSN, tt.value) t.Setenv(envKeySentryDSN, tt.value)
} else {
require.NoError(t, os.Unsetenv(envKeySentryDSN))
} }
got, err := config.EnvSentryDSNForTest(envKeySentryDSN) got, err := config.EnvSentryDSNForTest(envKeySentryDSN)
+30
View File
@@ -0,0 +1,30 @@
package config
import (
"os"
"strings"
"testing"
)
// ClearEnvForTest unsets every variable in the process environment
// for the rest of the test and puts each back when the test ends, so
// a test sees only the variables it sets itself, not whatever the
// developer's shell exports.
func ClearEnvForTest(t *testing.T) {
t.Helper()
for _, entry := range os.Environ() {
key, _, _ := strings.Cut(entry, "=")
// t.Setenv registers the restore; the Unsetenv after it is
// what makes the key absent, since a key set to the empty
// string is still present, and godotenv will not overwrite a
// present key.
t.Setenv(key, "")
err := os.Unsetenv(key)
if err != nil {
t.Fatalf("unsetting %s: %v", key, err)
}
}
}
+3 -2
View File
@@ -117,8 +117,8 @@ func readFirstBootSecrets(
} }
// bootAtDebug starts and stops the real application graph against // bootAtDebug starts and stops the real application graph against
// dataDir with DEBUG=true, and returns everything it wrote to standard // dataDir with DEBUG=true and nothing else set, and returns everything
// output. // it wrote to standard output.
// //
// config.New reads DEBUG from the environment exactly as the binary // config.New reads DEBUG from the environment exactly as the binary
// does, internal/logger builds the handler it builds in production, // does, internal/logger builds the handler it builds in production,
@@ -128,6 +128,7 @@ func readFirstBootSecrets(
func bootAtDebug(t *testing.T, dataDir string) string { func bootAtDebug(t *testing.T, dataDir string) string {
t.Helper() t.Helper()
config.ClearEnvForTest(t)
t.Setenv("DEBUG", "true") t.Setenv("DEBUG", "true")
t.Setenv("DATA_DIR", dataDir) t.Setenv("DATA_DIR", dataDir)
+2 -1
View File
@@ -77,7 +77,8 @@ func settingRows(cfg *config.Config) []settingRow {
{ {
"RETENTION_SWEEP_INTERVAL", "RETENTION_SWEEP_INTERVAL",
"How often the retention reaper and archive sweeper run " + "How often the retention reaper and archive sweeper run " +
"(Go duration, must be positive)", "(Go duration, must be positive). A value that does " +
"not parse, or is zero or negative, fails startup",
cfg.RetentionSweepInterval.String(), cfg.RetentionSweepInterval.String(),
}, },
{ {