From 2a47be454da37b5baae8637a72bcaabff7e97f477b24eba8bd7a1cec01403cd1 Mon Sep 17 00:00:00 2001 From: Lixen Wraith Date: Tue, 29 Sep 2026 01:11:44 -0400 Subject: [PATCH] v0.2.1 remove testify dep --- README.md | 3 +- builder_test.go | 130 +++++++++-------- config_test.go | 292 +++++++++++++++++++++++--------------- decode.go | 4 +- decode_test.go | 80 +++++------ doc/audit.md | 66 --------- dynamic_test.go | 61 ++++---- go.mod | 11 +- go.sum | 10 -- hardening_test.go | 6 +- loader_test.go | 205 +++++++++++++------------- test_helpers_test.go | 89 ++++++++++++ typed_defaults_test.go | 57 ++++++++ utility_test.go | 249 +++++++++++++++++--------------- validator_test.go | 162 +++++++-------------- watch_test.go | 273 +++++++++++------------------------ watcher_hardening_test.go | 6 +- 17 files changed, 849 insertions(+), 855 deletions(-) delete mode 100644 doc/audit.md create mode 100644 test_helpers_test.go create mode 100644 typed_defaults_test.go diff --git a/README.md b/README.md index ca8d427..3ac5e6c 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,7 @@ Thread-safe configuration from TOML files, environment variables, command-line arguments and registered defaults. Requires Go 1.27.1. The only runtime dependency -is [lixenwraith/toml](https://github.com/lixenwraith/toml), pinned to merged commit -`b59d068ff6a4`. Testify and its dependencies are used only by tests. +is [lixenwraith/toml](https://github.com/lixenwraith/toml). ```go package main diff --git a/builder_test.go b/builder_test.go index 96288ad..f7c3b4c 100644 --- a/builder_test.go +++ b/builder_test.go @@ -3,12 +3,8 @@ package config import ( "fmt" - "os" "path/filepath" "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestBuilder tests the builder pattern @@ -29,18 +25,22 @@ func TestBuilder(t *testing.T) { WithEnvPrefix("TEST_"). Build() - require.NoError(t, err) - assert.NotNil(t, cfg) + mustNoError(t, err) + if got := cfg; got == nil { + t.Errorf("got %v, want non-nil", got) + } val, exists := cfg.Get("host") - assert.True(t, exists) - assert.Equal(t, "localhost", val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, "localhost") }) t.Run("BuilderWithAllOptions", func(t *testing.T) { tmpDir := t.TempDir() configFile := filepath.Join(tmpDir, "test.toml") - os.WriteFile(configFile, []byte(`host = "filehost"`), 0644) + writeTestFile(t, configFile, []byte(`host = "filehost"`), 0644) type Config struct { Host string `toml:"hostname"` @@ -69,11 +69,11 @@ func TestBuilder(t *testing.T) { WithEnvWhitelist("server.hostname"). Build() - require.NoError(t, err) + mustNoError(t, err) // CLI should take precedence val, _ := cfg.Get("server.hostname") - assert.Equal(t, "clihost", val) + checkEqual(t, val, "clihost") }) t.Run("BuilderWithTarget", func(t *testing.T) { @@ -96,18 +96,24 @@ func TestBuilder(t *testing.T) { WithTarget(target). Build() - require.NoError(t, err) + mustNoError(t, err) // Verify paths were registered paths := cfg.GetRegisteredPaths() - assert.True(t, paths["db.host"]) - assert.True(t, paths["db.port"]) - assert.True(t, paths["cache.ttl"]) + if !paths["db.host"] { + t.Errorf("paths[\"db.host\"] should be true") + } + if !paths["db.port"] { + t.Errorf("paths[\"db.port\"] should be true") + } + if !paths["cache.ttl"] { + t.Errorf("paths[\"cache.ttl\"] should be true") + } // Test AsStruct result, err := cfg.AsStruct() - require.NoError(t, err) - assert.Equal(t, target, result) + mustNoError(t, err) + checkEqual(t, result, target) }) t.Run("BuilderWithValidator", func(t *testing.T) { @@ -145,9 +151,13 @@ func TestBuilder(t *testing.T) { WithValidator(validator). Build() - require.NoError(t, err) - assert.NotNil(t, cfg) - assert.True(t, validatorCalled) + mustNoError(t, err) + if got := cfg; got == nil { + t.Errorf("got %v, want non-nil", got) + } + if !validatorCalled { + t.Errorf("validatorCalled should be true") + } // Invalid case validatorCalled = false @@ -156,10 +166,13 @@ func TestBuilder(t *testing.T) { WithValidator(validator). Build() - assert.Nil(t, cfg2) - assert.Error(t, err) - assert.Contains(t, err.Error(), "configuration validation failed") - assert.True(t, validatorCalled) + if got := cfg2; got != nil { + t.Errorf("got %v, want nil", got) + } + checkErrorContains(t, err, "configuration validation failed") + if !validatorCalled { + t.Errorf("validatorCalled should be true") + } }) t.Run("BuilderErrorAccumulation", func(t *testing.T) { @@ -169,29 +182,29 @@ func TestBuilder(t *testing.T) { WithDefaults(struct{}{}). Build() - assert.Error(t, err) - assert.Contains(t, err.Error(), "unsupported tag name") + checkErrorContains(t, err, "unsupported tag name") // Invalid target _, err = NewBuilder(). WithTarget("not-a-pointer"). Build() - assert.Error(t, err) - assert.Contains(t, err.Error(), "requires non-nil pointer to struct") + checkErrorContains(t, err, "requires non-nil pointer to struct") }) t.Run("MustBuildPanic", func(t *testing.T) { // Should not panic with valid config - assert.NotPanics(t, func() { + { cfg := NewBuilder(). WithDefaults(struct{ Port int }{Port: 8080}). MustBuild() - assert.NotNil(t, cfg) - }) + if got := cfg; got == nil { + t.Errorf("got %v, want non-nil", got) + } + } // Should panic with error - assert.Panics(t, func() { + mustPanic(t, func() { NewBuilder(). WithTagName("invalid"). MustBuild() @@ -205,7 +218,7 @@ func TestFileDiscovery(t *testing.T) { tmpDir := t.TempDir() // Use .toml extension for TOML content configFile := filepath.Join(tmpDir, "custom.toml") - os.WriteFile(configFile, []byte(`test = "value"`), 0644) + writeTestFile(t, configFile, []byte(`test = "value"`), 0644) opts := DefaultDiscoveryOptions("myapp") @@ -217,20 +230,19 @@ func TestFileDiscovery(t *testing.T) { WithFileDiscovery(opts). Build() - require.NoError(t, err) + mustNoError(t, err) // Verify file was loaded val, _ := cfg.Get("test") - assert.Equal(t, "value", val) + checkEqual(t, val, "value") }) t.Run("DiscoveryWithEnvVar", func(t *testing.T) { tmpDir := t.TempDir() configFile := filepath.Join(tmpDir, "env.toml") - os.WriteFile(configFile, []byte(`test = "envvalue"`), 0644) + writeTestFile(t, configFile, []byte(`test = "envvalue"`), 0644) - os.Setenv("MYAPP_CONFIG", configFile) - defer os.Unsetenv("MYAPP_CONFIG") + t.Setenv("MYAPP_CONFIG", configFile) opts := DefaultDiscoveryOptions("myapp") @@ -241,18 +253,17 @@ func TestFileDiscovery(t *testing.T) { WithFileDiscovery(opts). Build() - require.NoError(t, err) + mustNoError(t, err) val, _ := cfg.Get("test") - assert.Equal(t, "envvalue", val) + checkEqual(t, val, "envvalue") }) t.Run("DiscoveryInCurrentDir", func(t *testing.T) { - // Create config in current directory - cwd, _ := os.Getwd() - configFile := filepath.Join(cwd, "myapp.toml") - os.WriteFile(configFile, []byte(`test = "cwdvalue"`), 0644) - defer os.Remove(configFile) + // Discovery runs in an isolated directory and never writes into the checkout. + t.Chdir(t.TempDir()) + configFile := "myapp.toml" + writeTestFile(t, configFile, []byte(`test = "cwdvalue"`), 0644) opts := FileDiscoveryOptions{ Name: "myapp", @@ -267,10 +278,10 @@ func TestFileDiscovery(t *testing.T) { WithFileDiscovery(opts). Build() - require.NoError(t, err) + mustNoError(t, err) val, _ := cfg.Get("test") - assert.Equal(t, "cwdvalue", val) + checkEqual(t, val, "cwdvalue") }) t.Run("DiscoveryPrecedence", func(t *testing.T) { @@ -279,12 +290,11 @@ func TestFileDiscovery(t *testing.T) { // Create multiple config files cliFile := filepath.Join(tmpDir, "cli.toml") envFile := filepath.Join(tmpDir, "env.toml") - os.WriteFile(cliFile, []byte(`test = "clifile"`), 0644) - os.WriteFile(envFile, []byte(`test = "envfile"`), 0644) + writeTestFile(t, cliFile, []byte(`test = "clifile"`), 0644) + writeTestFile(t, envFile, []byte(`test = "envfile"`), 0644) // CLI should take precedence over env - os.Setenv("MYAPP_CONFIG", envFile) - defer os.Unsetenv("MYAPP_CONFIG") + t.Setenv("MYAPP_CONFIG", envFile) opts := DefaultDiscoveryOptions("myapp") @@ -296,10 +306,10 @@ func TestFileDiscovery(t *testing.T) { WithFileDiscovery(opts). Build() - require.NoError(t, err) + mustNoError(t, err) val, _ := cfg.Get("test") - assert.Equal(t, "clifile", val) + checkEqual(t, val, "clifile") }) } @@ -323,7 +333,7 @@ func TestBuilderWithTypedValidator(t *testing.T) { WithTypedValidator(validator). Build() - require.NoError(t, err) + mustNoError(t, err) }) // Case 2: Invalid configuration @@ -341,8 +351,10 @@ func TestBuilderWithTypedValidator(t *testing.T) { WithTypedValidator(validator). Build() - require.Error(t, err) - assert.Contains(t, err.Error(), "typed configuration validation failed: port too low") + if err == nil { + t.Fatalf("expected an error") + } + checkErrorContains(t, err, "typed configuration validation failed: port too low") }) // Case 3: Mismatched validator signature @@ -357,7 +369,9 @@ func TestBuilderWithTypedValidator(t *testing.T) { WithTypedValidator(validator). Build() - require.Error(t, err) - assert.Contains(t, err.Error(), "typed validator signature") + if err == nil { + t.Fatalf("expected an error") + } + checkErrorContains(t, err, "typed validator signature") }) } diff --git a/config_test.go b/config_test.go index d3a3623..3be8709 100644 --- a/config_test.go +++ b/config_test.go @@ -5,23 +5,23 @@ import ( "fmt" "net" "net/url" - "os" "path/filepath" "sync" "testing" "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestConfigCreation tests various config creation patterns func TestConfigCreation(t *testing.T) { t.Run("NewWithDefaultOptions", func(t *testing.T) { cfg := New() - require.NotNil(t, cfg) - assert.NotNil(t, cfg.items) - assert.Equal(t, []Source{SourceCLI, SourceEnv, SourceFile, SourceDefault}, cfg.options.Sources) + if got := cfg; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + if got := cfg.items; got == nil { + t.Errorf("got %v, want non-nil", got) + } + checkEqual(t, cfg.options.Sources, []Source{SourceCLI, SourceEnv, SourceFile, SourceDefault}) }) t.Run("NewWithCustomOptions", func(t *testing.T) { @@ -30,9 +30,11 @@ func TestConfigCreation(t *testing.T) { EnvPrefix: "MYAPP_", } cfg := NewWithOptions(opts) - require.NotNil(t, cfg) - assert.Equal(t, opts.Sources, cfg.options.Sources) - assert.Equal(t, "MYAPP_", cfg.options.EnvPrefix) + if got := cfg; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, cfg.options.Sources, opts.Sources) + checkEqual(t, cfg.options.EnvPrefix, "MYAPP_") }) } @@ -61,13 +63,16 @@ func TestPathRegistration(t *testing.T) { cfg := New() err := cfg.Register(tt.path, tt.defaultVal) if tt.expectError { - assert.Error(t, err) - assert.Contains(t, err.Error(), tt.errorMsg) + checkErrorContains(t, err, tt.errorMsg) } else { - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } val, exists := cfg.Get(tt.path) - assert.True(t, exists) - assert.Equal(t, tt.defaultVal, val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, tt.defaultVal) } }) } @@ -106,45 +111,68 @@ func TestComplexStructRegistration(t *testing.T) { t.Run("TOMLTags", func(t *testing.T) { cfg := New() err := cfg.RegisterStruct("", defaultConfig) - require.NoError(t, err) + mustNoError(t, err) // Verify paths registered with TOML tags paths := cfg.GetRegisteredPaths("") - assert.True(t, paths["name"]) - assert.True(t, paths["db.host"]) - assert.True(t, paths["db.port"]) - assert.True(t, paths["db.max_connections"]) - assert.True(t, paths["db.timeout"]) - assert.True(t, paths["db.debug"]) - assert.True(t, paths["tags"]) - assert.True(t, paths["metadata"]) + if !paths["name"] { + t.Errorf("paths[\"name\"] should be true") + } + if !paths["db.host"] { + t.Errorf("paths[\"db.host\"] should be true") + } + if !paths["db.port"] { + t.Errorf("paths[\"db.port\"] should be true") + } + if !paths["db.max_connections"] { + t.Errorf("paths[\"db.max_connections\"] should be true") + } + if !paths["db.timeout"] { + t.Errorf("paths[\"db.timeout\"] should be true") + } + if !paths["db.debug"] { + t.Errorf("paths[\"db.debug\"] should be true") + } + if !paths["tags"] { + t.Errorf("paths[\"tags\"] should be true") + } + if !paths["metadata"] { + t.Errorf("paths[\"metadata\"] should be true") + } // Verify default values val, _ := cfg.Get("db.timeout") - assert.Equal(t, 30*time.Second, val) + checkEqual(t, val, 30*time.Second) }) t.Run("RejectJSONTags", func(t *testing.T) { cfg := New() - require.Error(t, cfg.RegisterStructWithTags("", defaultConfig, "json")) - require.Empty(t, cfg.GetRegisteredPaths()) + if err := cfg.RegisterStructWithTags("", defaultConfig, "json"); err == nil { + t.Fatalf("expected an error") + } + if got := len(cfg.GetRegisteredPaths()); got != 0 { + t.Fatalf("length = %d, want %d", got, 0) + } }) t.Run("UnsupportedTag", func(t *testing.T) { cfg := New() err := cfg.RegisterStructWithTags("", defaultConfig, "xml") - assert.Error(t, err) - assert.Contains(t, err.Error(), "unsupported tag name") + checkErrorContains(t, err, "unsupported tag name") }) t.Run("WithPrefix", func(t *testing.T) { cfg := New() err := cfg.RegisterStruct("server", defaultConfig) - require.NoError(t, err) + mustNoError(t, err) paths := cfg.GetRegisteredPaths("server.") - assert.True(t, paths["server.name"]) - assert.True(t, paths["server.db.host"]) + if !paths["server.name"] { + t.Errorf("paths[\"server.name\"] should be true") + } + if !paths["server.db.host"] { + t.Errorf("paths[\"server.db.host\"] should be true") + } }) } @@ -160,24 +188,24 @@ func TestSourcePrecedence(t *testing.T) { // Default precedence: CLI > Env > File > Default val, _ := cfg.Get("test.value") - assert.Equal(t, "from-cli", val) + checkEqual(t, val, "from-cli") // Remove CLI value cfg.ResetSource(SourceCLI) val, _ = cfg.Get("test.value") - assert.Equal(t, "from-env", val) + checkEqual(t, val, "from-env") // Change precedence cfg.SetLoadOptions(LoadOptions{ Sources: []Source{SourceFile, SourceEnv, SourceCLI, SourceDefault}, }) val, _ = cfg.Get("test.value") - assert.Equal(t, "from-file", val) + checkEqual(t, val, "from-file") // Test GetSources sources := cfg.GetSources("test.value") - assert.Equal(t, "from-file", sources[SourceFile]) - assert.Equal(t, "from-env", sources[SourceEnv]) + checkEqual(t, sources[SourceFile], "from-file") + checkEqual(t, sources[SourceEnv], "from-env") } // TestSetPrecedence tests runtime precedence switching @@ -193,18 +221,18 @@ func TestSetPrecedence(t *testing.T) { // Default precedence: CLI > Env > File > Default val, _ := cfg.Get("test.value") - assert.Equal(t, "from-cli", val) + checkEqual(t, val, "from-cli") // Switch to File > CLI > Env > Default err := cfg.SetPrecedence(SourceFile, SourceCLI, SourceEnv, SourceDefault) - require.NoError(t, err) + mustNoError(t, err) val, _ = cfg.Get("test.value") - assert.Equal(t, "from-file", val) + checkEqual(t, val, "from-file") // Verify precedence was updated precedence := cfg.GetPrecedence() - assert.Equal(t, []Source{SourceFile, SourceCLI, SourceEnv, SourceDefault}, precedence) + checkEqual(t, precedence, []Source{SourceFile, SourceCLI, SourceEnv, SourceDefault}) }) t.Run("NoPrecedenceChangeOptimization", func(t *testing.T) { @@ -215,15 +243,15 @@ func TestSetPrecedence(t *testing.T) { // Set same precedence initialPrecedence := cfg.GetPrecedence() err := cfg.SetPrecedence(initialPrecedence...) - require.NoError(t, err) + mustNoError(t, err) // Should be no-op, verify by checking version version1 := cfg.version.Load() err = cfg.SetPrecedence(initialPrecedence...) - require.NoError(t, err) + mustNoError(t, err) version2 := cfg.version.Load() - assert.Equal(t, version1, version2, "Version should not change on no-op") + checkEqual(t, version2, version1, "Version should not change on no-op") }) t.Run("AutoAddDefaultSource", func(t *testing.T) { @@ -231,11 +259,11 @@ func TestSetPrecedence(t *testing.T) { // Set precedence without SourceDefault err := cfg.SetPrecedence(SourceCLI, SourceFile, SourceEnv) - require.NoError(t, err) + mustNoError(t, err) // SourceDefault should be auto-appended precedence := cfg.GetPrecedence() - assert.Equal(t, []Source{SourceCLI, SourceFile, SourceEnv, SourceDefault}, precedence) + checkEqual(t, precedence, []Source{SourceCLI, SourceFile, SourceEnv, SourceDefault}) }) t.Run("InvalidSourceError", func(t *testing.T) { @@ -243,18 +271,17 @@ func TestSetPrecedence(t *testing.T) { // Try to set invalid source err := cfg.SetPrecedence("invalid", SourceFile) - assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid source") + checkErrorContains(t, err, "invalid source") // Precedence should remain unchanged precedence := cfg.GetPrecedence() - assert.Equal(t, []Source{SourceCLI, SourceEnv, SourceFile, SourceDefault}, precedence) + checkEqual(t, precedence, []Source{SourceCLI, SourceEnv, SourceFile, SourceDefault}) }) t.Run("PrecedenceChangeNotifications", func(t *testing.T) { tmpDir := t.TempDir() configFile := filepath.Join(tmpDir, "test.toml") - os.WriteFile(configFile, []byte(`value = "from-file"`), 0644) + writeTestFile(t, configFile, []byte(`value = "from-file"`), 0644) cfg := New() cfg.Register("value", "default") @@ -273,19 +300,16 @@ func TestSetPrecedence(t *testing.T) { changes := cfg.Watch() // Change precedence - should trigger notification - go func() { - time.Sleep(50 * time.Millisecond) // Let watcher start - cfg.SetPrecedence(SourceFile, SourceCLI, SourceEnv, SourceDefault) - }() + mustNoError(t, cfg.SetPrecedence(SourceFile, SourceCLI, SourceEnv, SourceDefault)) // Wait for precedence change notification select { case change := <-changes: - assert.Equal(t, "precedence:value", change) + checkEqual(t, change, "precedence:value") // Verify value changed val, _ := cfg.Get("value") - assert.Equal(t, "from-file", val) + checkEqual(t, val, "from-file") case <-time.After(500 * time.Millisecond): t.Error("Timeout waiting for precedence change notification") } @@ -305,16 +329,16 @@ func TestSetPrecedence(t *testing.T) { cfg.SetPrecedence(SourceEnv, SourceFile, SourceDefault) for _, path := range paths { val, _ := cfg.Get(path) - assert.Equal(t, "env-"+path, val) + checkEqual(t, val, "env-"+path) } // Switch: File wins err := cfg.SetPrecedence(SourceFile, SourceEnv, SourceDefault) - require.NoError(t, err) + mustNoError(t, err) for _, path := range paths { val, _ := cfg.Get(path) - assert.Equal(t, "file-"+path, val) + checkEqual(t, val, "file-"+path) } }) @@ -371,7 +395,9 @@ func TestSetPrecedence(t *testing.T) { for err := range errors { errs = append(errs, err) } - assert.Empty(t, errs, "Concurrent precedence changes should not produce errors") + if got := len(errs); got != 0 { + t.Errorf("length = %d, want %d: %s", got, 0, "Concurrent precedence changes should not produce errors") + } }) } @@ -381,7 +407,7 @@ func TestPrecedenceWithAutoUpdate(t *testing.T) { configFile := filepath.Join(tmpDir, "test.toml") // Initial file content - os.WriteFile(configFile, []byte(` + writeTestFile(t, configFile, []byte(` server = "file-server-1" port = 8080 `), 0644) @@ -396,7 +422,7 @@ func TestPrecedenceWithAutoUpdate(t *testing.T) { // CLI wins initially val, _ := cfg.Get("server") - assert.Equal(t, "cli-server", val) + checkEqual(t, val, "cli-server") // Enable auto-update opts := WatchOptions{ @@ -408,37 +434,40 @@ func TestPrecedenceWithAutoUpdate(t *testing.T) { // Switch precedence to File > CLI err := cfg.SetPrecedence(SourceFile, SourceCLI, SourceEnv, SourceDefault) - require.NoError(t, err) + mustNoError(t, err) // File should now win val, _ = cfg.Get("server") - assert.Equal(t, "file-server-1", val) + checkEqual(t, val, "file-server-1") + changes := cfg.Watch() // Update file - os.WriteFile(configFile, []byte(` + writeTestFile(t, configFile, []byte(` server = "file-server-2" port = 9090 `), 0644) - // Wait for auto-update - time.Sleep(300 * time.Millisecond) + // The watcher publishes registered-path events after updating the snapshot. + _ = receiveWatchEvent(t, changes) // File still wins with new value val, _ = cfg.Get("server") - assert.Equal(t, "file-server-2", val) + checkEqual(t, val, "file-server-2") // CLI value is preserved but not active cliVal, exists := cfg.GetSource("server", SourceCLI) - assert.True(t, exists) - assert.Equal(t, "cli-server", cliVal) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, cliVal, "cli-server") // Switch back to CLI > File err = cfg.SetPrecedence(SourceCLI, SourceFile, SourceEnv, SourceDefault) - require.NoError(t, err) + mustNoError(t, err) // CLI wins again val, _ = cfg.Get("server") - assert.Equal(t, "cli-server", val) + checkEqual(t, val, "cli-server") } // TestTypeConversion tests automatic type conversion through checked decoding @@ -469,7 +498,7 @@ func TestTypeConversion(t *testing.T) { } err := cfg.RegisterStruct("", defaults) - require.NoError(t, err) + mustNoError(t, err) // Test string conversions from environment cfg.SetSource(SourceEnv, "int", "100") @@ -481,22 +510,22 @@ func TestTypeConversion(t *testing.T) { cfg.SetSource(SourceEnv, "ipnet", "10.0.0.0/8") cfg.SetSource(SourceEnv, "url", "https://example.com:8080/path") cfg.SetSource(SourceEnv, "strings", "x,y,z") - require.NoError(t, cfg.SetSource(SourceEnv, "ints", "7,8,9")) + mustNoError(t, cfg.SetSource(SourceEnv, "ints", "7,8,9")) // Scan into struct var result TestConfig err = cfg.Scan(&result) - require.NoError(t, err) + mustNoError(t, err) - assert.Equal(t, int64(100), result.IntValue) - assert.Equal(t, 2.718, result.FloatValue) - assert.Equal(t, false, result.BoolValue) - assert.Equal(t, 90*time.Second, result.Duration) - assert.Equal(t, "2024-12-25T10:00:00Z", result.Time.Format(time.RFC3339)) - assert.Equal(t, "192.168.1.1", result.IP.String()) - assert.Equal(t, "10.0.0.0/8", result.IPNet.String()) - assert.Equal(t, "https://example.com:8080/path", result.URL.String()) - assert.Equal(t, []string{"x", "y", "z"}, result.StringSlice) + checkEqual(t, result.IntValue, int64(100)) + checkEqual(t, result.FloatValue, 2.718) + checkEqual(t, result.BoolValue, false) + checkEqual(t, result.Duration, 90*time.Second) + checkEqual(t, result.Time.Format(time.RFC3339), "2024-12-25T10:00:00Z") + checkEqual(t, result.IP.String(), "192.168.1.1") + checkEqual(t, result.IPNet.String(), "10.0.0.0/8") + checkEqual(t, result.URL.String(), "https://example.com:8080/path") + checkEqual(t, result.StringSlice, []string{"x", "y", "z"}) // Note: String to int slice conversion through env requires handling in the test } @@ -566,7 +595,9 @@ func TestConcurrentAccess(t *testing.T) { for err := range errors { errs = append(errs, err) } - assert.Empty(t, errs, "Concurrent access should not produce errors") + if got := len(errs); got != 0 { + t.Errorf("length = %d, want %d: %s", got, 0, "Concurrent access should not produce errors") + } } // TestUnregister tests path unregistration @@ -582,34 +613,47 @@ func TestUnregister(t *testing.T) { t.Run("UnregisterSinglePath", func(t *testing.T) { err := cfg.Unregister("server.port") - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } _, exists := cfg.Get("server.port") - assert.False(t, exists) + if exists { + t.Errorf("exists should be false") + } // Other paths should remain _, exists = cfg.Get("server.host") - assert.True(t, exists) + if !exists { + t.Errorf("exists should be true") + } }) t.Run("UnregisterParentPath", func(t *testing.T) { err := cfg.Unregister("server.tls") - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } // All child paths should be removed _, exists := cfg.Get("server.tls.enabled") - assert.False(t, exists) + if exists { + t.Errorf("exists should be false") + } _, exists = cfg.Get("server.tls.cert") - assert.False(t, exists) + if exists { + t.Errorf("exists should be false") + } // Sibling paths should remain _, exists = cfg.Get("server.host") - assert.True(t, exists) + if !exists { + t.Errorf("exists should be true") + } }) t.Run("UnregisterNonExistentPath", func(t *testing.T) { err := cfg.Unregister("nonexistent.path") - assert.Error(t, err) - assert.Contains(t, err.Error(), "path not registered") + checkErrorContains(t, err, "path not registered") }) } @@ -629,12 +673,16 @@ func TestResetFunctionality(t *testing.T) { // Env value should be gone _, exists := cfg.GetSource("test1", SourceEnv) - assert.False(t, exists) + if exists { + t.Errorf("exists should be false") + } // Other sources should remain val, exists := cfg.GetSource("test1", SourceFile) - assert.True(t, exists) - assert.Equal(t, "file1", val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, "file1") }) t.Run("ResetAll", func(t *testing.T) { @@ -643,12 +691,14 @@ func TestResetFunctionality(t *testing.T) { // All values should revert to defaults val1, _ := cfg.Get("test1") val2, _ := cfg.Get("test2") - assert.Equal(t, "default1", val1) - assert.Equal(t, "default2", val2) + checkEqual(t, val1, "default1") + checkEqual(t, val2, "default2") // Source values should be cleared sources := cfg.GetSources("test1") - assert.Empty(t, sources) + if got := len(sources); got != 0 { + t.Errorf("length = %d, want %d", got, 0) + } }) } @@ -664,8 +714,10 @@ func TestValueSizeLimit(t *testing.T) { } err := cfg.Set("test", string(largeValue)) - assert.Error(t, err) - assert.Equal(t, ErrValueSize, err) + if err == nil { + t.Errorf("expected an error") + } + checkEqual(t, err, ErrValueSize) } // TestGetRegisteredPaths tests path listing functionality @@ -687,24 +739,42 @@ func TestGetRegisteredPaths(t *testing.T) { t.Run("GetAllPaths", func(t *testing.T) { all := cfg.GetRegisteredPaths("") - assert.Len(t, all, len(paths)) + if got := len(all); got != len(paths) { + t.Errorf("length = %d, want %d", got, len(paths)) + } for _, path := range paths { - assert.True(t, all[path]) + if !(all[path]) { + t.Errorf("all[path] should be true") + } } }) t.Run("GetPathsWithPrefix", func(t *testing.T) { serverPaths := cfg.GetRegisteredPaths("server.") - assert.Len(t, serverPaths, 3) - assert.True(t, serverPaths["server.host"]) - assert.True(t, serverPaths["server.port"]) - assert.True(t, serverPaths["server.tls.enabled"]) + if got := len(serverPaths); got != 3 { + t.Errorf("length = %d, want %d", got, 3) + } + if !serverPaths["server.host"] { + t.Errorf("serverPaths[\"server.host\"] should be true") + } + if !serverPaths["server.port"] { + t.Errorf("serverPaths[\"server.port\"] should be true") + } + if !serverPaths["server.tls.enabled"] { + t.Errorf("serverPaths[\"server.tls.enabled\"] should be true") + } }) t.Run("GetPathsWithDefaults", func(t *testing.T) { defaults := cfg.GetRegisteredPathsWithDefaults("database.") - assert.Len(t, defaults, 2) - assert.Contains(t, defaults, "database.host") - assert.Contains(t, defaults, "database.port") + if got := len(defaults); got != 2 { + t.Errorf("length = %d, want %d", got, 2) + } + if _, ok := defaults["database.host"]; !ok { + t.Errorf("unexpected map membership for %q", "database.host") + } + if _, ok := defaults["database.port"]; !ok { + t.Errorf("unexpected map membership for %q", "database.port") + } }) } diff --git a/decode.go b/decode.go index 2db10c7..b898e8b 100644 --- a/decode.go +++ b/decode.go @@ -101,7 +101,9 @@ func decodeInto(data any, dst reflect.Value, depth int) error { dst.Set(next) return nil } - if atomicType(dst.Type()) && src.Type() == dst.Type() { + // Native struct defaults can occur inside slices, arrays and map values. + // They already have the destination type, but still need a checked deep copy. + if (atomicType(dst.Type()) || dst.Kind() == reflect.Struct) && src.Type() == dst.Type() { v, err := copyReflect(src, depth+1) if err != nil { return err diff --git a/decode_test.go b/decode_test.go index f826a3d..34a24d3 100644 --- a/decode_test.go +++ b/decode_test.go @@ -7,9 +7,6 @@ import ( "strings" "testing" "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestScanWithComplexTypes tests scanning with various complex types @@ -48,7 +45,7 @@ func TestScanWithComplexTypes(t *testing.T) { } err := cfg.RegisterStruct("", defaults) - require.NoError(t, err) + mustNoError(t, err) // Set values from different sources cfg.SetSource(SourceEnv, "network.ip", "192.168.1.100") @@ -67,19 +64,19 @@ func TestScanWithComplexTypes(t *testing.T) { // Scan into struct var result AppConfig err = cfg.Scan(&result) - require.NoError(t, err) + mustNoError(t, err) // Verify conversions - assert.Equal(t, "192.168.1.100", result.Network.IP.String()) - assert.Equal(t, "192.168.1.0/24", result.Network.IPNet.String()) - assert.Equal(t, "https://api.example.com:8443/v1", result.Network.URL.String()) - assert.Equal(t, 150*time.Second, result.Network.Timeout) - assert.Equal(t, 5, result.Network.Retry.Count) - assert.Equal(t, 10*time.Second, result.Network.Retry.Interval) - assert.Equal(t, []string{"prod", "staging", "test"}, result.Tags) - assert.Equal(t, []int{80, 443, 8080}, result.Ports) - assert.Equal(t, "production", result.Labels["env"]) - assert.Equal(t, "1.2.3", result.Labels["version"]) + checkEqual(t, result.Network.IP.String(), "192.168.1.100") + checkEqual(t, result.Network.IPNet.String(), "192.168.1.0/24") + checkEqual(t, result.Network.URL.String(), "https://api.example.com:8443/v1") + checkEqual(t, result.Network.Timeout, 150*time.Second) + checkEqual(t, result.Network.Retry.Count, 5) + checkEqual(t, result.Network.Retry.Interval, 10*time.Second) + checkEqual(t, result.Tags, []string{"prod", "staging", "test"}) + checkEqual(t, result.Ports, []int{80, 443, 8080}) + checkEqual(t, result.Labels["env"], "production") + checkEqual(t, result.Labels["version"], "1.2.3") } // TestScanWithBasePath tests scanning from nested paths @@ -102,18 +99,20 @@ func TestScanWithBasePath(t *testing.T) { // Scan only the server section var server ServerConfig err := cfg.Scan(&server, "app.server") - require.NoError(t, err) + mustNoError(t, err) - assert.Equal(t, "appserver", server.Host) - assert.Equal(t, 9000, server.Port) - assert.Equal(t, true, server.Enabled) + checkEqual(t, server.Host, "appserver") + checkEqual(t, server.Port, 9000) + checkEqual(t, server.Enabled, true) // Test non-existent base path var empty ServerConfig err = cfg.Scan(&empty, "app.nonexistent") - assert.NoError(t, err) // Should not error, just empty - assert.Equal(t, "", empty.Host) - assert.Equal(t, 0, empty.Port) + if err != nil { + t.Errorf("unexpected error: %v", err) + } // Should not error, just empty + checkEqual(t, empty.Host, "") + checkEqual(t, empty.Port, 0) } // TestScanFromSource tests scanning from specific sources @@ -143,8 +142,8 @@ func TestScanFromSource(t *testing.T) { t.Run(string(tt.source), func(t *testing.T) { var result Config err := cfg.ScanSource(tt.source, &result) - require.NoError(t, err) - assert.Equal(t, tt.expected, result.Value) + mustNoError(t, err) + checkEqual(t, result.Value, tt.expected) }) } } @@ -167,8 +166,7 @@ func TestInvalidScanTargets(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := cfg.Scan(tt.target) - assert.Error(t, err) - assert.Contains(t, err.Error(), tt.expectErr) + checkErrorContains(t, err, tt.expectErr) }) } } @@ -189,12 +187,14 @@ func TestCustomTypeConversion(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { cfg := New() - require.NoError(t, cfg.Register("value", tc.initial)) + mustNoError(t, cfg.Register("value", tc.initial)) err := cfg.Set("value", tc.input) - require.Error(t, err) - assert.Contains(t, err.Error(), tc.fragment) + if err == nil { + t.Fatalf("expected an error") + } + checkErrorContains(t, err, tc.fragment) got, _ := cfg.Get("value") - assert.Equal(t, tc.initial, got) + checkEqual(t, got, tc.initial) }) } } @@ -228,12 +228,12 @@ func TestZeroFields(t *testing.T) { } err := cfg.Scan(&result) - require.NoError(t, err) + mustNoError(t, err) // ZeroFields should reset all fields before decoding - assert.Equal(t, "newvalue", result.KeepValue) - assert.Equal(t, "resetdefault", result.ResetValue) - assert.Equal(t, "initial", result.NestedValue.Field) // Unregistered, so Scan should not touch it + checkEqual(t, result.KeepValue, "newvalue") + checkEqual(t, result.ResetValue, "resetdefault") + checkEqual(t, result.NestedValue.Field, "initial") // Unregistered, so Scan should not touch it } // TestWeaklyTypedInput tests weak type conversion @@ -259,11 +259,11 @@ func TestWeaklyTypedInput(t *testing.T) { var result Config err := cfg.Scan(&result) - require.NoError(t, err) + mustNoError(t, err) - assert.Equal(t, 42, result.IntFromString) - assert.Equal(t, 3.14159, result.FloatFromString) - assert.Equal(t, true, result.BoolFromString) - assert.Equal(t, "12345", result.StringFromInt) - assert.Equal(t, "true", result.StringFromBool) // Canonical boolean text + checkEqual(t, result.IntFromString, 42) + checkEqual(t, result.FloatFromString, 3.14159) + checkEqual(t, result.BoolFromString, true) + checkEqual(t, result.StringFromInt, "12345") + checkEqual(t, result.StringFromBool, "true") // Canonical boolean text } diff --git a/doc/audit.md b/doc/audit.md deleted file mode 100644 index 3e61379..0000000 --- a/doc/audit.md +++ /dev/null @@ -1,66 +0,0 @@ -# Config audit and TOML migration - -This branch updates only lixenwraith/config. It uses Go 1.27.1 and the user's merged -TOML main commit `b59d068ff6a48b86365a65e1121343f672f77a67`, pinned as -`v0.0.0-20260928180512-b59d068ff6a4` because no version tag existed at integration. -Logwisp adoption and other TOML consumers are separate follow-up work. - -## Confirmed defects and fixes - -| Finding | Result | Regression evidence | -| --- | --- | --- | -| Fractions/overflow could pass through weak numeric conversion | Checked TOML conversion and explicit destination-width parsing | `TestCheckedConversionsAndRollback`, `FuzzConfigNumericConversions` | -| Options, input containers and returned values could alias caller state | Owned options/values and detached reads/clones | `TestOptionsAreOwned`, `TestOwnedValuesAndSnapshots` | -| Updating the cached target changed old AsStruct results | Immutable cached result and a fresh snapshot per call | `TestOwnedValuesAndSnapshots` | -| ExportEnv compared uncomparable slices/maps directly | Deep equality, with callbacks outside Config locks | `TestExportEnvContainerValues` | -| Invalid sources could be published before decoding failed | Whole-source validation before mutation | `TestSourceReplacementIsTransactional` | -| Removed env/CLI values survived subsequent loads | Successful loads replace their complete source | `TestSourceReplacementIsTransactional` | -| A timed-out reload kept running and could publish after stop | Context, watcher identity, path and generation checked at commit | `TestReloadCannotPublishAfterStopTimeoutOrNewLoad` | -| A timeout could suppress retries of unchanged content | Retry timeouts while deduplicating their notifications | `TestWatcherRetriesTimedOutUnchangedContent` | -| Debounce did not cover errors from partial in-place writes | Errors and successful updates wait for a quiet interval | `TestWatcherDebouncesErrorsAndRecovers` | -| Metadata-only polling missed some replacements/edits | Content fingerprints on every poll; deletion/recreation recovery | `TestWatcherSameMetadataReplacementAndRecreation` | -| Source saving lost comments and widened permissions | Comment preamble, existing modes retained, new files 0600 | `TestSaveCommentsPermissionsAndFailure` | -| Atomic types were inconsistently registered/serialized | Exact type matching for duration/time/IP/CIDR/URL and pointer forms | `TestAtomicValuesAndFileRoundTrip` | -| A missing file could hide a joined invalid CLI/environment error | Missing-file fallback only when other sources succeed | `TestBuilderDoesNotHideInvalidSourceBehindMissingFile` | -| Required registration was a no-op; overlapping/duplicate fields were ambiguous | Required paths validated at Build; staged registration rejects conflicts | `TestRegistrationAndRequiredContracts` | -| Default source selection and narrow target schemas could be inconsistent | Default precedence honored; explicit defaults must match the typed target | `TestRegistrationAndRequiredContracts` | - -Reproduction against original main `ccd280b`: fractional-to-integer conversion -silently produced 1 from 1.5; modifying an input slice changed its registered -default; an out-of-range CLI batch returned success; and requesting a refreshed -AsStruct changed an earlier result under its reader. All four were reproduced with -public-API probes; committed regressions cover the corrected behavior. - -## Dependency and scope decision - -Mapstructure is replaceable for the package's actual object model. The replacement -supports TOML field names, ordinary containers, pointers and the existing standard -atomic types. It does not reproduce mapstructure's broad weak coercions or add a -custom hook API. Numeric correctness is shared with lixenwraith/toml. - -`go list -deps ./...` reports only the standard library, lixenwraith/toml and this -module. BurntSushi TOML, mapstructure and YAML are removed from runtime code. -Testify remains test-only, so its transitive YAML dependency still appears in go.mod -and go.sum. JSON/YAML files and tags are unsupported, with explicit rejection tests. - -No global reflection cache or lock was introduced. Known container sizes are used -for allocation. Stored values are immutable after admission. One watcher worker -owns observation/debounce state, and subscription cleanup has no per-subscriber -goroutine. File I/O and environment callbacks execute outside Config locks. - -## Verification and limits - -The full unit and race suites and go vet pass with Go 1.27.1. Numeric fuzzing ran -for 10 seconds and passed 234,155 executions, covering full uint64 preservation, -signed boundaries and narrowing. The 386 and Windows test binaries cross-compile -locally. This host cannot execute 386 binaries; CI executes that suite and also -runs race tests, vet, fuzzing and Windows compilation. Final checks are on the PR. - -TOML's supported subset is unchanged. Native datetime syntax is not added; time.Time -is represented by a quoted RFC3339Nano string. Full uint64 works in memory but values -above MaxInt64 cannot be saved as TOML integers. Comment text survives as a preamble; -locations and formatting do not. Unknown file keys are not saved. Depth is bounded -at 128 for configuration graphs. Live application validators remain an application -responsibility; Builder validators run only during initialization. Notifications -are best effort, and file timeouts prevent late publication without forcibly -interrupting kernel I/O. See the linked topic guides for migration details. diff --git a/dynamic_test.go b/dynamic_test.go index 8f42652..70c77bd 100644 --- a/dynamic_test.go +++ b/dynamic_test.go @@ -1,9 +1,7 @@ package config import ( - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "os" + "fmt" "path/filepath" "runtime" "testing" @@ -12,29 +10,37 @@ import ( func TestTOMLOnly(t *testing.T) { for _, format := range []string{"json", "yaml", "other"} { c := New() - require.Error(t, c.SetFileFormat(format)) + if err := c.SetFileFormat(format); err == nil { + t.Fatalf("expected an error") + } _, err := NewBuilder().WithTagName(format).Build() - require.Error(t, err) + if err == nil { + t.Fatalf("expected an error") + } var dst struct{ Value string } - require.Error(t, ScanMap(nil, &dst, format)) + if err := ScanMap(nil, &dst, format); err == nil { + t.Fatalf("expected an error") + } } for _, tc := range []struct{ ext, content string }{ {".toml", `value = "toml"`}, {".conf", `value = "toml"`}, {".config", `value = "toml"`}, } { c := New() - require.NoError(t, c.Register("value", "")) + mustNoError(t, c.Register("value", "")) path := filepath.Join(t.TempDir(), "config"+tc.ext) - require.NoError(t, os.WriteFile(path, []byte(tc.content), 0600)) - require.NoError(t, c.LoadFile(path)) + writeTestFile(t, path, []byte(tc.content), 0600) + mustNoError(t, c.LoadFile(path)) } for _, tc := range []struct{ ext, content string }{ {".json", `{"value":"json"}`}, {".yaml", "value: yaml"}, {".conf", `{"value":"json"}`}, } { c := New() - require.NoError(t, c.Register("value", "")) + mustNoError(t, c.Register("value", "")) path := filepath.Join(t.TempDir(), "config"+tc.ext) - require.NoError(t, os.WriteFile(path, []byte(tc.content), 0600)) - require.Error(t, c.LoadFile(path)) + writeTestFile(t, path, []byte(tc.content), 0600) + if err := c.LoadFile(path); err == nil { + t.Fatalf("expected an error") + } } } @@ -57,17 +63,21 @@ func TestSecurityOptions(t *testing.T) { for _, malPath := range maliciousPaths { err := cfg.LoadFile(malPath) - assert.Error(t, err, "Should reject path: %s", malPath) - assert.Contains(t, err.Error(), "path traversal") + if err == nil { + t.Errorf("expected an error: %s", fmt.Sprintf("Should reject path: %s", malPath)) + } + checkErrorContains(t, err, "path traversal") } // Valid paths should work validPath := filepath.Join(tmpDir, "config.toml") - os.WriteFile(validPath, []byte(`test = "value"`), 0644) + writeTestFile(t, validPath, []byte(`test = "value"`), 0644) cfg.Register("test", "") err := cfg.LoadFile(validPath) - assert.NoError(t, err, "Should accept valid absolute path") + if err != nil { + t.Errorf("unexpected error: %v: %s", err, "Should accept valid absolute path") + } }) t.Run("FileSizeLimit", func(t *testing.T) { @@ -82,11 +92,10 @@ func TestSecurityOptions(t *testing.T) { for i := range largeContent { largeContent[i] = 'a' } - require.NoError(t, os.WriteFile(largePath, largeContent, 0644)) + writeTestFile(t, largePath, largeContent, 0644) err := cfg.LoadFile(largePath) - assert.Error(t, err) - assert.Contains(t, err.Error(), "exceeds maximum size") + checkErrorContains(t, err, "exceeds maximum size") }) t.Run("FileOwnership", func(t *testing.T) { @@ -101,18 +110,12 @@ func TestSecurityOptions(t *testing.T) { // Create file owned by current user (should succeed) ownedPath := filepath.Join(tmpDir, "owned.toml") - require.NoError(t, os.WriteFile(ownedPath, []byte(`test = "value"`), 0644)) + writeTestFile(t, ownedPath, []byte(`test = "value"`), 0644) cfg.Register("test", "") err := cfg.LoadFile(ownedPath) - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } }) } - -// waitForWatchingState waits for watcher state, preventing race conditions of goroutine start and test check -func waitForWatchingState(t *testing.T, cfg *Config, expected bool, msgAndArgs ...any) { - t.Helper() - require.Eventually(t, func() bool { - return cfg.IsWatching() == expected - }, testEventuallyTimeout, 2*SpinWaitInterval, msgAndArgs...) -} diff --git a/go.mod b/go.mod index 1cc7688..e814792 100644 --- a/go.mod +++ b/go.mod @@ -2,13 +2,4 @@ module github.com/lixenwraith/config go 1.27.1 -require ( - github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4 - github.com/stretchr/testify v1.10.0 -) - -require ( - github.com/davecgh/go-spew v1.1.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect -) +require github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4 diff --git a/go.sum b/go.sum index 47ac435..531bace 100644 --- a/go.sum +++ b/go.sum @@ -1,12 +1,2 @@ -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4 h1:9qZU0iBIsqintDh39vieio0ygLiRrPmCEo6Ra45EQyY= github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4/go.mod h1:itcT/Vt5mwmX9KVGCMsWZr6vbU/JgZ24wV/I0J5cMt0= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/hardening_test.go b/hardening_test.go index 3f41fe5..2a74cf4 100644 --- a/hardening_test.go +++ b/hardening_test.go @@ -187,11 +187,11 @@ func TestSourceReplacementIsTransactional(t *testing.T) { t.Fatal("removed environment variable retained") } path := filepath.Join(t.TempDir(), "c.toml") - _ = os.WriteFile(path, []byte("port=100\nname=\"valid\""), 0600) + writeTestFile(t, path, []byte("port=100\nname=\"valid\""), 0600) if err := c.LoadFile(path); err != nil { t.Fatal(err) } - _ = os.WriteFile(path, []byte("port=65536\nname=\"invalid\""), 0600) + writeTestFile(t, path, []byte("port=65536\nname=\"invalid\""), 0600) if err := c.LoadFile(path); err == nil { t.Fatal("accepted invalid file") } @@ -212,7 +212,7 @@ func TestSaveCommentsPermissionsAndFailure(t *testing.T) { _ = c.Register("text", "") path := filepath.Join(t.TempDir(), "c.toml") original := "# heading\nvalue=7 # inline\ntext=\"hash # inside\"\n" - _ = os.WriteFile(path, []byte(original), 0600) + writeTestFile(t, path, []byte(original), 0600) if err := c.LoadFile(path); err != nil { t.Fatal(err) } diff --git a/loader_test.go b/loader_test.go index 9c1b804..662a775 100644 --- a/loader_test.go +++ b/loader_test.go @@ -2,12 +2,12 @@ package config import ( + "errors" + "fmt" "os" "path/filepath" + "strings" "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestFileLoading tests TOML file loading @@ -31,7 +31,7 @@ key = "/path/to/key.pem" connections = [1, 2, 3] tags = ["primary", "replica"] ` - os.WriteFile(configFile, []byte(content), 0644) + writeTestFile(t, configFile, []byte(content), 0644) cfg := New() // Register all paths @@ -44,46 +44,49 @@ tags = ["primary", "replica"] cfg.Register("database.tags", []string{}) err := cfg.LoadFile(configFile) - require.NoError(t, err) + mustNoError(t, err) // Verify loaded values host, _ := cfg.Get("server.host") - assert.Equal(t, "example.com", host) + checkEqual(t, host, "example.com") port, _ := cfg.Get("server.port") - assert.Equal(t, int64(9000), port) + checkEqual(t, port, int64(9000)) enabled, _ := cfg.Get("server.enabled") - assert.Equal(t, true, enabled) + checkEqual(t, enabled, true) cert, _ := cfg.Get("server.tls.cert") - assert.Equal(t, "/path/to/cert.pem", cert) + checkEqual(t, cert, "/path/to/cert.pem") // Arrays are loaded as []any connections, _ := cfg.Get("database.connections") - assert.Equal(t, []any{int64(1), int64(2), int64(3)}, connections) + checkEqual(t, connections, []any{int64(1), int64(2), int64(3)}) }) t.Run("InvalidTOMLFile", func(t *testing.T) { configFile := filepath.Join(tmpDir, "invalid.toml") - os.WriteFile(configFile, []byte(`invalid = toml content`), 0644) + writeTestFile(t, configFile, []byte(`invalid = toml content`), 0644) cfg := New() err := cfg.LoadFile(configFile) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to parse TOML") + checkErrorContains(t, err, "failed to parse TOML") }) t.Run("NonExistentFile", func(t *testing.T) { cfg := New() err := cfg.LoadFile("/non/existent/file.toml") - assert.Error(t, err) - assert.ErrorIs(t, err, ErrConfigNotFound) + if err == nil { + t.Errorf("expected an error") + } + if err := err; !errors.Is(err, ErrConfigNotFound) { + t.Errorf("error = %v, want errors.Is(_, %v)", err, ErrConfigNotFound) + } }) t.Run("UnregisteredPathsIgnored", func(t *testing.T) { configFile := filepath.Join(tmpDir, "extra.toml") - os.WriteFile(configFile, []byte(` + writeTestFile(t, configFile, []byte(` registered = "value" unregistered = "ignored" `), 0644) @@ -92,59 +95,51 @@ unregistered = "ignored" cfg.Register("registered", "") err := cfg.LoadFile(configFile) - require.NoError(t, err) + mustNoError(t, err) val, exists := cfg.Get("registered") - assert.True(t, exists) - assert.Equal(t, "value", val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, "value") _, exists = cfg.Get("unregistered") - assert.False(t, exists) + if exists { + t.Errorf("exists should be false") + } }) } // TestEnvironmentLoading tests environment variable loading func TestEnvironmentLoading(t *testing.T) { - // Save and restore environment - originalEnv := os.Environ() - defer func() { - os.Clearenv() - for _, e := range originalEnv { - parts := splitEnvVar(e) - if len(parts) == 2 { - os.Setenv(parts[0], parts[1]) - } - } - }() - t.Run("DefaultEnvTransform", func(t *testing.T) { cfg := New() cfg.Register("server.host", "localhost") cfg.Register("server.port", 8080) cfg.Register("enable_debug", false) - os.Setenv("APP_SERVER_HOST", "envhost") - os.Setenv("APP_SERVER_PORT", "9090") - os.Setenv("APP_ENABLE_DEBUG", "true") + t.Setenv("APP_SERVER_HOST", "envhost") + t.Setenv("APP_SERVER_PORT", "9090") + t.Setenv("APP_ENABLE_DEBUG", "true") err := cfg.LoadEnv("APP_") - require.NoError(t, err) + mustNoError(t, err) host, _ := cfg.Get("server.host") - assert.Equal(t, "envhost", host) + checkEqual(t, host, "envhost") port, _ := cfg.Get("server.port") - assert.Equal(t, "9090", port) // String from env + checkEqual(t, port, "9090") // String from env debug, _ := cfg.Get("enable_debug") - assert.Equal(t, "true", debug) // String from env + checkEqual(t, debug, "true") // String from env }) t.Run("CustomEnvTransform", func(t *testing.T) { cfg := New() cfg.Register("db.host", "localhost") - os.Setenv("DATABASE_HOSTNAME", "customhost") + t.Setenv("DATABASE_HOSTNAME", "customhost") opts := LoadOptions{ Sources: []Source{SourceEnv, SourceDefault}, @@ -157,10 +152,10 @@ func TestEnvironmentLoading(t *testing.T) { } err := cfg.loadWithOptions("", nil, opts) - require.NoError(t, err) + mustNoError(t, err) host, _ := cfg.Get("db.host") - assert.Equal(t, "customhost", host) + checkEqual(t, host, "customhost") }) t.Run("EnvWhitelist", func(t *testing.T) { @@ -168,8 +163,8 @@ func TestEnvironmentLoading(t *testing.T) { cfg.Register("allowed.path", "default1") cfg.Register("blocked.path", "default2") - os.Setenv("ALLOWED_PATH", "env1") - os.Setenv("BLOCKED_PATH", "env2") + t.Setenv("ALLOWED_PATH", "env1") + t.Setenv("BLOCKED_PATH", "env2") opts := LoadOptions{ Sources: []Source{SourceEnv, SourceDefault}, @@ -177,13 +172,13 @@ func TestEnvironmentLoading(t *testing.T) { } err := cfg.loadWithOptions("", nil, opts) - require.NoError(t, err) + mustNoError(t, err) allowed, _ := cfg.Get("allowed.path") - assert.Equal(t, "env1", allowed) + checkEqual(t, allowed, "env1") blocked, _ := cfg.Get("blocked.path") - assert.Equal(t, "default2", blocked) // Should not load from env + checkEqual(t, blocked, "default2") // Should not load from env }) t.Run("DiscoverEnv", func(t *testing.T) { @@ -192,16 +187,18 @@ func TestEnvironmentLoading(t *testing.T) { cfg.Register("test.two", "") cfg.Register("other.value", "") - os.Setenv("PREFIX_TEST_ONE", "value1") - os.Setenv("PREFIX_TEST_TWO", "value2") - os.Setenv("PREFIX_OTHER_VALUE", "value3") - os.Setenv("UNRELATED_VAR", "ignored") + t.Setenv("PREFIX_TEST_ONE", "value1") + t.Setenv("PREFIX_TEST_TWO", "value2") + t.Setenv("PREFIX_OTHER_VALUE", "value3") + t.Setenv("UNRELATED_VAR", "ignored") discovered := cfg.DiscoverEnv("PREFIX_") - assert.Len(t, discovered, 3) - assert.Equal(t, "PREFIX_TEST_ONE", discovered["test.one"]) - assert.Equal(t, "PREFIX_TEST_TWO", discovered["test.two"]) - assert.Equal(t, "PREFIX_OTHER_VALUE", discovered["other.value"]) + if got := len(discovered); got != 3 { + t.Errorf("length = %d, want %d", got, 3) + } + checkEqual(t, discovered["test.one"], "PREFIX_TEST_ONE") + checkEqual(t, discovered["test.two"], "PREFIX_TEST_TWO") + checkEqual(t, discovered["other.value"], "PREFIX_OTHER_VALUE") }) } @@ -272,14 +269,16 @@ func TestCLIParsing(t *testing.T) { } err := cfg.LoadCLI(tt.args) - require.NoError(t, err) + mustNoError(t, err) // Verify values for path, expected := range tt.expected { if path != "" { val, exists := cfg.Get(path) - assert.True(t, exists, "Path %s should exist", path) - assert.Equal(t, expected, val) + if !exists { + t.Errorf("exists should be true: %s", fmt.Sprintf("Path %s should exist", path)) + } + checkEqual(t, val, expected) } } }) @@ -287,9 +286,10 @@ func TestCLIParsing(t *testing.T) { t.Run("InvalidKeySegment", func(t *testing.T) { result, err := parseArgs([]string{"--invalid!key=value"}) - assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid command-line key segment") - assert.Nil(t, result) + checkErrorContains(t, err, "invalid command-line key segment") + if got := result; got != nil { + t.Errorf("got %v, want nil", got) + } }) } @@ -297,18 +297,14 @@ func TestCLIParsing(t *testing.T) { func TestLoadWithOptions(t *testing.T) { tmpDir := t.TempDir() configFile := filepath.Join(tmpDir, "config.toml") - os.WriteFile(configFile, []byte(` + writeTestFile(t, configFile, []byte(` [server] host = "filehost" port = 8080 `), 0644) - os.Setenv("TEST_SERVER_HOST", "envhost") - os.Setenv("TEST_SERVER_PORT", "9090") - defer func() { - os.Unsetenv("TEST_SERVER_HOST") - os.Unsetenv("TEST_SERVER_PORT") - }() + t.Setenv("TEST_SERVER_HOST", "envhost") + t.Setenv("TEST_SERVER_PORT", "9090") cfg := New() cfg.Register("server.host", "defaulthost") @@ -322,21 +318,21 @@ port = 8080 } err := cfg.loadWithOptions(configFile, args, opts) - require.NoError(t, err) + mustNoError(t, err) // CLI should win port, _ := cfg.Get("server.port") - assert.Equal(t, "7070", port) + checkEqual(t, port, "7070") // ENV should win over file host, _ := cfg.Get("server.host") - assert.Equal(t, "envhost", host) + checkEqual(t, host, "envhost") // Test source inspection sources := cfg.GetSources("server.port") - assert.Equal(t, "7070", sources[SourceCLI]) - assert.Equal(t, "9090", sources[SourceEnv]) - assert.Equal(t, int64(8080), sources[SourceFile]) + checkEqual(t, sources[SourceCLI], "7070") + checkEqual(t, sources[SourceEnv], "9090") + checkEqual(t, sources[SourceFile], int64(8080)) } // TestAtomicSave tests atomic file saving @@ -355,23 +351,27 @@ func TestAtomicSave(t *testing.T) { t.Run("SaveCurrentState", func(t *testing.T) { savePath := filepath.Join(tmpDir, "saved.toml") err := cfg.Save(savePath) - require.NoError(t, err) + mustNoError(t, err) // Verify file exists and is readable content, err := os.ReadFile(savePath) - require.NoError(t, err) - assert.Contains(t, string(content), "savehost") - assert.Contains(t, string(content), "9999") + mustNoError(t, err) + if got := string(content); !strings.Contains(got, "savehost") { + t.Errorf("unexpected substring membership: %q in %q", "savehost", got) + } + if got := string(content); !strings.Contains(got, "9999") { + t.Errorf("unexpected substring membership: %q in %q", "9999", got) + } // Load into new config to verify cfg2 := New() cfg2.Register("server.host", "") cfg2.Register("server.port", 0) err = cfg2.LoadFile(savePath) - require.NoError(t, err) + mustNoError(t, err) host, _ := cfg2.Get("server.host") - assert.Equal(t, "savehost", host) + checkEqual(t, host, "savehost") }) t.Run("SaveSpecificSource", func(t *testing.T) { @@ -381,23 +381,31 @@ func TestAtomicSave(t *testing.T) { savePath := filepath.Join(tmpDir, "env-only.toml") err := cfg.SaveSource(savePath, SourceEnv) - require.NoError(t, err) + mustNoError(t, err) content, err := os.ReadFile(savePath) - require.NoError(t, err) - assert.Contains(t, string(content), "envhost") - assert.Contains(t, string(content), "7777") - assert.NotContains(t, string(content), "6666") + mustNoError(t, err) + if got := string(content); !strings.Contains(got, "envhost") { + t.Errorf("unexpected substring membership: %q in %q", "envhost", got) + } + if got := string(content); !strings.Contains(got, "7777") { + t.Errorf("unexpected substring membership: %q in %q", "7777", got) + } + if got := string(content); strings.Contains(got, "6666") { + t.Errorf("unexpected substring membership: %q in %q", "6666", got) + } }) t.Run("SaveToNonExistentDirectory", func(t *testing.T) { savePath := filepath.Join(tmpDir, "new", "dir", "config.toml") err := cfg.Save(savePath) - require.NoError(t, err) + mustNoError(t, err) // Verify file was created _, err = os.Stat(savePath) - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } }) } @@ -414,21 +422,12 @@ func TestExportEnv(t *testing.T) { exports := cfg.ExportEnv("APP_") - assert.Len(t, exports, 2) - assert.Equal(t, "exporthost", exports["APP_SERVER_HOST"]) - assert.Equal(t, "true", exports["APP_FEATURE_ENABLED"]) - assert.NotContains(t, exports, "APP_SERVER_PORT") // Still default -} - -// splitEnvVar splits environment variable into key and value -func splitEnvVar(env string) []string { - parts := make([]string, 2) - for i := 0; i < len(env); i++ { - if env[i] == '=' { - parts[0] = env[:i] - parts[1] = env[i+1:] - return parts - } + if got := len(exports); got != 2 { + t.Errorf("length = %d, want %d", got, 2) } - return []string{env} + checkEqual(t, exports["APP_SERVER_HOST"], "exporthost") + checkEqual(t, exports["APP_FEATURE_ENABLED"], "true") + if _, ok := exports["APP_SERVER_PORT"]; ok { + t.Errorf("unexpected map membership for %q", "APP_SERVER_PORT") + } // Still default } diff --git a/test_helpers_test.go b/test_helpers_test.go new file mode 100644 index 0000000..13c6059 --- /dev/null +++ b/test_helpers_test.go @@ -0,0 +1,89 @@ +package config + +import ( + "os" + "reflect" + "strings" + "testing" + "time" +) + +// Container and interface comparisons must retain Go types and nil/empty distinctions. +func checkEqual(t testing.TB, got, want any, context ...string) { + t.Helper() + if !reflect.DeepEqual(got, want) { + prefix := "" + if len(context) > 0 { + prefix = strings.Join(context, "; ") + ": " + } + t.Errorf("%sgot %#v (%T), want %#v (%T)", prefix, got, got, want, want) + } +} + +func mustNoError(t testing.TB, err error) { + t.Helper() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func checkErrorContains(t testing.TB, err error, fragment string) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), fragment) { + t.Errorf("error = %v, want text containing %q", err, fragment) + } +} + +func mustPanic(t testing.TB, fn func()) { + t.Helper() + defer func() { + if recover() == nil { + t.Fatal("expected a panic") + } + }() + fn() +} + +func writeTestFile(t testing.TB, path string, data []byte, mode os.FileMode) { + t.Helper() + if err := os.WriteFile(path, data, mode); err != nil { + t.Fatalf("write fixture %q: %v", path, err) + } +} + +// File-backed capture avoids pipe capacity limits and restores stdout even if +// the tested operation calls Fatal. Tests using process globals run serially. +func captureStdout(t testing.TB, fn func()) string { + t.Helper() + file, err := os.CreateTemp(t.TempDir(), "stdout-*") + mustNoError(t, err) + previous := os.Stdout + t.Cleanup(func() { os.Stdout = previous; _ = file.Close() }) + os.Stdout = file + fn() + os.Stdout = previous + mustNoError(t, file.Close()) + data, err := os.ReadFile(file.Name()) + mustNoError(t, err) + return string(data) +} + +func checkWatchingState(t testing.TB, cfg *Config, expected bool, context ...string) { + t.Helper() + // Starting and stopping publish their state synchronously. + checkEqual(t, cfg.IsWatching(), expected, context...) +} + +func receiveWatchEvent(t testing.TB, changes <-chan string) string { + t.Helper() + select { + case event, ok := <-changes: + if !ok { + t.Fatal("watch channel closed before the expected event") + } + return event + case <-time.After(testWatchTimeout): + t.Fatal("timed out waiting for a watch event") + return "" + } +} diff --git a/typed_defaults_test.go b/typed_defaults_test.go new file mode 100644 index 0000000..6ed3461 --- /dev/null +++ b/typed_defaults_test.go @@ -0,0 +1,57 @@ +package config + +import "testing" + +func TestTypedStructDefaultsAndSnapshots(t *testing.T) { + type plugin struct { + Limit uint64 `toml:"limit"` + Data map[string]any `toml:"data"` + } + type settings struct { + Plugins []plugin `toml:"plugins"` + ByName map[string]*plugin `toml:"by_name"` + Fixed [1]plugin `toml:"fixed"` + } + item := plugin{Limit: ^uint64(0), Data: map[string]any{"tags": []string{"original"}}} + initial := settings{Plugins: []plugin{item}, ByName: map[string]*plugin{"one": &item}, Fixed: [1]plugin{item}} + cfg, err := NewBuilder().WithTarget(&initial).WithArgs(nil).Build() + if err != nil { + t.Fatal(err) + } + value, err := cfg.AsStruct() + if err != nil { + t.Fatal(err) + } + first := value.(*settings) + initial.Plugins[0].Data["tags"].([]string)[0] = "caller" + first.Plugins[0].Data["tags"].([]string)[0] = "snapshot" + first.ByName["one"].Limit = 0 + first.Fixed[0].Data["tags"] = nil + value, err = cfg.AsStruct() + if err != nil { + t.Fatal(err) + } + next := value.(*settings) + for _, got := range []plugin{next.Plugins[0], *next.ByName["one"], next.Fixed[0]} { + if got.Limit != ^uint64(0) || got.Data["tags"].([]string)[0] != "original" { + t.Fatalf("native struct was truncated or aliased: %+v", got) + } + } +} + +func TestTypedStructDecodeStillRejectsCycles(t *testing.T) { + type node struct { + Next *node `toml:"next"` + } + cyclic := &node{} + cyclic.Next = cyclic + target := struct { + Nodes []node `toml:"nodes"` + }{Nodes: []node{{}}} + if err := ScanMap(map[string]any{"nodes": []node{*cyclic}}, &target); err == nil { + t.Fatal("native struct copying accepted a cycle") + } + if len(target.Nodes) != 1 || target.Nodes[0].Next != nil { + t.Fatal("failed decode modified the destination") + } +} diff --git a/utility_test.go b/utility_test.go index ae7dc16..fc7ea69 100644 --- a/utility_test.go +++ b/utility_test.go @@ -5,18 +5,16 @@ import ( "flag" "os" "path/filepath" + "strings" "testing" "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestQuickFunctions tests the utility Quick* functions func TestQuickFunctions(t *testing.T) { tmpDir := t.TempDir() configFile := filepath.Join(tmpDir, "quick.toml") - os.WriteFile(configFile, []byte(` + writeTestFile(t, configFile, []byte(` host = "quickhost" port = 7777 `), 0644) @@ -37,18 +35,18 @@ port = 7777 // Mock os.Args oldArgs := os.Args os.Args = []string{"cmd", "--port=9999"} - defer func() { os.Args = oldArgs }() + t.Cleanup(func() { os.Args = oldArgs }) cfg, err := Quick(defaults, "QUICK_", configFile) - require.NoError(t, err) + mustNoError(t, err) // CLI should override port, _ := cfg.Get("port") - assert.Equal(t, "9999", port) + checkEqual(t, port, "9999") // File value host, _ := cfg.Get("host") - assert.Equal(t, "quickhost", host) + checkEqual(t, host, "quickhost") }) t.Run("QuickCustom", func(t *testing.T) { @@ -58,22 +56,24 @@ port = 7777 } cfg, err := QuickCustom(defaults, opts, configFile) - require.NoError(t, err) + mustNoError(t, err) // Should use file value port, _ := cfg.Get("port") - assert.Equal(t, int64(7777), port) + checkEqual(t, port, int64(7777)) }) t.Run("MustQuickPanic", func(t *testing.T) { // Valid case - should not panic - assert.NotPanics(t, func() { + { cfg := MustQuick(defaults, "TEST_", configFile) - assert.NotNil(t, cfg) - }) + if got := cfg; got == nil { + t.Errorf("got %v, want non-nil", got) + } + } // Invalid struct - should panic - assert.Panics(t, func() { + mustPanic(t, func() { MustQuick("not-a-struct", "TEST_", configFile) }) }) @@ -86,15 +86,15 @@ port = 7777 } cfg, err := QuickTyped(target, "TYPED_", configFile) - require.NoError(t, err) + mustNoError(t, err) // Should populate from file updated, err := cfg.AsStruct() - require.NoError(t, err) + mustNoError(t, err) typedCfg := updated.(*QuickConfig) - assert.Equal(t, "quickhost", typedCfg.Host) - assert.Equal(t, 7777, typedCfg.Port) + checkEqual(t, typedCfg.Host, "quickhost") + checkEqual(t, typedCfg.Port, 7777) }) } @@ -110,24 +110,34 @@ func TestFlagGeneration(t *testing.T) { t.Run("GenerateFlags", func(t *testing.T) { fs := cfg.GenerateFlags() - require.NotNil(t, fs) + if got := fs; got == nil { + t.Fatalf("got %v, want non-nil", got) + } // Verify flags exist hostFlag := fs.Lookup("server.host") - require.NotNil(t, hostFlag) - assert.Equal(t, "localhost", hostFlag.DefValue) + if got := hostFlag; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, hostFlag.DefValue, "localhost") portFlag := fs.Lookup("server.port") - require.NotNil(t, portFlag) - assert.Equal(t, "8080", portFlag.DefValue) + if got := portFlag; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, portFlag.DefValue, "8080") debugFlag := fs.Lookup("debug.enabled") - require.NotNil(t, debugFlag) - assert.Equal(t, "false", debugFlag.DefValue) + if got := debugFlag; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, debugFlag.DefValue, "false") timeoutFlag := fs.Lookup("timeout") - require.NotNil(t, timeoutFlag) - assert.Equal(t, "30.5", timeoutFlag.DefValue) + if got := timeoutFlag; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, timeoutFlag.DefValue, "30.5") }) t.Run("BindFlags", func(t *testing.T) { @@ -138,21 +148,21 @@ func TestFlagGeneration(t *testing.T) { // Parse with test values err := fs.Parse([]string{"-server.host=flaghost", "-server.port=5555", "-debug.enabled"}) - require.NoError(t, err) + mustNoError(t, err) // Bind to config err = cfg.BindFlags(fs) - require.NoError(t, err) + mustNoError(t, err) // Verify values were set host, _ := cfg.Get("server.host") - assert.Equal(t, "flaghost", host) + checkEqual(t, host, "flaghost") port, _ := cfg.Get("server.port") - assert.Equal(t, "5555", port) + checkEqual(t, port, "5555") debug, _ := cfg.Get("debug.enabled") - assert.Equal(t, "true", debug) + checkEqual(t, debug, "true") }) t.Run("BindFlagsError", func(t *testing.T) { @@ -161,8 +171,7 @@ func TestFlagGeneration(t *testing.T) { fs.Parse([]string{"-unregistered.path=test"}) err := cfg.BindFlags(fs) - assert.Error(t, err) - assert.Contains(t, err.Error(), "unregistered flag") + checkErrorContains(t, err, "unregistered flag") }) } @@ -175,10 +184,9 @@ func TestValidation(t *testing.T) { t.Run("ValidationFails", func(t *testing.T) { err := cfg.Validate("required.host", "required.port") - assert.Error(t, err) - assert.Contains(t, err.Error(), "missing required configuration") - assert.Contains(t, err.Error(), "required.host") - assert.Contains(t, err.Error(), "required.port") + checkErrorContains(t, err, "missing required configuration") + checkErrorContains(t, err, "required.host") + checkErrorContains(t, err, "required.port") }) t.Run("ValidationPasses", func(t *testing.T) { @@ -186,13 +194,14 @@ func TestValidation(t *testing.T) { cfg.Set("required.port", 8080) err := cfg.Validate("required.host", "required.port") - assert.NoError(t, err) + if err != nil { + t.Errorf("unexpected error: %v", err) + } }) t.Run("ValidationUnregisteredPath", func(t *testing.T) { err := cfg.Validate("nonexistent.path") - assert.Error(t, err) - assert.Contains(t, err.Error(), "nonexistent.path (not registered)") + checkErrorContains(t, err, "nonexistent.path (not registered)") }) t.Run("ValidationWithSourceValue", func(t *testing.T) { @@ -203,7 +212,9 @@ func TestValidation(t *testing.T) { cfg2.SetSource(SourceEnv, "test", "default") err := cfg2.Validate("test") - assert.NoError(t, err) // Should pass because env provided value + if err != nil { + t.Errorf("unexpected error: %v", err) + } // Should pass because env provided value }) } @@ -220,35 +231,25 @@ func TestDebugAndDump(t *testing.T) { t.Run("Debug", func(t *testing.T) { debug := cfg.Debug() - assert.Contains(t, debug, "Configuration Debug Info") - assert.Contains(t, debug, "Precedence:") - assert.Contains(t, debug, "server.host:") - assert.Contains(t, debug, "Current: envhost") - assert.Contains(t, debug, "Default: localhost") - assert.Contains(t, debug, "file: filehost") - assert.Contains(t, debug, "env: envhost") + for _, fragment := range []string{"Configuration Debug Info", "Precedence:", "server.host:", "Current: envhost", "Default: localhost", "file: filehost", "env: envhost"} { + if !strings.Contains(debug, fragment) { + t.Errorf("debug output %q is missing %q", debug, fragment) + } + } }) t.Run("Dump", func(t *testing.T) { - // Capture stdout - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w + outputStr := captureStdout(t, func() { mustNoError(t, cfg.Dump()) }) - err := cfg.Dump() - assert.NoError(t, err) - - w.Close() - os.Stdout = oldStdout - - // Read output - output := make([]byte, 1024) - n, _ := r.Read(output) - outputStr := string(output[:n]) - - assert.Contains(t, outputStr, "[server]") - assert.Contains(t, outputStr, "host = ") - assert.Contains(t, outputStr, "port = ") + if got := outputStr; !strings.Contains(got, "[server]") { + t.Errorf("unexpected substring membership: %q in %q", "[server]", got) + } + if got := outputStr; !strings.Contains(got, "host = ") { + t.Errorf("unexpected substring membership: %q in %q", "host = ", got) + } + if got := outputStr; !strings.Contains(got, "port = ") { + t.Errorf("unexpected substring membership: %q in %q", "port = ", got) + } }) } @@ -262,16 +263,22 @@ func TestClone(t *testing.T) { cfg.SetSource(SourceEnv, "shared.value", "envvalue") clone := cfg.Clone() - require.NotNil(t, clone) + if got := clone; got == nil { + t.Fatalf("got %v, want non-nil", got) + } // Verify values are copied val, exists := clone.Get("original.value") - assert.True(t, exists) - assert.Equal(t, "filevalue", val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, "filevalue") val, exists = clone.Get("shared.value") - assert.True(t, exists) - assert.Equal(t, "envvalue", val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, "envvalue") // Modify clone should not affect original clone.Set("original.value", "clonevalue") @@ -279,12 +286,12 @@ func TestClone(t *testing.T) { originalVal, _ := cfg.Get("original.value") cloneVal, _ := clone.Get("original.value") - assert.Equal(t, "filevalue", originalVal) - assert.Equal(t, "clonevalue", cloneVal) + checkEqual(t, originalVal, "filevalue") + checkEqual(t, cloneVal, "clonevalue") // Verify source data is copied sources := clone.GetSources("shared.value") - assert.Equal(t, "envvalue", sources[SourceEnv]) + checkEqual(t, sources[SourceEnv], "envvalue") } // TestGenericHelpers tests generic helper functions @@ -297,20 +304,22 @@ func TestGenericHelpers(t *testing.T) { t.Run("GetTyped", func(t *testing.T) { port, err := GetTyped[int](cfg, "server.port") - require.NoError(t, err) - assert.Equal(t, 8080, port) + mustNoError(t, err) + checkEqual(t, port, 8080) host, err := GetTyped[string](cfg, "server.host") - require.NoError(t, err) - assert.Equal(t, "localhost", host) + mustNoError(t, err) + checkEqual(t, host, "localhost") // Test with custom decode hook type readTimeout, err := GetTyped[time.Duration](cfg, "timeouts.read") - require.NoError(t, err) - assert.Equal(t, 5*time.Second, readTimeout) + mustNoError(t, err) + checkEqual(t, readTimeout, 5*time.Second) _, err = GetTyped[int](cfg, "nonexistent.path") - assert.Error(t, err) + if err == nil { + t.Errorf("expected an error") + } }) t.Run("ScanTyped", func(t *testing.T) { @@ -320,10 +329,12 @@ func TestGenericHelpers(t *testing.T) { } serverConf, err := ScanTyped[ServerConfig](cfg, "server") - require.NoError(t, err) - require.NotNil(t, serverConf) - assert.Equal(t, "localhost", serverConf.Host) - assert.Equal(t, 8080, serverConf.Port) + mustNoError(t, err) + if got := serverConf; got == nil { + t.Fatalf("got %v, want non-nil", got) + } + checkEqual(t, serverConf.Host, "localhost") + checkEqual(t, serverConf.Port, 8080) }) } @@ -334,13 +345,15 @@ func TestGetTypedWithDefault(t *testing.T) { // Get with default when path doesn't exist port, err := GetTypedWithDefault(cfg, "server.port", int64(8080)) - require.NoError(t, err) - assert.Equal(t, int64(8080), port) + mustNoError(t, err) + checkEqual(t, port, int64(8080)) // Verify it was actually set val, exists := cfg.Get("server.port") - assert.True(t, exists) - assert.Equal(t, int64(8080), val) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, val, int64(8080)) }) t.Run("PathAlreadySet", func(t *testing.T) { @@ -350,8 +363,8 @@ func TestGetTypedWithDefault(t *testing.T) { // Should return existing value, not default host, err := GetTypedWithDefault(cfg, "server.host", "default.com") - require.NoError(t, err) - assert.Equal(t, "example.com", host) + mustNoError(t, err) + checkEqual(t, host, "example.com") }) t.Run("DifferentTypes", func(t *testing.T) { @@ -359,16 +372,18 @@ func TestGetTypedWithDefault(t *testing.T) { // Test with various types timeout, err := GetTypedWithDefault(cfg, "timeouts.read", 30*time.Second) - require.NoError(t, err) - assert.Equal(t, 30*time.Second, timeout) + mustNoError(t, err) + checkEqual(t, timeout, 30*time.Second) enabled, err := GetTypedWithDefault(cfg, "features.enabled", true) - require.NoError(t, err) - assert.True(t, enabled) + mustNoError(t, err) + if !enabled { + t.Errorf("enabled should be true") + } tags, err := GetTypedWithDefault(cfg, "app.tags", []string{"default", "tag"}) - require.NoError(t, err) - assert.Equal(t, []string{"default", "tag"}, tags) + mustNoError(t, err) + checkEqual(t, tags, []string{"default", "tag"}) }) } @@ -396,16 +411,18 @@ func TestScanMap(t *testing.T) { var target Config err := ScanMap(configMap, &target) - require.NoError(t, err) - assert.Equal(t, "localhost", target.Server.Host) - assert.Equal(t, 8080, target.Server.Port) - assert.Equal(t, 15*time.Second, target.Server.Timeout) - assert.Equal(t, "info", target.LogLevel) + mustNoError(t, err) + checkEqual(t, target.Server.Host, "localhost") + checkEqual(t, target.Server.Port, 8080) + checkEqual(t, target.Server.Timeout, 15*time.Second) + checkEqual(t, target.LogLevel, "info") }) t.Run("RejectJSONTags", func(t *testing.T) { var target Config - require.Error(t, ScanMap(map[string]any{}, &target, "json")) + if err := ScanMap(map[string]any{}, &target, "json"); err == nil { + t.Fatalf("expected an error") + } }) t.Run("NilMapInput", func(t *testing.T) { @@ -414,13 +431,15 @@ func TestScanMap(t *testing.T) { target.Server.Port = 1234 err := ScanMap(nil, &target) - require.NoError(t, err) + mustNoError(t, err) // Verify that fields are NOT changed when the map is empty, // reflecting the observed behavior. - assert.Equal(t, "initial", target.LogLevel) - assert.Equal(t, 1234, target.Server.Port) - assert.Empty(t, target.Server.Host) + checkEqual(t, target.LogLevel, "initial") + checkEqual(t, target.Server.Port, 1234) + if got := len(target.Server.Host); got != 0 { + t.Errorf("length = %d, want %d", got, 0) + } }) t.Run("PartialMapBehavior", func(t *testing.T) { @@ -433,13 +452,13 @@ func TestScanMap(t *testing.T) { target.LogLevel = "initial_log" err := ScanMap(configMap, &target) - require.NoError(t, err) + mustNoError(t, err) // Mapped field should be updated - assert.Equal(t, "warn", target.LogLevel) + checkEqual(t, target.LogLevel, "warn") // Unmapped fields should be untouched - assert.Equal(t, "initial_host", target.Server.Host, "Unmapped field should be untouched") - assert.Equal(t, 1234, target.Server.Port, "Unmapped field should be untouched") + checkEqual(t, target.Server.Host, "initial_host", "Unmapped field should be untouched") + checkEqual(t, target.Server.Port, 1234, "Unmapped field should be untouched") }) t.Run("InvalidTarget", func(t *testing.T) { @@ -447,8 +466,10 @@ func TestScanMap(t *testing.T) { var target Config // Not a pointer err := ScanMap(configMap, target) - assert.Error(t, err) + if err == nil { + t.Errorf("expected an error") + } // Invalid targets return a categorized type error. - assert.Contains(t, err.Error(), "must be non-nil pointer") + checkErrorContains(t, err, "must be non-nil pointer") }) } diff --git a/validator_test.go b/validator_test.go index 6217b72..989d53e 100644 --- a/validator_test.go +++ b/validator_test.go @@ -1,163 +1,99 @@ -// FILE: lixenwraith/config/validator_test.go package config import ( + "fmt" "testing" - - "github.com/stretchr/testify/assert" ) -// TestPortValidator tests the Port validator -func TestPortValidator(t *testing.T) { - tests := []struct { - name string - port int64 - wantErr bool - }{ - {"ValidLowPort", 1, false}, - {"ValidCommonPort", 8080, false}, - {"ValidHighPort", 65535, false}, - {"InvalidZeroPort", 0, true}, - {"InvalidNegativePort", -1, true}, - {"InvalidTooHighPort", 65536, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := Port(tt.port) - if tt.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) +// Each value is a subtest, so one failure does not hide another boundary case. +func checkValidator[T any](t *testing.T, validate func(T) error, valid, invalid []T, errorText ...string) { + t.Helper() + for _, group := range []struct { + name string + values []T + wantError bool + }{{"valid", valid, false}, {"invalid", invalid, true}} { + t.Run(group.name, func(t *testing.T) { + for _, value := range group.values { + t.Run(fmt.Sprintf("%v", value), func(t *testing.T) { + err := validate(value) + if (err != nil) != group.wantError { + t.Fatalf("validate(%#v) = %v, want error: %v", value, err, group.wantError) + } + if group.wantError { + for _, fragment := range errorText { + checkErrorContains(t, err, fragment) + } + } + }) } }) } } -// TestPositiveValidator tests the Positive validator +func TestPortValidator(t *testing.T) { + checkValidator(t, Port, []int64{1, 8080, 65535}, []int64{0, -1, 65536}) +} + func TestPositiveValidator(t *testing.T) { t.Run("Int64", func(t *testing.T) { - assert.NoError(t, Positive(int64(1))) - assert.Error(t, Positive(int64(0))) - assert.Error(t, Positive(int64(-1))) + checkValidator(t, Positive[int64], []int64{1}, []int64{0, -1}) }) - t.Run("Float64", func(t *testing.T) { - assert.NoError(t, Positive(0.001)) - assert.Error(t, Positive(0.0)) - assert.Error(t, Positive(-0.001)) + checkValidator(t, Positive[float64], []float64{0.001}, []float64{0, -0.001}) }) } -// TestNonNegativeValidator tests the NonNegative validator func TestNonNegativeValidator(t *testing.T) { t.Run("Int64", func(t *testing.T) { - assert.NoError(t, NonNegative(int64(1))) - assert.NoError(t, NonNegative(int64(0))) - assert.Error(t, NonNegative(int64(-1))) + checkValidator(t, NonNegative[int64], []int64{1, 0}, []int64{-1}) }) - t.Run("Float64", func(t *testing.T) { - assert.NoError(t, NonNegative(0.001)) - assert.NoError(t, NonNegative(0.0)) - assert.Error(t, NonNegative(-0.001)) + checkValidator(t, NonNegative[float64], []float64{0.001, 0}, []float64{-0.001}) }) } -// TestIPAddressValidators tests all IP-related validators func TestIPAddressValidators(t *testing.T) { - t.Run("IPAddress", func(t *testing.T) { - assert.NoError(t, IPAddress("192.168.1.1")) - assert.NoError(t, IPAddress("2001:0db8:85a3:0000:0000:8a2e:0370:7334")) - assert.NoError(t, IPAddress("")) - assert.NoError(t, IPAddress("0.0.0.0")) - assert.NoError(t, IPAddress("::")) - assert.Error(t, IPAddress("not-an-ip")) - assert.Error(t, IPAddress("192.168.1.256")) - }) - - t.Run("IPv4Address", func(t *testing.T) { - assert.NoError(t, IPv4Address("192.168.1.1")) - assert.NoError(t, IPv4Address("")) - assert.NoError(t, IPv4Address("0.0.0.0")) - assert.Error(t, IPv4Address("::1")) // Is not IPv4 - assert.Error(t, IPv4Address("not-an-ip")) - }) - - t.Run("IPv6Address", func(t *testing.T) { - assert.NoError(t, IPv6Address("2001:db8::1")) - assert.NoError(t, IPv6Address("")) - assert.NoError(t, IPv6Address("::")) - assert.Error(t, IPv6Address("127.0.0.1")) // Is not IPv6 - assert.Error(t, IPv6Address("not-an-ip")) - }) + for _, tc := range []struct { + name string + validate func(string) error + valid, invalid []string + }{ + {"IPAddress", IPAddress, []string{"192.168.1.1", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "", "0.0.0.0", "::"}, []string{"not-an-ip", "192.168.1.256"}}, + {"IPv4Address", IPv4Address, []string{"192.168.1.1", "", "0.0.0.0"}, []string{"::1", "not-an-ip"}}, + {"IPv6Address", IPv6Address, []string{"2001:db8::1", "", "::"}, []string{"127.0.0.1", "not-an-ip"}}, + } { + t.Run(tc.name, func(t *testing.T) { checkValidator(t, tc.validate, tc.valid, tc.invalid) }) + } } -// TestURLPathValidator tests the URLPath validator func TestURLPathValidator(t *testing.T) { - assert.NoError(t, URLPath("/api/v1")) - assert.NoError(t, URLPath("/")) - assert.NoError(t, URLPath("")) - assert.Error(t, URLPath("api/v1")) - assert.Error(t, URLPath("no-slash")) + checkValidator(t, URLPath, []string{"/api/v1", "/", ""}, []string{"api/v1", "no-slash"}) } -// TestOneOfValidator tests the OneOf validator func TestOneOfValidator(t *testing.T) { t.Run("String", func(t *testing.T) { - validator := OneOf("prod", "dev", "staging") - assert.NoError(t, validator("prod")) - assert.NoError(t, validator("dev")) - err := validator("test") - assert.Error(t, err) - assert.Contains(t, err.Error(), "must be one of") + checkValidator(t, OneOf("prod", "dev", "staging"), []string{"prod", "dev"}, []string{"test"}, "must be one of") }) - t.Run("Int", func(t *testing.T) { - validator := OneOf(200, 404, 500) - assert.NoError(t, validator(404)) - err := validator(302) - assert.Error(t, err) - assert.Contains(t, err.Error(), "must be one of") + checkValidator(t, OneOf(200, 404, 500), []int{404}, []int{302}, "must be one of") }) } -// TestRangeValidator tests the Range validator func TestRangeValidator(t *testing.T) { t.Run("Int64", func(t *testing.T) { - validator := Range[int64](10, 100) - assert.NoError(t, validator(10)) - assert.NoError(t, validator(50)) - assert.NoError(t, validator(100)) - assert.Error(t, validator(9)) - assert.Error(t, validator(101)) + checkValidator(t, Range[int64](10, 100), []int64{10, 50, 100}, []int64{9, 101}) }) - t.Run("Float64", func(t *testing.T) { - validator := Range[float64](-1.5, 1.5) - assert.NoError(t, validator(-1.5)) - assert.NoError(t, validator(0.0)) - assert.NoError(t, validator(1.5)) - assert.Error(t, validator(-1.51)) - assert.Error(t, validator(1.51)) + checkValidator(t, Range[float64](-1.5, 1.5), []float64{-1.5, 0, 1.5}, []float64{-1.51, 1.51}) }) } -// TestPatternValidator tests the Pattern validator func TestPatternValidator(t *testing.T) { - // Simple email regex - validator := Pattern(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) - assert.NoError(t, validator("test@example.com")) - assert.NoError(t, validator("user.name+alias@domain.co.uk")) - assert.Error(t, validator("not-an-email")) - assert.Error(t, validator("test@example")) + checkValidator(t, Pattern(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`), + []string{"test@example.com", "user.name+alias@domain.co.uk"}, []string{"not-an-email", "test@example"}) } -// TestNonEmptyValidator tests the NonEmpty validator func TestNonEmptyValidator(t *testing.T) { - assert.NoError(t, NonEmpty("hello")) - assert.NoError(t, NonEmpty(" a ")) - assert.Error(t, NonEmpty("")) - assert.Error(t, NonEmpty(" ")) - assert.Error(t, NonEmpty(" \t\n ")) + checkValidator(t, NonEmpty, []string{"hello", " a "}, []string{"", " ", " \t\n "}) } diff --git a/watch_test.go b/watch_test.go index fe02d48..d949650 100644 --- a/watch_test.go +++ b/watch_test.go @@ -9,31 +9,13 @@ import ( "sync" "testing" "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -// Test-specific timing constants derived from production values -// These accelerate test execution while maintaining timing relationships +// Real watcher tests wait for events; debounce and timeout cases use manualWatcher. const ( - // testAcceleration reduces all intervals by this factor for faster tests - testAcceleration = 10 - - // Accelerated test timings - testPollInterval = DefaultPollInterval / testAcceleration // 100ms (from 1s) - testDebounce = DefaultDebounce / testAcceleration // 50ms (from 500ms) - testReloadTimeout = DefaultReloadTimeout / testAcceleration // 500ms (from 5s) - testShutdownTimeout = ShutdownTimeout // Keep original for safety - testSpinWaitInterval = SpinWaitInterval // Keep original for CPU efficiency - - // Test assertion timeouts - testEventuallyTimeout = testReloadTimeout // Aligns with reload timing - testWatchTimeout = 2 * DefaultPollInterval // 2s for change propagation - - // Derived test multipliers with clear purpose - testPollWindow = 3 * testPollInterval // 300ms change detection window - testStateStabilize = 4 * testDebounce // 200ms for state convergence + testPollInterval = DefaultPollInterval / 10 + testDebounce = DefaultDebounce / 10 + testWatchTimeout = 2 * DefaultPollInterval ) // TestAutoUpdate tests automatic configuration reloading @@ -50,7 +32,7 @@ host = "localhost" [features] enabled = true ` - require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0644)) + writeTestFile(t, configPath, []byte(initialConfig), 0644) // Create config with defaults type TestConfig struct { @@ -72,12 +54,14 @@ enabled = true WithDefaults(defaults). WithFile(configPath). Build() - require.NoError(t, err) + mustNoError(t, err) // Verify initial values port, exists := cfg.Get("server.port") - assert.True(t, exists) - assert.Equal(t, int64(8080), port) + if !exists { + t.Errorf("exists should be true") + } + checkEqual(t, port, int64(8080)) // Enable auto-update with fast polling opts := WatchOptions{ @@ -91,18 +75,6 @@ enabled = true // Start watching changes := cfg.Watch() - // Collect changes - var mu sync.Mutex - changedPaths := make(map[string]bool) - - go func() { - for path := range changes { - mu.Lock() - changedPaths[path] = true - mu.Unlock() - } - }() - // Update config file updatedConfig := ` [server] @@ -112,28 +84,30 @@ host = "0.0.0.0" [features] enabled = false ` - require.NoError(t, os.WriteFile(configPath, []byte(updatedConfig), 0644)) + writeTestFile(t, configPath, []byte(updatedConfig), 0644) - // Wait for changes to be detected - time.Sleep(testPollWindow) + // Wait for publication events, without a collector goroutine or fixed sleep. + changedPaths := make(map[string]bool) + for range 3 { + changedPaths[receiveWatchEvent(t, changes)] = true + } // Verify new values port, _ = cfg.Get("server.port") - assert.Equal(t, int64(9090), port) + checkEqual(t, port, int64(9090)) host, _ := cfg.Get("server.host") - assert.Equal(t, "0.0.0.0", host) + checkEqual(t, host, "0.0.0.0") enabled, _ := cfg.Get("features.enabled") - assert.Equal(t, false, enabled) + checkEqual(t, enabled, false) // Check that changes were notified - mu.Lock() - defer mu.Unlock() - expectedChanges := []string{"server.port", "server.host", "features.enabled"} for _, path := range expectedChanges { - assert.True(t, changedPaths[path], "Expected change notification for %s", path) + if !(changedPaths[path]) { + t.Errorf("changedPaths[path] should be true: %s", fmt.Sprintf("Expected change notification for %s", path)) + } } } @@ -143,12 +117,12 @@ func TestWatchFileDeleted(t *testing.T) { configPath := filepath.Join(tmpDir, "test.toml") // Create initial config - require.NoError(t, os.WriteFile(configPath, []byte(`test = "value"`), 0644)) + writeTestFile(t, configPath, []byte(`test = "value"`), 0644) cfg := New() cfg.Register("test", "default") - require.NoError(t, cfg.LoadFile(configPath)) + mustNoError(t, cfg.LoadFile(configPath)) // Enable watching opts := WatchOptions{ @@ -161,15 +135,9 @@ func TestWatchFileDeleted(t *testing.T) { changes := cfg.Watch() // Delete file - require.NoError(t, os.Remove(configPath)) + mustNoError(t, os.Remove(configPath)) - // Wait for deletion detection - select { - case path := <-changes: - assert.Equal(t, "file_deleted", path) - case <-time.After(testEventuallyTimeout): - t.Error("Timeout waiting for deletion notification") - } + checkEqual(t, receiveWatchEvent(t, changes), EventFileDeleted) } // TestWatchPermissionChange tests permission change detection @@ -183,11 +151,11 @@ func TestWatchPermissionChange(t *testing.T) { configPath := filepath.Join(tmpDir, "test.toml") // Create config with specific permissions - require.NoError(t, os.WriteFile(configPath, []byte(`test = "value"`), 0644)) + writeTestFile(t, configPath, []byte(`test = "value"`), 0644) cfg := New() cfg.Register("test", "default") - require.NoError(t, cfg.LoadFile(configPath)) + mustNoError(t, cfg.LoadFile(configPath)) // Enable watching with permission verification opts := WatchOptions{ @@ -201,15 +169,9 @@ func TestWatchPermissionChange(t *testing.T) { changes := cfg.Watch() // Change permissions to world-writable (security risk) - require.NoError(t, os.Chmod(configPath, 0666)) + mustNoError(t, os.Chmod(configPath, 0666)) - // Wait for permission change detection - select { - case path := <-changes: - assert.Equal(t, "permissions_changed", path) - case <-time.After(testEventuallyTimeout): - t.Error("Timeout waiting for permission change notification") - } + checkEqual(t, receiveWatchEvent(t, changes), EventPermissionsChanged) } // TestMaxWatchers tests watcher limit enforcement @@ -220,8 +182,8 @@ func TestMaxWatchers(t *testing.T) { // Create config file tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "test.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`test = "value"`), 0644)) - require.NoError(t, cfg.LoadFile(configPath)) + writeTestFile(t, configPath, []byte(`test = "value"`), 0644) + mustNoError(t, cfg.LoadFile(configPath)) // Enable watching with low max watchers opts := WatchOptions{ @@ -231,94 +193,45 @@ func TestMaxWatchers(t *testing.T) { cfg.AutoUpdateWithOptions(opts) defer cfg.StopAutoUpdate() - // Create maximum allowed watchers - channels := make([]<-chan string, 0, 4) - for i := 0; i < 4; i++ { + for i := range 4 { ch := cfg.Watch() - channels = append(channels, ch) - - // Check if channel is open - if i < 3 { - // First 3 should be open - select { - case _, ok := <-ch: - assert.True(t, ok || i < 3, "Channel %d should be open", i) - default: - // Channel is open and empty, expected + select { + case _, ok := <-ch: + if ok || i < 3 { + t.Errorf("subscriber %d closed unexpectedly or received a spurious event", i) } - } else { - // 4th should be closed immediately - select { - case _, ok := <-ch: - assert.False(t, ok, "Channel 3 should be closed (max watchers exceeded)") - case <-time.After(testEventuallyTimeout): - t.Error("Channel 3 should be closed immediately") + default: + if i == 3 { + t.Error("excess subscriber was not closed synchronously") } } } // Verify watcher count - assert.Equal(t, 3, cfg.WatcherCount()) + checkEqual(t, cfg.WatcherCount(), 3) } // TestRapidDebounce tests that rapid changes are debounced func TestRapidDebounce(t *testing.T) { - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "test.toml") - - // Create initial config - require.NoError(t, os.WriteFile(configPath, []byte(`value = 1`), 0644)) - - cfg := New() - cfg.Register("value", 0) - require.NoError(t, cfg.LoadFile(configPath)) - - // Enable watching with longer debounce - opts := WatchOptions{ - PollInterval: testDebounce, - Debounce: testStateStabilize, - } - cfg.AutoUpdateWithOptions(opts) - defer cfg.StopAutoUpdate() - - changes := cfg.Watch() - - var changeCount int - var mu sync.Mutex - done := make(chan bool) - - go func() { - for { - select { - case <-changes: - mu.Lock() - changeCount++ - mu.Unlock() - case <-done: - return - } - } - }() - - // Make rapid changes + cfg, path := watcherFixture(t) + w := manualWatcher(t, cfg, path) + w.opts.Debounce = time.Hour + changes := w.subscribe() for i := 2; i <= 5; i++ { - content := fmt.Sprintf(`value = %d`, i) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - time.Sleep(testDebounce) // Less than debounce period + writeTestFile(t, path, []byte(fmt.Sprintf("value = %d", i)), 0600) + w.checkAndReload(cfg) + if len(changes) != 0 { + t.Fatal("change published before the quiet interval") + } } - - // Wait for debounce to complete - time.Sleep(2 * testStateStabilize) - done <- true - - // Should only see one change due to debounce - mu.Lock() - defer mu.Unlock() - assert.Equal(t, 1, changeCount, "Expected 1 change due to debounce, got %d", changeCount) - - // Verify final value - val, _ := cfg.Get("value") - assert.Equal(t, int64(5), val) + w.observedAt = time.Now().Add(-2 * time.Hour) + w.checkAndReload(cfg) + checkEqual(t, receiveWatchEvent(t, changes), "value") + if len(changes) != 0 { + t.Fatal("debounced changes emitted multiple events") + } + value, _ := cfg.Get("value") + checkEqual(t, value, int64(5)) } // TestWatchWithoutFile tests watching behavior when no file is configured @@ -331,23 +244,27 @@ func TestWatchWithoutFile(t *testing.T) { select { case _, ok := <-ch: - assert.False(t, ok, "Channel should be closed when no file to watch") + if ok { + t.Errorf("ok should be false: %s", "Channel should be closed when no file to watch") + } case <-time.After(10 * time.Millisecond): t.Error("Channel should be closed immediately") } - assert.False(t, cfg.IsWatching()) + if cfg.IsWatching() { + t.Errorf("cfg.IsWatching() should be false") + } } // TestConcurrentWatchOperations tests thread safety of watch operations func TestConcurrentWatchOperations(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "test.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`value = 1`), 0644)) + writeTestFile(t, configPath, []byte(`value = 1`), 0644) cfg := New() cfg.Register("value", 0) - require.NoError(t, cfg.LoadFile(configPath)) + mustNoError(t, cfg.LoadFile(configPath)) opts := WatchOptions{ PollInterval: testDebounce, @@ -400,15 +317,7 @@ func TestConcurrentWatchOperations(t *testing.T) { go func(id int) { defer wg.Done() - isWatching := false - for j := 0; j < 5; j++ { // Poll a few times, double-dip wait for goroutine to start - if cfg.IsWatching() { - isWatching = true - break - } - time.Sleep(2 * SpinWaitInterval) - } - if !isWatching { + if !cfg.IsWatching() { errors <- fmt.Errorf("checker %d: IsWatching returned false", id) } }(i) @@ -422,46 +331,24 @@ func TestConcurrentWatchOperations(t *testing.T) { for err := range errors { errs = append(errs, err) } - assert.Empty(t, errs, "Concurrent operations should not produce errors") -} - -// TestReloadTimeout tests reload timeout handling -func TestReloadTimeout(t *testing.T) { - // This test would require mocking file operations to simulate a slow read - // For now, we'll test that timeout option is respected in configuration - - cfg := New() - cfg.Register("test", "value") - - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "test.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`test = "value"`), 0644)) - require.NoError(t, cfg.LoadFile(configPath)) - - // Very short timeout - opts := WatchOptions{ - PollInterval: testPollInterval, - ReloadTimeout: 1 * time.Nanosecond, + if got := len(errs); got != 0 { + t.Errorf("length = %d, want %d: %s", got, 0, "Concurrent operations should not produce errors") } - cfg.AutoUpdateWithOptions(opts) - defer cfg.StopAutoUpdate() - - waitForWatchingState(t, cfg, true) } // TestStopAutoUpdate tests clean shutdown of watcher func TestStopAutoUpdate(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "test.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`test = "value"`), 0644)) + writeTestFile(t, configPath, []byte(`test = "value"`), 0644) cfg := New() cfg.Register("test", "value") - require.NoError(t, cfg.LoadFile(configPath)) + mustNoError(t, cfg.LoadFile(configPath)) // Start watching cfg.AutoUpdate() - waitForWatchingState(t, cfg, true, "Watcher should be active after first start") + checkWatchingState(t, cfg, true, "Watcher should be active after first start") ch := cfg.Watch() @@ -469,20 +356,22 @@ func TestStopAutoUpdate(t *testing.T) { cfg.StopAutoUpdate() // Verify stopped - waitForWatchingState(t, cfg, false, "Watcher should be inactive after stop") - assert.Equal(t, 0, cfg.WatcherCount()) + checkWatchingState(t, cfg, false, "Watcher should be inactive after stop") + checkEqual(t, cfg.WatcherCount(), 0) // Channel should eventually close select { case _, ok := <-ch: - assert.False(t, ok, "Channel should be closed after stop") + if ok { + t.Errorf("ok should be false: %s", "Channel should be closed after stop") + } case <-time.After(ShutdownTimeout): - // OK, channel might not close immediately + t.Fatal("watch channel did not close after stop") } // Starting again should work cfg.AutoUpdate() - waitForWatchingState(t, cfg, true, "Watcher should be active after restart") + checkWatchingState(t, cfg, true, "Watcher should be active after restart") cfg.StopAutoUpdate() } @@ -496,13 +385,13 @@ func BenchmarkWatchOverhead(b *testing.B) { for i := 0; i < 100; i++ { configContent += fmt.Sprintf("value%d = %d\n", i, i) } - require.NoError(b, os.WriteFile(configPath, []byte(configContent), 0644)) + writeTestFile(b, configPath, []byte(configContent), 0644) cfg := New() for i := 0; i < 100; i++ { cfg.Register(fmt.Sprintf("value%d", i), 0) } - require.NoError(b, cfg.LoadFile(configPath)) + mustNoError(b, cfg.LoadFile(configPath)) // Enable watching opts := WatchOptions{ diff --git a/watcher_hardening_test.go b/watcher_hardening_test.go index 87f5cb2..684445a 100644 --- a/watcher_hardening_test.go +++ b/watcher_hardening_test.go @@ -122,7 +122,7 @@ func TestWatcherDebouncesErrorsAndRecovers(t *testing.T) { t.Fatal(event) } // A stable invalid edit is reported once and leaves the old value intact. - _ = os.WriteFile(path, []byte("value=256\n"), 0600) + writeTestFile(t, path, []byte("value=256\n"), 0600) w.checkAndReload(c) w.observedAt = time.Now().Add(-2 * time.Hour) w.checkAndReload(c) @@ -208,7 +208,7 @@ func TestWatcherSameMetadataReplacementAndRecreation(t *testing.T) { w.checkAndReload(c) info, _ := os.Stat(path) replacement := path + ".new" - _ = os.WriteFile(replacement, []byte("value=2\n"), 0600) + writeTestFile(t, replacement, []byte("value=2\n"), 0600) _ = os.Chtimes(replacement, info.ModTime(), info.ModTime()) if err := os.Rename(replacement, path); err != nil { t.Fatal(err) @@ -233,7 +233,7 @@ func TestWatcherSameMetadataReplacementAndRecreation(t *testing.T) { t.Fatalf("repeated deletion: %s", event) default: } - _ = os.WriteFile(path, []byte("value=3\n"), 0600) + writeTestFile(t, path, []byte("value=3\n"), 0600) w.checkAndReload(c) if value, _ := c.Get("value"); value != int64(3) { t.Fatal("recreation missed")