feat(env): add type-safe environment variable utilities

jremy42 · jremy42 · commit ab1750307754 · 2025-11-21T10:44:06.000+01:00
diff --git a/internal/env/env.go b/internal/env/env.go
@@ -0,0 +1,85 @@
+// Package env provides utilities for reading environment variables with defaults and type safety.
+package env
+
+import (
+	"os"
+	"strconv"
+	"time"
+)
+
+// GetBool reads a boolean environment variable.
+// Returns defaultValue if the variable is not set or invalid.
+func GetBool(key string, defaultValue bool) bool {
+	value := os.Getenv(key)
+	if value == "" {
+		return defaultValue
+	}
+
+	switch value {
+	case "1", "true", "TRUE", "True", "yes", "YES", "Yes":
+		return true
+	case "0", "false", "FALSE", "False", "no", "NO", "No":
+		return false
+	default:
+		return defaultValue
+	}
+}
+
+// GetString reads a string environment variable.
+// Returns defaultValue if the variable is not set.
+func GetString(key string, defaultValue string) string {
+	value := os.Getenv(key)
+	if value == "" {
+		return defaultValue
+	}
+	return value
+}
+
+// GetInt reads an integer environment variable.
+// Returns defaultValue if the variable is not set or invalid.
+func GetInt(key string, defaultValue int) int {
+	value := os.Getenv(key)
+	if value == "" {
+		return defaultValue
+	}
+
+	intValue, err := strconv.Atoi(value)
+	if err != nil {
+		return defaultValue
+	}
+
+	return intValue
+}
+
+// GetDuration reads a duration environment variable.
+// Accepts values like "5s", "10m", "1h30m".
+// Returns defaultValue if the variable is not set or invalid.
+func GetDuration(key string, defaultValue time.Duration) time.Duration {
+	value := os.Getenv(key)
+	if value == "" {
+		return defaultValue
+	}
+
+	duration, err := time.ParseDuration(value)
+	if err != nil {
+		return defaultValue
+	}
+
+	return duration
+}
+
+// IsSet returns true if the environment variable is set (even if empty).
+func IsSet(key string) bool {
+	_, exists := os.LookupEnv(key)
+	return exists
+}
+
+// MustGetString reads a string environment variable and panics if not set.
+// Use this for required configuration values.
+func MustGetString(key string) string {
+	value := os.Getenv(key)
+	if value == "" {
+		panic("environment variable " + key + " is required but not set")
+	}
+	return value
+}
diff --git a/internal/env/env_test.go b/internal/env/env_test.go
@@ -0,0 +1,158 @@
+package env
+
+import (
+	"os"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetBool(t *testing.T) {
+	tests := []struct {
+		name         string
+		value        string
+		defaultValue bool
+		expected     bool
+	}{
+		{"true", "true", false, true},
+		{"TRUE", "TRUE", false, true},
+		{"1", "1", false, true},
+		{"yes", "yes", false, true},
+		{"false", "false", true, false},
+		{"FALSE", "FALSE", true, false},
+		{"0", "0", true, false},
+		{"no", "no", true, false},
+		{"empty", "", false, false},
+		{"empty_default_true", "", true, true},
+		{"invalid", "invalid", false, false},
+		{"invalid_default_true", "invalid", true, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key := "TEST_BOOL_" + tt.name
+			if tt.value != "" {
+				os.Setenv(key, tt.value)
+				defer os.Unsetenv(key)
+			}
+			result := GetBool(key, tt.defaultValue)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestGetString(t *testing.T) {
+	tests := []struct {
+		name         string
+		value        string
+		defaultValue string
+		expected     string
+	}{
+		{"set", "value", "default", "value"},
+		{"empty", "", "default", "default"},
+		{"spaces", "  spaces  ", "default", "  spaces  "},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key := "TEST_STRING_" + tt.name
+			if tt.value != "" {
+				os.Setenv(key, tt.value)
+				defer os.Unsetenv(key)
+			}
+			result := GetString(key, tt.defaultValue)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestGetInt(t *testing.T) {
+	tests := []struct {
+		name         string
+		value        string
+		defaultValue int
+		expected     int
+	}{
+		{"positive", "42", 0, 42},
+		{"negative", "-10", 0, -10},
+		{"zero", "0", 100, 0},
+		{"empty", "", 99, 99},
+		{"invalid", "not_a_number", 50, 50},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key := "TEST_INT_" + tt.name
+			if tt.value != "" {
+				os.Setenv(key, tt.value)
+				defer os.Unsetenv(key)
+			}
+			result := GetInt(key, tt.defaultValue)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestGetDuration(t *testing.T) {
+	tests := []struct {
+		name         string
+		value        string
+		defaultValue time.Duration
+		expected     time.Duration
+	}{
+		{"seconds", "5s", time.Minute, 5 * time.Second},
+		{"minutes", "10m", time.Second, 10 * time.Minute},
+		{"hours", "2h", time.Second, 2 * time.Hour},
+		{"combined", "1h30m", time.Second, 90 * time.Minute},
+		{"empty", "", time.Minute, time.Minute},
+		{"invalid", "invalid", 5 * time.Second, 5 * time.Second},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key := "TEST_DURATION_" + tt.name
+			if tt.value != "" {
+				os.Setenv(key, tt.value)
+				defer os.Unsetenv(key)
+			}
+			result := GetDuration(key, tt.defaultValue)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsSet(t *testing.T) {
+	key := "TEST_IS_SET"
+
+	// Not set
+	assert.False(t, IsSet(key))
+
+	// Set to empty
+	os.Setenv(key, "")
+	defer os.Unsetenv(key)
+	assert.True(t, IsSet(key))
+
+	// Set to value
+	os.Setenv(key, "value")
+	assert.True(t, IsSet(key))
+}
+
+func TestMustGetString(t *testing.T) {
+	t.Run("set", func(t *testing.T) {
+		key := "TEST_MUST_GET_SET"
+		os.Setenv(key, "value")
+		defer os.Unsetenv(key)
+
+		result := MustGetString(key)
+		assert.Equal(t, "value", result)
+	})
+
+	t.Run("not_set_panics", func(t *testing.T) {
+		key := "TEST_MUST_GET_NOT_SET"
+		require.Panics(t, func() {
+			MustGetString(key)
+		})
+	})
+}
diff --git a/internal/namespaces/rdb/v1/custom_benchmark_test.go b/internal/namespaces/rdb/v1/custom_benchmark_test.go
@@ -12,7 +12,9 @@ import (
 	"time"
 
 	"github.com/scaleway/scaleway-cli/v2/core"
+	"github.com/scaleway/scaleway-cli/v2/internal/env"
 	"github.com/scaleway/scaleway-cli/v2/internal/namespaces/rdb/v1"
+	"github.com/scaleway/scaleway-cli/v2/internal/testhelpers"
 	rdbSDK "github.com/scaleway/scaleway-sdk-go/api/rdb/v1"
 	"github.com/scaleway/scaleway-sdk-go/scw"
 )
@@ -21,13 +23,21 @@ import (
 //
 // Baseline stored in testdata/benchmark.baseline (like golden files).
 //
+// Install benchstat (required for comparison):
+//
+//	go install golang.org/x/perf/cmd/benchstat@latest
+//
 // To compare performance:
 //
 //	benchstat testdata/benchmark.baseline <(CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x .)
 //
 // To update baseline:
 //
 //	CLI_RUN_BENCHMARKS=true go test -bench=. -benchtime=100x . > testdata/benchmark.baseline
+//
+// Or use the automated tool (installs benchstat automatically):
+//
+//	go run ./cmd/scw-benchstat --install-benchstat --bench=. --count=10
 
 const (
 	defaultCmdTimeout    = 30 * time.Second
@@ -49,60 +59,7 @@ func TestMain(m *testing.M) {
 
 func setupBenchmark(b *testing.B) (*scw.Client, core.TestMetadata, func(args []string) any) {
 	b.Helper()
-
-	clientOpts := []scw.ClientOption{
-		scw.WithDefaultRegion(scw.RegionFrPar),
-		scw.WithDefaultZone(scw.ZoneFrPar1),
-		scw.WithUserAgent("cli-benchmark-test"),
-		scw.WithEnv(),
-	}
-
-	config, err := scw.LoadConfig()
-	if err == nil {
-		activeProfile, err := config.GetActiveProfile()
-		if err == nil {
-			envProfile := scw.LoadEnvProfile()
-			profile := scw.MergeProfiles(activeProfile, envProfile)
-			clientOpts = append(clientOpts, scw.WithProfile(profile))
-		}
-	}
-
-	client, err := scw.NewClient(clientOpts...)
-	if err != nil {
-		b.Fatalf(
-			"Failed to create Scaleway client: %v\nMake sure you have configured your credentials with 'scw config'",
-			err,
-		)
-	}
-
-	meta := core.TestMetadata{
-		"t": b,
-	}
-
-	executeCmd := func(args []string) any {
-		stdoutBuffer := &bytes.Buffer{}
-		stderrBuffer := &bytes.Buffer{}
-		_, result, err := core.Bootstrap(&core.BootstrapConfig{
-			Args:             args,
-			Commands:         rdb.GetCommands().Copy(),
-			BuildInfo:        nil,
-			Stdout:           stdoutBuffer,
-			Stderr:           stderrBuffer,
-			Client:           client,
-			DisableTelemetry: true,
-			DisableAliases:   true,
-			OverrideEnv:      map[string]string{},
-			Ctx:              context.Background(),
-		})
-		if err != nil {
-			b.Errorf("error executing cmd (%s): %v\nstdout: %s\nstderr: %s",
-				args, err, stdoutBuffer.String(), stderrBuffer.String())
-		}
-
-		return result
-	}
-
-	return client, meta, executeCmd
+	return testhelpers.SetupBenchmark(b, rdb.GetCommands())
 }
 
 func cleanupWithRetry(b *testing.B, name string, resourceID string, cleanupFn func() error) {
@@ -138,7 +95,7 @@ type benchmarkStats struct {
 
 func newBenchmarkStats() *benchmarkStats {
 	return &benchmarkStats{
-		enabled: os.Getenv("CLI_BENCH_TRACE") == "true",
+		enabled: env.GetBool("CLI_BENCH_TRACE", false),
 		timings: make([]time.Duration, 0, 1000),
 	}
 }
@@ -291,7 +248,7 @@ func cleanupSharedInstance() {
 }
 
 func BenchmarkInstanceGet(b *testing.B) {
-	if os.Getenv("CLI_RUN_BENCHMARKS") != "true" {
+	if !env.GetBool("CLI_RUN_BENCHMARKS", false) {
 		b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.")
 	}
 
@@ -327,7 +284,7 @@ func BenchmarkInstanceGet(b *testing.B) {
 }
 
 func BenchmarkBackupGet(b *testing.B) {
-	if os.Getenv("CLI_RUN_BENCHMARKS") != "true" {
+	if !env.GetBool("CLI_RUN_BENCHMARKS", false) {
 		b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.")
 	}
 
@@ -396,7 +353,7 @@ func BenchmarkBackupGet(b *testing.B) {
 }
 
 func BenchmarkBackupList(b *testing.B) {
-	if os.Getenv("CLI_RUN_BENCHMARKS") != "true" {
+	if !env.GetBool("CLI_RUN_BENCHMARKS", false) {
 		b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.")
 	}
 
@@ -478,7 +435,7 @@ func BenchmarkBackupList(b *testing.B) {
 }
 
 func BenchmarkDatabaseList(b *testing.B) {
-	if os.Getenv("CLI_RUN_BENCHMARKS") != "true" {
+	if !env.GetBool("CLI_RUN_BENCHMARKS", false) {
 		b.Skip("Skipping benchmark. Set CLI_RUN_BENCHMARKS=true to run.")
 	}
 
diff --git a/internal/testhelpers/helpers_benchmark.go b/internal/testhelpers/helpers_benchmark.go
