From ed76a36ab9b4c8f5a56049aa201dd8ce04836ecb Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 15:37:08 +1200 Subject: [PATCH 01/10] refactor(cmd/helm): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- cmd/helm/helm_test.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index a9362f772..05d05dece 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -18,13 +18,13 @@ package main import ( "bytes" - "errors" "os" "os/exec" "runtime" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCliPluginExitCode(t *testing.T) { @@ -62,20 +62,13 @@ func TestCliPluginExitCode(t *testing.T) { err := cmd.Run() exiterr := &exec.ExitError{} - ok := errors.As(err, &exiterr) - if !ok { - t.Fatalf("Unexpected error type returned by os.Exit: %T", err) - } + require.ErrorAs(t, err, &exiterr) assert.Empty(t, stdout.String()) expectedStderr := "level=WARN msg=\"failed to load plugin (ignoring)\" plugin_yaml=../../pkg/cmd/testdata/helmhome/helm/plugins/noversion/plugin.yaml error=\"failed to load plugin \\\"../../pkg/cmd/testdata/helmhome/helm/plugins/noversion\\\": plugin `version` is required\"\nError: plugin \"exitwith\" exited with error\n" - if stderr.String() != expectedStderr { - t.Errorf("Expected %q written to stderr: Got %q", expectedStderr, stderr.String()) - } + assert.Equal(t, expectedStderr, stderr.String()) - if exiterr.ExitCode() != 43 { - t.Errorf("Expected exit code 43: Got %d", exiterr.ExitCode()) - } + assert.Equal(t, 43, exiterr.ExitCode()) } } From f4d713abb210230b3935c4a9685f1f34a8330970 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 16:17:14 +1200 Subject: [PATCH 02/10] refactor(pkg): convert tests to testify assert/require Convert pkg/gates, pkg/kubeenv, pkg/ignore, and pkg/helmpath test files from native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) to github.com/stretchr/testify equivalents (assert.X, require.X). Signed-off-by: George Jenkins --- pkg/gates/gates_test.go | 18 ++++------ pkg/helmpath/home_unix_test.go | 20 ++++------- pkg/helmpath/home_windows_test.go | 15 ++++---- pkg/helmpath/lazypath_darwin_test.go | 25 ++++--------- pkg/helmpath/lazypath_unix_test.go | 25 ++++--------- pkg/helmpath/lazypath_windows_test.go | 25 ++++--------- pkg/ignore/rules_test.go | 52 +++++++++------------------ 7 files changed, 56 insertions(+), 124 deletions(-) diff --git a/pkg/gates/gates_test.go b/pkg/gates/gates_test.go index 4d77199e6..217ee1bbe 100644 --- a/pkg/gates/gates_test.go +++ b/pkg/gates/gates_test.go @@ -18,6 +18,8 @@ package gates import ( "os" "testing" + + "github.com/stretchr/testify/assert" ) const name string = "HELM_EXPERIMENTAL_FEATURE" @@ -25,31 +27,23 @@ const name string = "HELM_EXPERIMENTAL_FEATURE" func TestIsEnabled(t *testing.T) { g := Gate(name) - if g.IsEnabled() { - t.Errorf("feature gate shows as available, but the environment variable %s was not set", name) - } + assert.False(t, g.IsEnabled(), "feature gate shows as available, but the environment variable %s was not set", name) t.Setenv(name, "1") - if !g.IsEnabled() { - t.Errorf("feature gate shows as disabled, but the environment variable %s was set", name) - } + assert.True(t, g.IsEnabled(), "feature gate shows as disabled, but the environment variable %s was set", name) } func TestError(t *testing.T) { os.Unsetenv(name) g := Gate(name) - if g.Error().Error() != "this feature has been marked as experimental and is not enabled by default. Please set HELM_EXPERIMENTAL_FEATURE=1 in your environment to use this feature" { - t.Errorf("incorrect error message. Received %s", g.Error().Error()) - } + assert.Equal(t, "this feature has been marked as experimental and is not enabled by default. Please set HELM_EXPERIMENTAL_FEATURE=1 in your environment to use this feature", g.Error().Error()) } func TestString(t *testing.T) { os.Unsetenv(name) g := Gate(name) - if g.String() != "HELM_EXPERIMENTAL_FEATURE" { - t.Errorf("incorrect string representation. Received %s", g.String()) - } + assert.Equal(t, "HELM_EXPERIMENTAL_FEATURE", g.String()) } diff --git a/pkg/helmpath/home_unix_test.go b/pkg/helmpath/home_unix_test.go index a64c9bcd6..84c912502 100644 --- a/pkg/helmpath/home_unix_test.go +++ b/pkg/helmpath/home_unix_test.go @@ -16,9 +16,10 @@ package helmpath import ( - "runtime" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/helmpath/xdg" ) @@ -26,20 +27,13 @@ func TestHelmHome(t *testing.T) { t.Setenv(xdg.CacheHomeEnvVar, "/cache") t.Setenv(xdg.ConfigHomeEnvVar, "/config") t.Setenv(xdg.DataHomeEnvVar, "/data") - isEq := func(t *testing.T, got, expected string) { - t.Helper() - if expected != got { - t.Error(runtime.GOOS) - t.Errorf("Expected %q, got %q", expected, got) - } - } - - isEq(t, CachePath(), "/cache/helm") - isEq(t, ConfigPath(), "/config/helm") - isEq(t, DataPath(), "/data/helm") + + assert.Equal(t, "/cache/helm", CachePath()) + assert.Equal(t, "/config/helm", ConfigPath()) + assert.Equal(t, "/data/helm", DataPath()) // test to see if lazy-loading environment variables at runtime works t.Setenv(xdg.CacheHomeEnvVar, "/cache2") - isEq(t, CachePath(), "/cache2/helm") + assert.Equal(t, "/cache2/helm", CachePath()) } diff --git a/pkg/helmpath/home_windows_test.go b/pkg/helmpath/home_windows_test.go index 38fe5e4f1..4c2c7d11a 100644 --- a/pkg/helmpath/home_windows_test.go +++ b/pkg/helmpath/home_windows_test.go @@ -19,6 +19,8 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/helmpath/xdg" ) @@ -26,18 +28,13 @@ func TestHelmHome(t *testing.T) { os.Setenv(xdg.CacheHomeEnvVar, "c:\\") os.Setenv(xdg.ConfigHomeEnvVar, "d:\\") os.Setenv(xdg.DataHomeEnvVar, "e:\\") - isEq := func(t *testing.T, a, b string) { - if a != b { - t.Errorf("Expected %q, got %q", b, a) - } - } - isEq(t, CachePath(), "c:\\helm") - isEq(t, ConfigPath(), "d:\\helm") - isEq(t, DataPath(), "e:\\helm") + assert.Equal(t, "c:\\helm", CachePath()) + assert.Equal(t, "d:\\helm", ConfigPath()) + assert.Equal(t, "e:\\helm", DataPath()) // test to see if lazy-loading environment variables at runtime works os.Setenv(xdg.CacheHomeEnvVar, "f:\\") - isEq(t, CachePath(), "f:\\helm") + assert.Equal(t, "f:\\helm", CachePath()) } diff --git a/pkg/helmpath/lazypath_darwin_test.go b/pkg/helmpath/lazypath_darwin_test.go index e3006d0d5..143750f04 100644 --- a/pkg/helmpath/lazypath_darwin_test.go +++ b/pkg/helmpath/lazypath_darwin_test.go @@ -20,6 +20,7 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" "k8s.io/client-go/util/homedir" "helm.sh/helm/v4/pkg/helmpath/xdg" @@ -36,17 +37,13 @@ func TestDataPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "Library", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) t.Setenv(xdg.DataHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) } func TestConfigPath(t *testing.T) { @@ -54,17 +51,13 @@ func TestConfigPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "Library", "Preferences", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) t.Setenv(xdg.ConfigHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) } func TestCachePath(t *testing.T) { @@ -72,15 +65,11 @@ func TestCachePath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "Library", "Caches", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) t.Setenv(xdg.CacheHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) } diff --git a/pkg/helmpath/lazypath_unix_test.go b/pkg/helmpath/lazypath_unix_test.go index 4b0f2429b..6f8c6c2e8 100644 --- a/pkg/helmpath/lazypath_unix_test.go +++ b/pkg/helmpath/lazypath_unix_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" "k8s.io/client-go/util/homedir" "helm.sh/helm/v4/pkg/helmpath/xdg" @@ -33,47 +34,35 @@ const ( func TestDataPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), ".local", "share", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) t.Setenv(xdg.DataHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) } func TestConfigPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), ".config", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) t.Setenv(xdg.ConfigHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) } func TestCachePath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), ".cache", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) t.Setenv(xdg.CacheHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) } diff --git a/pkg/helmpath/lazypath_windows_test.go b/pkg/helmpath/lazypath_windows_test.go index ebd95e812..df8ad2d80 100644 --- a/pkg/helmpath/lazypath_windows_test.go +++ b/pkg/helmpath/lazypath_windows_test.go @@ -20,6 +20,7 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" "k8s.io/client-go/util/homedir" "helm.sh/helm/v4/pkg/helmpath/xdg" @@ -37,17 +38,13 @@ func TestDataPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "foo", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) os.Setenv(xdg.DataHomeEnvVar, filepath.Join(homedir.HomeDir(), "xdg")) expected = filepath.Join(homedir.HomeDir(), "xdg", appName, testFile) - if lazy.dataPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) - } + assert.Equal(t, expected, lazy.dataPath(testFile)) } func TestConfigPath(t *testing.T) { @@ -56,17 +53,13 @@ func TestConfigPath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "foo", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) os.Setenv(xdg.ConfigHomeEnvVar, filepath.Join(homedir.HomeDir(), "xdg")) expected = filepath.Join(homedir.HomeDir(), "xdg", appName, testFile) - if lazy.configPath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) - } + assert.Equal(t, expected, lazy.configPath(testFile)) } func TestCachePath(t *testing.T) { @@ -75,15 +68,11 @@ func TestCachePath(t *testing.T) { expected := filepath.Join(homedir.HomeDir(), "foo", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) os.Setenv(xdg.CacheHomeEnvVar, filepath.Join(homedir.HomeDir(), "xdg")) expected = filepath.Join(homedir.HomeDir(), "xdg", appName, testFile) - if lazy.cachePath(testFile) != expected { - t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) - } + assert.Equal(t, expected, lazy.cachePath(testFile)) } diff --git a/pkg/ignore/rules_test.go b/pkg/ignore/rules_test.go index 9581cf09f..02ba63588 100644 --- a/pkg/ignore/rules_test.go +++ b/pkg/ignore/rules_test.go @@ -21,6 +21,9 @@ import ( "os" "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var testdata = "./testdata" @@ -36,22 +39,14 @@ baz/bar/foo.txt one/more ` r, err := parseString(rules) - if err != nil { - t.Fatalf("Error parsing rules: %s", err) - } + require.NoError(t, err, "Error parsing rules") - if len(r.patterns) != 4 { - t.Errorf("Expected 4 rules, got %d", len(r.patterns)) - } + assert.Len(t, r.patterns, 4) expects := []string{"foo", "bar/*", "baz/bar/foo.txt", "one/more"} for i, p := range r.patterns { - if p.raw != expects[i] { - t.Errorf("Expected %q, got %q", expects[i], p.raw) - } - if p.match == nil { - t.Errorf("Expected %s to have a matcher function.", p.raw) - } + assert.Equal(t, expects[i], p.raw) + assert.NotNil(t, p.match, "Expected %s to have a matcher function.", p.raw) } } @@ -59,26 +54,19 @@ func TestParseFail(t *testing.T) { shouldFail := []string{"foo/**/bar", "[z-"} for _, fail := range shouldFail { _, err := parseString(fail) - if err == nil { - t.Errorf("Rule %q should have failed", fail) - } + assert.Error(t, err, "Rule %q should have failed", fail) } } func TestParseFile(t *testing.T) { f := filepath.Join(testdata, HelmIgnore) - if _, err := os.Stat(f); err != nil { - t.Fatalf("Fixture %s missing: %s", f, err) - } + _, err := os.Stat(f) + require.NoError(t, err, "Fixture %s missing", f) r, err := ParseFile(f) - if err != nil { - t.Fatalf("Failed to parse rules file: %s", err) - } + require.NoError(t, err, "Failed to parse rules file") - if len(r.patterns) != 3 { - t.Errorf("Expected 3 patterns, got %d", len(r.patterns)) - } + assert.Len(t, r.patterns, 3) } func TestIgnore(t *testing.T) { @@ -126,17 +114,11 @@ func TestIgnore(t *testing.T) { for _, test := range tests { r, err := parseString(test.pattern) - if err != nil { - t.Fatalf("Failed to parse: %s", err) - } + require.NoError(t, err, "Failed to parse: %s", test.pattern) fi, err := os.Stat(filepath.Join(testdata, test.name)) - if err != nil { - t.Fatalf("Fixture missing: %s", err) - } + require.NoError(t, err, "Fixture missing: %s", test.name) - if r.Ignore(test.name, fi) != test.expect { - t.Errorf("Expected %q to be %v for pattern %q", test.name, test.expect, test.pattern) - } + assert.Equal(t, test.expect, r.Ignore(test.name, fi), "Expected %q to be %v for pattern %q", test.name, test.expect, test.pattern) } } @@ -144,9 +126,7 @@ func TestAddDefaults(t *testing.T) { r := Rules{} r.AddDefaults() - if len(r.patterns) != 1 { - t.Errorf("Expected 1 default patterns, got %d", len(r.patterns)) - } + assert.Len(t, r.patterns, 1) } func parseString(str string) (*Rules, error) { From 9ad5620edc1b51507d111f7f998cc0f5b95e5726 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 12:16:54 -0700 Subject: [PATCH 03/10] Update pkg/ignore/rules_test.go Co-authored-by: Terry Howe Signed-off-by: George Jenkins --- pkg/ignore/rules_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ignore/rules_test.go b/pkg/ignore/rules_test.go index 02ba63588..30e157a52 100644 --- a/pkg/ignore/rules_test.go +++ b/pkg/ignore/rules_test.go @@ -41,7 +41,7 @@ one/more r, err := parseString(rules) require.NoError(t, err, "Error parsing rules") - assert.Len(t, r.patterns, 4) + require.Len(t, r.patterns, 4) expects := []string{"foo", "bar/*", "baz/bar/foo.txt", "one/more"} for i, p := range r.patterns { From daa045d4923007c1ab815b39038b81491506dc0a Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 17 Jun 2026 21:52:08 +1200 Subject: [PATCH 04/10] refactor(pkg/strvals): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- pkg/strvals/literal_parser_test.go | 45 +++++------ pkg/strvals/parser_test.go | 119 +++++++++++++---------------- 2 files changed, 73 insertions(+), 91 deletions(-) diff --git a/pkg/strvals/literal_parser_test.go b/pkg/strvals/literal_parser_test.go index cc3d73d50..d7892844f 100644 --- a/pkg/strvals/literal_parser_test.go +++ b/pkg/strvals/literal_parser_test.go @@ -20,6 +20,9 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" ) @@ -278,27 +281,25 @@ func TestParseLiteral(t *testing.T) { got, err := ParseLiteral(tt.str) if err != nil { if !tt.err { - t.Fatalf("%s: %s", tt.str, err) + require.NoError(t, err, tt.str) } continue } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.str) + assert.Fail(t, "Expected error. Got nil", tt.str) } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.str) } } } @@ -385,33 +386,31 @@ func TestParseLiteralInto(t *testing.T) { for _, tt := range tests { if err := ParseLiteralInto(tt.input, tt.got); err != nil { - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.input) + assert.Fail(t, "Expected error. Got nil", tt.input) } if tt.input2 != "" { if err := ParseLiteralInto(tt.input2, tt.got); err != nil { - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.input2) + assert.Fail(t, "Expected error. Got nil", tt.input2) } } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(tt.got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.input, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.input) } } } @@ -452,30 +451,28 @@ func TestParseLiteralNestedLevels(t *testing.T) { if tt.err { if tt.errStr != "" { if err.Error() != tt.errStr { - t.Errorf("Expected error: %s. Got error: %s", tt.errStr, err.Error()) + assert.Equal(t, tt.errStr, err.Error()) } } continue } - t.Fatalf("%s: %s", tt.str, err) + require.NoError(t, err, tt.str) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.str) + assert.Fail(t, "Expected error. Got nil", tt.str) } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.str) } } } diff --git a/pkg/strvals/parser_test.go b/pkg/strvals/parser_test.go index e3f8b9d8e..1a9559236 100644 --- a/pkg/strvals/parser_test.go +++ b/pkg/strvals/parser_test.go @@ -20,6 +20,9 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" ) @@ -78,24 +81,24 @@ func TestSetIndex(t *testing.T) { got, err := setIndex(tt.initial, tt.add, tt.val) if err != nil && tt.err == false { - t.Fatalf("%s: Expected no error but error returned", tt.name) + require.Fail(t, "Expected no error but error returned", tt.name) } else if err == nil && tt.err == true { - t.Fatalf("%s: Expected error but no error returned", tt.name) + require.Fail(t, "Expected error but no error returned", tt.name) } if len(got) != len(tt.expect) { - t.Fatalf("%s: Expected length %d, got %d", tt.name, len(tt.expect), len(got)) + require.Len(t, got, len(tt.expect), tt.name) } if !tt.err { if gg := got[tt.add].(int); gg != tt.val { - t.Errorf("%s, Expected value %d, got %d", tt.name, tt.val, gg) + assert.Equal(t, tt.val, gg, tt.name) } } for k, v := range got { if v != tt.expect[k] { - t.Errorf("%s, Expected value %d, got %d", tt.name, tt.expect[k], v) + assert.Equal(t, tt.expect[k], v, tt.name) } } } @@ -384,23 +387,21 @@ func TestParseSet(t *testing.T) { if tt.err { continue } - t.Fatalf("%s: %s", tt.str, err) + require.NoError(t, err, tt.str) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.str) + assert.Fail(t, "Expected error. Got nil", tt.str) } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.str) } } for _, tt := range testsString { @@ -409,23 +410,21 @@ func TestParseSet(t *testing.T) { if tt.err { continue } - t.Fatalf("%s: %s", tt.str, err) + require.NoError(t, err, tt.str) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.str) + assert.Fail(t, "Expected error. Got nil", tt.str) } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.str) } } } @@ -513,32 +512,30 @@ func TestParseInto(t *testing.T) { } for _, tt := range tests { if err := ParseInto(tt.input, tt.got); err != nil { - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.input) + assert.Fail(t, "Expected error. Got nil", tt.input) } if tt.input2 != "" { if err := ParseInto(tt.input2, tt.got); err != nil { - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.input2) + assert.Fail(t, "Expected error. Got nil", tt.input2) } } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(tt.got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.input, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.input) } } } @@ -560,20 +557,18 @@ func TestParseIntoString(t *testing.T) { } if err := ParseIntoString(input, got); err != nil { - t.Fatal(err) + require.NoError(t, err) } y1, err := yaml.Marshal(expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", input, y1, y2) + assert.Equal(t, string(y1), string(y2), input) } } @@ -658,22 +653,22 @@ func TestParseJSON(t *testing.T) { if tt.err { continue } - t.Fatalf("%s: %s", tt.input, err) + require.NoError(t, err, tt.input) } if tt.err { - t.Fatalf("%s: Expected error. Got nil", tt.input) + require.Fail(t, "Expected error. Got nil", tt.input) } y1, err := yaml.Marshal(tt.expect) if err != nil { - t.Fatalf("Error serializing expected value: %s", err) + require.NoError(t, err, "Error serializing expected value") } y2, err := yaml.Marshal(tt.got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.input, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.input) } } } @@ -686,28 +681,24 @@ func TestParseFile(t *testing.T) { rs2v := func(rs []rune) (any, error) { v := string(rs) if v != "path1" { - t.Errorf("%s: runesToVal: Expected value path1, got %s", input, v) + assert.Equal(t, "path1", v, "%s: runesToVal", input) return "", nil } return "value1", nil } got, err := ParseFile(input, rs2v) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y1, err := yaml.Marshal(expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", input, y1, y2) + assert.Equal(t, string(y1), string(y2), input) } } @@ -720,27 +711,25 @@ func TestParseIntoFile(t *testing.T) { rs2v := func(rs []rune) (any, error) { v := string(rs) if v != "path1" { - t.Errorf("%s: runesToVal: Expected value path1, got %s", input, v) + assert.Equal(t, "path1", v, "%s: runesToVal", input) return "", nil } return "value1", nil } if err := ParseIntoFile(input, got, rs2v); err != nil { - t.Fatal(err) + require.NoError(t, err) } y1, err := yaml.Marshal(expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", input, y1, y2) + assert.Equal(t, string(y1), string(y2), input) } } @@ -748,12 +737,10 @@ func TestToYAML(t *testing.T) { // The TestParse does the hard part. We just verify that YAML formatting is // happening. o, err := ToYAML("name=value") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) expect := "name: value" if o != expect { - t.Errorf("Expected %q, got %q", expect, o) + assert.Equal(t, expect, o) } } @@ -792,28 +779,26 @@ func TestParseSetNestedLevels(t *testing.T) { if tt.err { if tt.errStr != "" { if err.Error() != tt.errStr { - t.Errorf("Expected error: %s. Got error: %s", tt.errStr, err.Error()) + assert.Equal(t, tt.errStr, err.Error()) } } continue } - t.Fatalf("%s: %s", tt.str, err) + require.NoError(t, err, tt.str) } if tt.err { - t.Errorf("%s: Expected error. Got nil", tt.str) + assert.Fail(t, "Expected error. Got nil", tt.str) } y1, err := yaml.Marshal(tt.expect) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) y2, err := yaml.Marshal(got) if err != nil { - t.Fatalf("Error serializing parsed value: %s", err) + require.NoError(t, err, "Error serializing parsed value") } if string(y1) != string(y2) { - t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2) + assert.Equal(t, string(y1), string(y2), tt.str) } } } From b42faba9d293d45d2654dac5b540adb5b50e5b4f Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 15:15:19 +1200 Subject: [PATCH 05/10] refactor(repotest): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- pkg/repo/v1/repotest/server.go | 76 +++++++++-------------------- pkg/repo/v1/repotest/server_test.go | 70 +++++++------------------- 2 files changed, 40 insertions(+), 106 deletions(-) diff --git a/pkg/repo/v1/repotest/server.go b/pkg/repo/v1/repotest/server.go index 61dcd6931..06cdc9172 100644 --- a/pkg/repo/v1/repotest/server.go +++ b/pkg/repo/v1/repotest/server.go @@ -30,6 +30,8 @@ import ( "github.com/distribution/distribution/v3/registry" _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" // used for docker test registry _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" // used for docker test registry + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/crypto/bcrypt" "sigs.k8s.io/yaml" @@ -44,9 +46,8 @@ func BasicAuthMiddleware(t *testing.T) http.HandlerFunc { t.Helper() return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) - } + assert.True(t, ok && username == "username" && password == "password", + "Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) }) } @@ -97,9 +98,8 @@ func NewTempServer(t *testing.T, options ...ServerOption) *Server { t.Cleanup(func() { os.RemoveAll(srv.docroot) }) if srv.chartSourceGlob != "" { - if _, err := srv.CopyCharts(srv.chartSourceGlob); err != nil { - t.Fatal(err) - } + _, err := srv.CopyCharts(srv.chartSourceGlob) + require.NoError(t, err) } return srv @@ -109,9 +109,7 @@ func NewTempServer(t *testing.T, options ...ServerOption) *Server { func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { t.Helper() absdocroot, err := filepath.Abs(docroot) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) s := &Server{ docroot: absdocroot, @@ -131,9 +129,7 @@ func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { s.start() // Add the testing repository as the only repo. Server must be started for the server's URL to be valid - if err := setTestingRepository(s.URL(), filepath.Join(s.docroot, "repositories.yaml")); err != nil { - t.Fatal(err) - } + require.NoError(t, setTestingRepository(s.URL(), filepath.Join(s.docroot, "repositories.yaml"))) return s } @@ -169,22 +165,16 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { testUsername, testPassword := "username", "password" pwBytes, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.DefaultCost) - if err != nil { - t.Fatal("error generating bcrypt password for test htpasswd file") - } + require.NoError(t, err, "error generating bcrypt password for test htpasswd file") htpasswdPath := filepath.Join(dir, testHtpasswdFileBasename) err = os.WriteFile(htpasswdPath, fmt.Appendf(nil, "%s:%s\n", testUsername, string(pwBytes)), 0o644) - if err != nil { - t.Fatal("error creating test htpasswd file") - } + require.NoError(t, err, "error creating test htpasswd file") // Registry config config := &configuration.Configuration{} lnCfg := net.ListenConfig{} ln, err := lnCfg.Listen(t.Context(), "tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("error finding free port for test registry: %v", err) - } + require.NoError(t, err, "error finding free port for test registry") defer ln.Close() port := ln.Addr().(*net.TCPAddr).Port @@ -201,9 +191,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { registryURL := fmt.Sprintf("localhost:%d", port) r, err := registry.NewRegistry(t.Context(), config) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) return &OCIServer{ Registry: r, @@ -237,53 +225,37 @@ func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServ ociRegistry.ClientOptWriter(os.Stdout), ociRegistry.ClientOptCredentialsFile(credentialsFile), ) - if err != nil { - t.Fatalf("error creating registry client: %v", err) - } + require.NoError(t, err, "error creating registry client") err = registryClient.Login( srv.RegistryURL, ociRegistry.LoginOptBasicAuth(srv.TestUsername, srv.TestPassword), ociRegistry.LoginOptInsecure(true), ociRegistry.LoginOptPlainText(true)) - if err != nil { - t.Fatalf("error logging into registry with good credentials: %v", err) - } + require.NoError(t, err, "error logging into registry with good credentials") ref := srv.RegistryURL + "/u/ocitestuser/oci-dependent-chart:0.1.0" err = chartutil.ExpandFile(srv.Dir, filepath.Join(srv.Dir, "oci-dependent-chart-0.1.0.tgz")) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // valid chart ch, err := loader.LoadDir(filepath.Join(srv.Dir, "oci-dependent-chart")) - if err != nil { - t.Fatal("error loading chart") - } + require.NoError(t, err, "error loading chart") err = os.RemoveAll(filepath.Join(srv.Dir, "oci-dependent-chart")) - if err != nil { - t.Fatal("error removing chart before push") - } + require.NoError(t, err, "error removing chart before push") // save it back to disk.. absPath, err := chartutil.Save(ch, srv.Dir) - if err != nil { - t.Fatal("could not create chart archive") - } + require.NoError(t, err, "could not create chart archive") // load it into memory... contentBytes, err := os.ReadFile(absPath) - if err != nil { - t.Fatal("could not load chart into memory") - } + require.NoError(t, err, "could not load chart into memory") result, err := registryClient.Push(contentBytes, ref) - if err != nil { - t.Fatalf("error pushing dependent chart: %s", err) - } + require.NoError(t, err, "error pushing dependent chart") t.Logf("Manifest.Digest: %s, Manifest.Size: %d, "+ "Config.Digest: %s, Config.Size: %d, "+ "Chart.Digest: %s, Chart.Size: %d", @@ -306,14 +278,10 @@ func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServ absPath = filepath.Join(srv.Dir, fmt.Sprintf("%s-%s.tgz", c.Metadata.Name, c.Metadata.Version)) contentBytes, err = os.ReadFile(absPath) - if err != nil { - t.Fatal("could not load chart into memory") - } + require.NoError(t, err, "could not load chart into memory") result, err = registryClient.Push(contentBytes, dependingRef) - if err != nil { - t.Fatalf("error pushing depending chart: %s", err) - } + require.NoError(t, err, "error pushing depending chart") t.Logf("Manifest.Digest: %s, Manifest.Size: %d, "+ "Config.Digest: %s, Config.Size: %d, "+ "Chart.Digest: %s, Chart.Size: %d", diff --git a/pkg/repo/v1/repotest/server_test.go b/pkg/repo/v1/repotest/server_test.go index 1364fc9c9..7008e2305 100644 --- a/pkg/repo/v1/repotest/server_test.go +++ b/pkg/repo/v1/repotest/server_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" @@ -40,18 +41,11 @@ func TestServer(t *testing.T) { defer srv.Stop() c, err := srv.CopyCharts("testdata/*.tgz") - if err != nil { - // Some versions of Go don't correctly fire defer on Fatal. - t.Fatal(err) - } + require.NoError(t, err) - if len(c) != 1 { - t.Errorf("Unexpected chart count: %d", len(c)) - } + assert.Len(t, c, 1) - if filepath.Base(c[0]) != "examplechart-0.1.0.tgz" { - t.Errorf("Unexpected chart: %s", c[0]) - } + assert.Equal(t, "examplechart-0.1.0.tgz", filepath.Base(c[0])) req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/examplechart-0.1.0.tgz", http.NoBody) require.NoError(t, err) @@ -60,9 +54,7 @@ func TestServer(t *testing.T) { require.NoError(t, err) res.Body.Close() - if res.ContentLength < 500 { - t.Errorf("Expected at least 500 bytes of data, got %d", res.ContentLength) - } + assert.GreaterOrEqual(t, res.ContentLength, int64(500)) req, err = http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/index.yaml", http.NoBody) require.NoError(t, err) @@ -73,27 +65,19 @@ func TestServer(t *testing.T) { require.NoError(t, err) m := repo.NewIndexFile() - if err := yaml.Unmarshal(data, m); err != nil { - t.Fatal(err) - } + require.NoError(t, yaml.Unmarshal(data, m)) - if l := len(m.Entries); l != 1 { - t.Fatalf("Expected 1 entry, got %d", l) - } + require.Len(t, m.Entries, 1) expect := "examplechart" - if !m.Has(expect, "0.1.0") { - t.Errorf("missing %q", expect) - } + assert.True(t, m.Has(expect, "0.1.0"), "missing %q", expect) req, err = http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/index.yaml-nosuchthing", http.NoBody) require.NoError(t, err) res, err = client.Do(req) require.NoError(t, err) res.Body.Close() - if res.StatusCode != http.StatusNotFound { - t.Fatalf("Expected 404, got %d", res.StatusCode) - } + require.Equal(t, http.StatusNotFound, res.StatusCode) } func TestNewTempServer(t *testing.T) { @@ -125,9 +109,7 @@ func TestNewTempServer(t *testing.T) { ) defer srv.Stop() - if srv.srv.URL == "" { - t.Fatal("unstarted server") - } + require.NotEmpty(t, srv.srv.URL, "unstarted server") client := srv.Client() @@ -137,9 +119,7 @@ func TestNewTempServer(t *testing.T) { res, err := client.Do(req) require.NoError(t, err) res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Errorf("Expected 200, got %d", res.StatusCode) - } + assert.Equal(t, http.StatusOK, res.StatusCode) } { req, err := http.NewRequestWithContext(t.Context(), http.MethodHead, srv.URL()+"/examplechart-0.1.0.tgz", http.NoBody) @@ -147,18 +127,14 @@ func TestNewTempServer(t *testing.T) { res, err := client.Do(req) require.NoError(t, err) res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Errorf("Expected 200, got %d", res.StatusCode) - } + assert.Equal(t, http.StatusOK, res.StatusCode) } req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/examplechart-0.1.0.tgz", http.NoBody) require.NoError(t, err) res, err := client.Do(req) require.NoError(t, err) res.Body.Close() - if res.ContentLength < 500 { - t.Errorf("Expected at least 500 bytes of data, got %d", res.ContentLength) - } + assert.GreaterOrEqual(t, res.ContentLength, int64(500)) req, err = http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/index.yaml", http.NoBody) require.NoError(t, err) res, err = client.Do(req) @@ -167,24 +143,16 @@ func TestNewTempServer(t *testing.T) { res.Body.Close() require.NoError(t, err) m := repo.NewIndexFile() - if err := yaml.Unmarshal(data, m); err != nil { - t.Fatal(err) - } - if l := len(m.Entries); l != 1 { - t.Fatalf("Expected 1 entry, got %d", l) - } + require.NoError(t, yaml.Unmarshal(data, m)) + require.Len(t, m.Entries, 1) expect := "examplechart" - if !m.Has(expect, "0.1.0") { - t.Errorf("missing %q", expect) - } + assert.True(t, m.Has(expect, "0.1.0"), "missing %q", expect) req, err = http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL()+"/index.yaml-nosuchthing", http.NoBody) require.NoError(t, err) res, err = client.Do(req) require.NoError(t, err) res.Body.Close() - if res.StatusCode != http.StatusNotFound { - t.Fatalf("Expected 404, got %d", res.StatusCode) - } + require.Equal(t, http.StatusNotFound, res.StatusCode) }) } } @@ -199,7 +167,5 @@ func TestNewTempServer_TLS(t *testing.T) { ) defer srv.Stop() - if !strings.HasPrefix(srv.URL(), "https://") { - t.Fatal("non-TLS server") - } + require.True(t, strings.HasPrefix(srv.URL(), "https://"), "non-TLS server") } From 3ce02c585a18b38c5dba1177a54c3a0322c53d3c Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 16:26:28 +1200 Subject: [PATCH 06/10] refactor(internal): convert tests to testify assert/require Convert internal/fileutil, internal/tlsutil, and internal/resolver test files from native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) to github.com/stretchr/testify equivalents (assert.X, require.X). Signed-off-by: George Jenkins --- internal/fileutil/fileutil_test.go | 70 +++++++++--------------------- internal/resolver/resolver_test.go | 51 +++++++++------------- internal/tlsutil/tls_test.go | 55 +++++++---------------- 3 files changed, 56 insertions(+), 120 deletions(-) diff --git a/internal/fileutil/fileutil_test.go b/internal/fileutil/fileutil_test.go index 71fcae177..07cbe7695 100644 --- a/internal/fileutil/fileutil_test.go +++ b/internal/fileutil/fileutil_test.go @@ -22,6 +22,9 @@ import ( "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestAtomicWriteFile tests the happy path of AtomicWriteFile function. @@ -35,28 +38,17 @@ func TestAtomicWriteFile(t *testing.T) { mode := os.FileMode(0644) err := AtomicWriteFile(testpath, reader, mode) - if err != nil { - t.Errorf("AtomicWriteFile error: %s", err) - } + assert.NoError(t, err) got, err := os.ReadFile(testpath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if stringContent != string(got) { - t.Fatalf("expected: %s, got: %s", stringContent, string(got)) - } + require.Equal(t, stringContent, string(got)) gotinfo, err := os.Stat(testpath) - if err != nil { - t.Fatal(err) - } - - if mode != gotinfo.Mode() { - t.Fatalf("expected %s: to be the same mode as %s", - mode, gotinfo.Mode()) - } + require.NoError(t, err) + + require.Equal(t, mode, gotinfo.Mode()) } // TestAtomicWriteFile_CreateTempError tests the error path when os.CreateTemp fails @@ -67,9 +59,7 @@ func TestAtomicWriteFile_CreateTempError(t *testing.T) { mode := os.FileMode(0644) err := AtomicWriteFile(invalidPath, reader, mode) - if err == nil { - t.Error("Expected error when CreateTemp fails, but got nil") - } + assert.Error(t, err, "Expected error when CreateTemp fails") } // TestAtomicWriteFile_EmptyContent tests with empty content @@ -81,18 +71,12 @@ func TestAtomicWriteFile_EmptyContent(t *testing.T) { mode := os.FileMode(0644) err := AtomicWriteFile(testpath, reader, mode) - if err != nil { - t.Errorf("AtomicWriteFile error with empty content: %s", err) - } + assert.NoError(t, err, "AtomicWriteFile error with empty content") got, err := os.ReadFile(testpath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if len(got) != 0 { - t.Fatalf("expected empty content, got: %s", string(got)) - } + require.Empty(t, got) } // TestAtomicWriteFile_LargeContent tests with large content @@ -106,18 +90,12 @@ func TestAtomicWriteFile_LargeContent(t *testing.T) { mode := os.FileMode(0644) err := AtomicWriteFile(testpath, reader, mode) - if err != nil { - t.Errorf("AtomicWriteFile error with large content: %s", err) - } + assert.NoError(t, err, "AtomicWriteFile error with large content") got, err := os.ReadFile(testpath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if largeContent != string(got) { - t.Fatalf("expected large content to match, got different length: %d vs %d", len(largeContent), len(got)) - } + require.Equal(t, largeContent, string(got)) } // TestPlatformAtomicWriteFile_OverwritesExisting verifies that the platform @@ -127,21 +105,13 @@ func TestPlatformAtomicWriteFile_OverwritesExisting(t *testing.T) { path := filepath.Join(dir, "overwrite_test") first := bytes.NewReader([]byte("first")) - if err := PlatformAtomicWriteFile(path, first, 0644); err != nil { - t.Fatalf("first write failed: %v", err) - } + require.NoError(t, PlatformAtomicWriteFile(path, first, 0644), "first write failed") second := bytes.NewReader([]byte("second")) - if err := PlatformAtomicWriteFile(path, second, 0644); err != nil { - t.Fatalf("second write failed: %v", err) - } + require.NoError(t, PlatformAtomicWriteFile(path, second, 0644), "second write failed") contents, err := os.ReadFile(path) - if err != nil { - t.Fatalf("failed reading result: %v", err) - } + require.NoError(t, err, "failed reading result") - if string(contents) != "second" { - t.Fatalf("expected file to be overwritten, got %q", string(contents)) - } + require.Equal(t, "second", string(contents)) } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1e33837a9..547419e17 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -19,6 +19,9 @@ import ( "runtime" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/registry" ) @@ -149,34 +152,24 @@ func TestResolve(t *testing.T) { if tt.err { return } - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Fatalf("Expected error in test %q", tt.name) + require.Failf(t, "Expected error in test %q", tt.name) } - if h, err := HashReq(tt.req, tt.expect.Dependencies); err != nil { - t.Fatal(err) - } else if h != l.Digest { - t.Errorf("%q: hashes don't match.", tt.name) - } + h, err := HashReq(tt.req, tt.expect.Dependencies) + require.NoError(t, err) + assert.Equal(t, l.Digest, h, "%q: hashes don't match.", tt.name) // Check fields. - if len(l.Dependencies) != len(tt.req) { - t.Errorf("%s: wrong number of dependencies in lock", tt.name) - } + assert.Len(t, l.Dependencies, len(tt.req), "%s: wrong number of dependencies in lock", tt.name) d0 := l.Dependencies[0] e0 := tt.expect.Dependencies[0] - if d0.Name != e0.Name { - t.Errorf("%s: expected name %s, got %s", tt.name, e0.Name, d0.Name) - } - if d0.Repository != e0.Repository { - t.Errorf("%s: expected repo %s, got %s", tt.name, e0.Repository, d0.Repository) - } - if d0.Version != e0.Version { - t.Errorf("%s: expected version %s, got %s", tt.name, e0.Version, d0.Version) - } + assert.Equal(t, e0.Name, d0.Name, tt.name) + assert.Equal(t, e0.Repository, d0.Repository, tt.name) + assert.Equal(t, e0.Version, d0.Version, tt.name) }) } } @@ -231,13 +224,11 @@ func TestHashReq(t *testing.T) { {Name: "alpine", Version: tt.lockVersion, Repository: "http://localhost:8879/charts"}, } h, err := HashReq(req, lock) - if err != nil { - t.Fatal(err) - } - if !tt.wantError && expect != h { - t.Errorf("Expected %q, got %q", expect, h) - } else if tt.wantError && expect == h { - t.Errorf("Expected not %q, but same", expect) + require.NoError(t, err) + if !tt.wantError { + assert.Equal(t, expect, h) + } else { + assert.NotEqual(t, expect, h, "Expected not %q, but same", expect) } }) } @@ -293,18 +284,16 @@ func TestGetLocalPath(t *testing.T) { if tt.err { return } - t.Fatal(err) + require.NoError(t, err) } if tt.err { - t.Fatalf("Expected error in test %q", tt.name) + require.Failf(t, "Expected error in test %q", tt.name) } expect := tt.expect if runtime.GOOS == "windows" { expect = tt.winExpect } - if p != expect { - t.Errorf("%q: expected %q, got %q", tt.name, expect, p) - } + assert.Equal(t, expect, p, tt.name) }) } } diff --git a/internal/tlsutil/tls_test.go b/internal/tlsutil/tls_test.go index e6859f99f..324b00969 100644 --- a/internal/tlsutil/tls_test.go +++ b/internal/tlsutil/tls_test.go @@ -19,6 +19,9 @@ package tlsutil import ( "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const tlsTestDir = "../../testdata" @@ -32,9 +35,7 @@ const ( func testfile(t *testing.T, file string) (path string) { t.Helper() path, err := filepath.Abs(filepath.Join(tlsTestDir, file)) - if err != nil { - t.Fatalf("error getting absolute path to test file %q: %v", file, err) - } + require.NoError(t, err, "error getting absolute path to test file %q", file) return path } @@ -50,38 +51,22 @@ func TestNewTLSConfig(t *testing.T) { WithCertKeyPairFiles(certFile, keyFile), WithCAFile(caCertFile), ) - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if got := len(cfg.Certificates); got != 1 { - t.Fatalf("expecting 1 client certificates, got %d", got) - } - if cfg.InsecureSkipVerify { - t.Fatal("insecure skip verify mismatch, expecting false") - } - if cfg.RootCAs == nil { - t.Fatal("mismatch tls RootCAs, expecting non-nil") - } + require.Len(t, cfg.Certificates, 1) + require.False(t, cfg.InsecureSkipVerify, "insecure skip verify mismatch, expecting false") + require.NotNil(t, cfg.RootCAs, "mismatch tls RootCAs, expecting non-nil") } { cfg, err := NewTLSConfig( WithInsecureSkipVerify(insecureSkipTLSVerify), WithCAFile(caCertFile), ) - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if got := len(cfg.Certificates); got != 0 { - t.Fatalf("expecting 0 client certificates, got %d", got) - } - if cfg.InsecureSkipVerify { - t.Fatal("insecure skip verify mismatch, expecting false") - } - if cfg.RootCAs == nil { - t.Fatal("mismatch tls RootCAs, expecting non-nil") - } + require.Empty(t, cfg.Certificates) + require.False(t, cfg.InsecureSkipVerify, "insecure skip verify mismatch, expecting false") + require.NotNil(t, cfg.RootCAs, "mismatch tls RootCAs, expecting non-nil") } { @@ -89,18 +74,10 @@ func TestNewTLSConfig(t *testing.T) { WithInsecureSkipVerify(insecureSkipTLSVerify), WithCertKeyPairFiles(certFile, keyFile), ) - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if got := len(cfg.Certificates); got != 1 { - t.Fatalf("expecting 1 client certificates, got %d", got) - } - if cfg.InsecureSkipVerify { - t.Fatal("insecure skip verify mismatch, expecting false") - } - if cfg.RootCAs != nil { - t.Fatal("mismatch tls RootCAs, expecting nil") - } + require.Len(t, cfg.Certificates, 1) + require.False(t, cfg.InsecureSkipVerify, "insecure skip verify mismatch, expecting false") + require.Nil(t, cfg.RootCAs, "mismatch tls RootCAs, expecting nil") } } From 8b0a167484a7f6de4a2cb12bf81641873a275d7b Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 17 Jun 2026 21:36:47 +1200 Subject: [PATCH 07/10] refactor(pkg/registry): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- pkg/registry/chart_test.go | 39 +++++++------------ pkg/registry/client_test.go | 31 +++++---------- pkg/registry/plugin_test.go | 17 +++----- pkg/registry/reference_test.go | 66 ++++++++++--------------------- pkg/registry/tag_test.go | 71 +++++++++------------------------- pkg/registry/transport_test.go | 35 ++++++----------- 6 files changed, 79 insertions(+), 180 deletions(-) diff --git a/pkg/registry/chart_test.go b/pkg/registry/chart_test.go index 0b247601b..23a32506d 100644 --- a/pkg/registry/chart_test.go +++ b/pkg/registry/chart_test.go @@ -17,11 +17,12 @@ limitations under the License. package registry // import "helm.sh/helm/v4/pkg/registry" import ( - "reflect" "testing" "time" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" chart "helm.sh/helm/v4/pkg/chart/v2" ) @@ -147,10 +148,7 @@ func TestGenerateOCIChartAnnotations(t *testing.T) { for _, tt := range tests { result := generateChartOCIAnnotations(tt.chart, nowString) - - if !reflect.DeepEqual(tt.expect, result) { - t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, result) - } + assert.Equal(t, tt.expect, result, tt.name) } } @@ -218,10 +216,7 @@ func TestGenerateOCIAnnotations(t *testing.T) { for _, tt := range tests { result := generateOCIAnnotations(tt.chart, nowString) - - if !reflect.DeepEqual(tt.expect, result) { - t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, result) - } + assert.Equal(t, tt.expect, result, tt.name) } } @@ -237,29 +232,23 @@ func TestGenerateOCICreatedAnnotations(t *testing.T) { result := generateOCIAnnotations(testChart, nowTimeString) // Check that created annotation exists - if _, ok := result[ocispec.AnnotationCreated]; !ok { - t.Errorf("%s annotation not created", ocispec.AnnotationCreated) - } + _, ok := result[ocispec.AnnotationCreated] + assert.True(t, ok, "%s annotation not created", ocispec.AnnotationCreated) // Verify value of created artifact in RFC3339 format - if _, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated]); err != nil { - t.Errorf("%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated]) - } + _, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated]) + assert.NoError(t, err, "%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated]) // Verify default creation time set result = generateOCIAnnotations(testChart, "") // Check that created annotation exists - if _, ok := result[ocispec.AnnotationCreated]; !ok { - t.Errorf("%s annotation not created", ocispec.AnnotationCreated) - } + _, ok = result[ocispec.AnnotationCreated] + require.True(t, ok, "%s annotation not created", ocispec.AnnotationCreated) - if createdTimeAnnotation, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated]); err != nil { - t.Errorf("%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated]) + createdTimeAnnotation, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated]) + require.NoError(t, err, "%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated]) - // Verify creation annotation after time test began - if !nowTime.Before(createdTimeAnnotation) { - t.Errorf("%s annotation with value '%s' not configured properly. Annotation value is not after %s", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated], nowTimeString) - } - } + // Verify creation annotation after (or equals) time test began + assert.False(t, nowTime.Before(createdTimeAnnotation), "%s annotation with value '%s' not configured properly. Annotation value is not after %s", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated], nowTimeString) } diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 702dfff69..97b7c7c4c 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -25,6 +25,7 @@ import ( "testing" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "oras.land/oras-go/v2/content/memory" ) @@ -77,22 +78,15 @@ func TestLogin_ResetsForceAttemptOAuth2_OnSuccess(t *testing.T) { ClientOptWriter(io.Discard), ClientOptCredentialsFile(credFile), ) - if err != nil { - t.Fatalf("NewClient error: %v", err) - } + require.NoError(t, err, "NewClient error") - if c.authorizer == nil || c.authorizer.ForceAttemptOAuth2 { - t.Fatal("expected ForceAttemptOAuth2 default to be false") - } + require.NotNil(t, c.authorizer) + require.False(t, c.authorizer.ForceAttemptOAuth2, "expected ForceAttemptOAuth2 default to be false") // Call Login with plain HTTP against our test server - if err := c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")); err != nil { - t.Fatalf("Login error: %v", err) - } + require.NoError(t, c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")), "Login error") - if c.authorizer.ForceAttemptOAuth2 { - t.Error("ForceAttemptOAuth2 should be false after successful Login") - } + assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after successful Login") } // Verifies that Login restores ForceAttemptOAuth2 to false even when ping fails. @@ -109,16 +103,12 @@ func TestLogin_ResetsForceAttemptOAuth2_OnFailure(t *testing.T) { ClientOptWriter(io.Discard), ClientOptCredentialsFile(credFile), ) - if err != nil { - t.Fatalf("NewClient error: %v", err) - } + require.NoError(t, err, "NewClient error") // Invoke Login, expect an error but ForceAttemptOAuth2 must end false _ = c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")) - if c.authorizer.ForceAttemptOAuth2 { - t.Error("ForceAttemptOAuth2 should be false after failed Login") - } + assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after failed Login") } // TestWarnIfHostHasPath verifies that warnIfHostHasPath correctly detects path components. @@ -159,10 +149,7 @@ func TestWarnIfHostHasPath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := warnIfHostHasPath(tt.host) - if got != tt.wantWarn { - t.Errorf("warnIfHostHasPath(%q) = %v, want %v", tt.host, got, tt.wantWarn) - } + assert.Equal(t, tt.wantWarn, warnIfHostHasPath(tt.host)) }) } } diff --git a/pkg/registry/plugin_test.go b/pkg/registry/plugin_test.go index 61995ece9..bb557c5b8 100644 --- a/pkg/registry/plugin_test.go +++ b/pkg/registry/plugin_test.go @@ -18,6 +18,9 @@ package registry import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetPluginName(t *testing.T) { @@ -74,20 +77,12 @@ func TestGetPluginName(t *testing.T) { pluginName, err := GetPluginName(tt.source) if tt.expectErr { - if err == nil { - t.Error("expected error but got none") - } + assert.Error(t, err, "expected error but got none") return } - if err != nil { - t.Errorf("unexpected error: %v", err) - return - } - - if pluginName != tt.expected { - t.Errorf("expected plugin name %q, got %q", tt.expected, pluginName) - } + require.NoError(t, err) + assert.Equal(t, tt.expected, pluginName) }) } } diff --git a/pkg/registry/reference_test.go b/pkg/registry/reference_test.go index b6872cc37..f1a87fefc 100644 --- a/pkg/registry/reference_test.go +++ b/pkg/registry/reference_test.go @@ -16,31 +16,21 @@ limitations under the License. package registry -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" +) func verify(t *testing.T, actual reference, registry, repository, tag, digest string) { t.Helper() - if registry != actual.orasReference.Registry { - t.Errorf("Oras reference registry expected %v actual %v", registry, actual.Registry) - } - if repository != actual.orasReference.Repository { - t.Errorf("Oras reference repository expected %v actual %v", repository, actual.Repository) - } - if tag != actual.orasReference.Reference { - t.Errorf("Oras reference reference expected %v actual %v", tag, actual.Tag) - } - if registry != actual.Registry { - t.Errorf("Registry expected %v actual %v", registry, actual.Registry) - } - if repository != actual.Repository { - t.Errorf("Repository expected %v actual %v", repository, actual.Repository) - } - if tag != actual.Tag { - t.Errorf("Tag expected %v actual %v", tag, actual.Tag) - } - if digest != actual.Digest { - t.Errorf("Digest expected %v actual %v", digest, actual.Digest) - } + assert.Equal(t, registry, actual.orasReference.Registry, "Oras reference registry") + assert.Equal(t, repository, actual.orasReference.Repository, "Oras reference repository") + assert.Equal(t, tag, actual.orasReference.Reference, "Oras reference reference") + assert.Equal(t, registry, actual.Registry, "Registry") + assert.Equal(t, repository, actual.Repository, "Repository") + assert.Equal(t, tag, actual.Tag, "Tag") + assert.Equal(t, digest, actual.Digest, "Digest") expectedString := registry if repository != "" { expectedString = expectedString + "/" + repository @@ -50,51 +40,35 @@ func verify(t *testing.T, actual reference, registry, repository, tag, digest st } else { expectedString = expectedString + "@" + digest } - if actual.String() != expectedString { - t.Errorf("String expected %s actual %s", expectedString, actual.String()) - } + assert.Equal(t, expectedString, actual.String(), "String") } func TestNewReference(t *testing.T) { actual, err := newReference("registry.example.com/repository:1.0@sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "registry.example.com", "repository", "1.0", "sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") actual, err = newReference("oci://registry.example.com/repository:1.0@sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "registry.example.com", "repository", "1.0", "sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") actual, err = newReference("a/b:1@c") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "a", "b", "1", "c") actual, err = newReference("a/b:@") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "a", "b", "", "") actual, err = newReference("registry.example.com/repository:1.0+001") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "registry.example.com", "repository", "1.0_001", "") actual, err = newReference("thing:1.0") - if err == nil { - t.Errorf("Expect error error %v", err) - } + assert.Error(t, err) verify(t, actual, "", "", "", "") actual, err = newReference("registry.example.com/the/repository@sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") - if err != nil { - t.Errorf("Unexpected error %v", err) - } + assert.NoError(t, err) verify(t, actual, "registry.example.com", "the/repository", "", "sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888") } diff --git a/pkg/registry/tag_test.go b/pkg/registry/tag_test.go index e3ec47703..830eac7e1 100644 --- a/pkg/registry/tag_test.go +++ b/pkg/registry/tag_test.go @@ -17,32 +17,25 @@ limitations under the License. package registry import ( - "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestGetTagMatchingVersionOrConstraint_ExactMatch(t *testing.T) { tags := []string{"1.0.0", "1.2.3", "2.0.0"} got, err := GetTagMatchingVersionOrConstraint(tags, "1.2.3") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.2.3" { - t.Fatalf("expected exact match '1.2.3', got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.2.3", got, "expected exact match") } func TestGetTagMatchingVersionOrConstraint_EmptyVersionWildcard(t *testing.T) { // Includes a non-semver tag which should be skipped tags := []string{"latest", "0.9.0", "1.0.0"} got, err := GetTagMatchingVersionOrConstraint(tags, "") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(t, err) // Should pick the first valid semver tag in order, which is 0.9.0 - if got != "0.9.0" { - t.Fatalf("expected '0.9.0', got %q", got) - } + require.Equal(t, "0.9.0", got) } func TestGetTagMatchingVersionOrConstraint_ConstraintRange(t *testing.T) { @@ -50,73 +43,47 @@ func TestGetTagMatchingVersionOrConstraint_ConstraintRange(t *testing.T) { // Caret range got, err := GetTagMatchingVersionOrConstraint(tags, "^1.0.0") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.0.0" { // first match in order - t.Fatalf("expected '1.0.0', got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.0.0", got, "first match in order") // Compound range got, err = GetTagMatchingVersionOrConstraint(tags, ">=1.0.0 <2.0.0") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.0.0" { - t.Fatalf("expected '1.0.0', got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.0.0", got) } func TestGetTagMatchingVersionOrConstraint_InvalidConstraint(t *testing.T) { tags := []string{"1.0.0"} _, err := GetTagMatchingVersionOrConstraint(tags, ">a1") - if err == nil { - t.Fatal("expected error for invalid constraint") - } + require.Error(t, err, "expected error for invalid constraint") } func TestGetTagMatchingVersionOrConstraint_NoMatches(t *testing.T) { tags := []string{"0.1.0", "0.2.0"} _, err := GetTagMatchingVersionOrConstraint(tags, ">=1.0.0") - if err == nil { - t.Fatal("expected error when no tags match") - } - if !strings.Contains(err.Error(), ">=1.0.0") { - t.Fatalf("expected error to contain version string, got: %v", err) - } + require.Error(t, err, "expected error when no tags match") + require.Contains(t, err.Error(), ">=1.0.0", "expected error to contain version string") } func TestGetTagMatchingVersionOrConstraint_SkipsNonSemverTags(t *testing.T) { tags := []string{"alpha", "1.0.0", "beta", "1.1.0"} got, err := GetTagMatchingVersionOrConstraint(tags, ">=1.0.0 <2.0.0") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.0.0" { - t.Fatalf("expected '1.0.0', got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.0.0", got) } func TestGetTagMatchingVersionOrConstraint_OrderMatters_FirstMatchReturned(t *testing.T) { // Both 1.2.0 and 1.3.0 satisfy >=1.2.0 <2.0.0, but the function returns the first in input order tags := []string{"1.3.0", "1.2.0"} got, err := GetTagMatchingVersionOrConstraint(tags, ">=1.2.0 <2.0.0") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.3.0" { - t.Fatalf("expected '1.3.0' (first satisfying tag), got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.3.0", got, "first satisfying tag") } func TestGetTagMatchingVersionOrConstraint_ExactMatchHasPrecedence(t *testing.T) { // Exact match should be returned even if another earlier tag would match the parsed constraint tags := []string{"1.3.0", "1.2.3"} got, err := GetTagMatchingVersionOrConstraint(tags, "1.2.3") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != "1.2.3" { - t.Fatalf("expected exact match '1.2.3', got %q", got) - } + require.NoError(t, err) + require.Equal(t, "1.2.3", got, "expected exact match") } diff --git a/pkg/registry/transport_test.go b/pkg/registry/transport_test.go index b4990c526..e72c97c0d 100644 --- a/pkg/registry/transport_test.go +++ b/pkg/registry/transport_test.go @@ -22,6 +22,9 @@ import ( "io" "net/http" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var errMockRead = errors.New("mock read error") @@ -127,9 +130,7 @@ func Test_isPrintableContentType(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := isPrintableContentType(tt.contentType); got != tt.want { - t.Errorf("isPrintableContentType() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, isPrintableContentType(tt.contentType)) }) } } @@ -292,21 +293,13 @@ func Test_logResponseBody(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := logResponseBody(tt.resp); got != tt.want { - t.Errorf("logResponseBody() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, logResponseBody(tt.resp)) // validate the response body if tt.resp.Body != nil { readBytes, err := io.ReadAll(tt.resp.Body) - if err != nil { - t.Errorf("failed to read body after logResponseBody(), err= %v", err) - } - if !bytes.Equal(readBytes, tt.wantData) { - t.Errorf("resp.Body after logResponseBody() = %v, want %v", readBytes, tt.wantData) - } - if closeErr := tt.resp.Body.Close(); closeErr != nil { - t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) - } + require.NoError(t, err, "failed to read body after logResponseBody()") + assert.True(t, bytes.Equal(tt.wantData, readBytes), "resp.Body after logResponseBody()") + assert.NoError(t, tt.resp.Body.Close(), "failed to close body after logResponseBody()") } }) } @@ -331,12 +324,8 @@ func Test_logResponseBody_error(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := logResponseBody(tt.resp); got != tt.want { - t.Errorf("logResponseBody() = %v, want %v", got, tt.want) - } - if closeErr := tt.resp.Body.Close(); closeErr != nil { - t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) - } + assert.Equal(t, tt.want, logResponseBody(tt.resp)) + assert.NoError(t, tt.resp.Body.Close(), "failed to close body after logResponseBody()") }) } } @@ -391,9 +380,7 @@ func Test_containsCredentials(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := containsCredentials(tt.body); got != tt.want { - t.Errorf("containsCredentials() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, containsCredentials(tt.body)) }) } } From 4cce0e9e2a870508864c1d497918e201dc0f339c Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 17 Jun 2026 21:32:52 +1200 Subject: [PATCH 08/10] refactor(pkg/release): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- pkg/release/v1/util/filter_test.go | 29 ++++++---------- pkg/release/v1/util/kind_sorter_test.go | 35 ++++++------------- pkg/release/v1/util/manifest_sorter_test.go | 38 ++++++--------------- pkg/release/v1/util/manifest_test.go | 6 ++-- pkg/release/v1/util/sorter_test.go | 6 ++-- 5 files changed, 38 insertions(+), 76 deletions(-) diff --git a/pkg/release/v1/util/filter_test.go b/pkg/release/v1/util/filter_test.go index 1004a4c57..d6bda8f84 100644 --- a/pkg/release/v1/util/filter_test.go +++ b/pkg/release/v1/util/filter_test.go @@ -19,23 +19,19 @@ package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( "testing" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) func TestFilterAny(t *testing.T) { ls := Any(StatusFilter(common.StatusUninstalled)).Filter(releases) - if len(ls) != 2 { - t.Fatalf("expected 2 results, got '%d'", len(ls)) - } + require.Len(t, ls, 2) r0, r1 := ls[0], ls[1] - switch { - case r0.Info.Status != common.StatusUninstalled: - t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) - case r1.Info.Status != common.StatusUninstalled: - t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) - } + require.Equal(t, common.StatusUninstalled, r0.Info.Status) + require.Equal(t, common.StatusUninstalled, r1.Info.Status) } func TestFilterAll(t *testing.T) { @@ -47,14 +43,9 @@ func TestFilterAll(t *testing.T) { }) ls := All(fn).Filter(releases) - if len(ls) != 1 { - t.Fatalf("expected 1 result, got '%d'", len(ls)) - } - - switch r0 := ls[0]; { - case r0.Version == 4: - t.Fatal("got release with status revision 4") - case r0.Info.Status == common.StatusUninstalled: - t.Fatal("got release with status UNINSTALLED") - } + require.Len(t, ls, 1) + + r0 := ls[0] + require.NotEqual(t, 4, r0.Version, "got release with status revision 4") + require.NotEqual(t, common.StatusUninstalled, r0.Info.Status, "got release with status UNINSTALLED") } diff --git a/pkg/release/v1/util/kind_sorter_test.go b/pkg/release/v1/util/kind_sorter_test.go index 919de24e5..793ef87fe 100644 --- a/pkg/release/v1/util/kind_sorter_test.go +++ b/pkg/release/v1/util/kind_sorter_test.go @@ -20,6 +20,9 @@ import ( "bytes" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + release "helm.sh/helm/v4/pkg/release/v1" ) @@ -193,21 +196,15 @@ func TestKindSorter(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - if got, want := len(test.expected), len(manifests); got != want { - t.Fatalf("Expected %d names in order, got %d", want, got) - } + require.Equal(t, len(manifests), len(test.expected), "Expected %d names in order", len(manifests)) defer buf.Reset() orig := manifests for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) - } + assert.Equal(t, test.expected, buf.String()) for i, manifest := range orig { - if manifest != manifests[i] { - t.Fatal("Expected input to sortManifestsByKind to stay the same") - } + require.Equal(t, manifest, manifests[i], "Expected input to sortManifestsByKind to stay the same") } }) } @@ -267,9 +264,7 @@ func TestKindSorterKeepOriginalOrder(t *testing.T) { for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) - } + assert.Equal(t, test.expected, buf.String()) }) } } @@ -289,9 +284,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { expectedOrder := []Manifest{namespace, unknown} for i, manifest := range manifests { - if expectedOrder[i].Name != manifest.Name { - t.Errorf("Expected %s, got %s", expectedOrder[i].Name, manifest.Name) - } + assert.Equal(t, expectedOrder[i].Name, manifest.Name) } } @@ -326,22 +319,16 @@ func TestKindSorterForHooks(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - if got, want := len(test.expected), len(hooks); got != want { - t.Fatalf("Expected %d names in order, got %d", want, got) - } + require.Equal(t, len(hooks), len(test.expected), "Expected %d names in order", len(hooks)) defer buf.Reset() orig := hooks for _, r := range sortHooksByKind(hooks, test.order) { buf.WriteString(r.Name) } for i, hook := range orig { - if hook != hooks[i] { - t.Fatal("Expected input to sortHooksByKind to stay the same") - } - } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) + require.Equal(t, hook, hooks[i], "Expected input to sortHooksByKind to stay the same") } + assert.Equal(t, test.expected, buf.String()) }) } } diff --git a/pkg/release/v1/util/manifest_sorter_test.go b/pkg/release/v1/util/manifest_sorter_test.go index baa320be0..2cdb55dc3 100644 --- a/pkg/release/v1/util/manifest_sorter_test.go +++ b/pkg/release/v1/util/manifest_sorter_test.go @@ -20,6 +20,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" release "helm.sh/helm/v4/pkg/release/v1" @@ -138,55 +140,39 @@ metadata: } hs, generic, err := SortManifests(manifests, nil, InstallOrder) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + require.NoError(t, err) // This test will fail if 'six' or 'seven' was added. - if len(generic) != 2 { - t.Errorf("Expected 2 generic manifests, got %d", len(generic)) - } + assert.Len(t, generic, 2) - if len(hs) != 4 { - t.Errorf("Expected 4 hooks, got %d", len(hs)) - } + assert.Len(t, hs, 4) for _, out := range hs { found := false for _, expect := range data { if out.Path == expect.path { found = true - if out.Path != expect.path { - t.Errorf("Expected path %s, got %s", expect.path, out.Path) - } + assert.Equal(t, expect.path, out.Path) nameFound := false for _, expectedName := range expect.name { if out.Name == expectedName { nameFound = true } } - if !nameFound { - t.Errorf("Got unexpected name %s", out.Name) - } + assert.True(t, nameFound, "Got unexpected name %s", out.Name) kindFound := false for _, expectedKind := range expect.kind { if out.Kind == expectedKind { kindFound = true } } - if !kindFound { - t.Errorf("Got unexpected kind %s", out.Kind) - } + assert.True(t, kindFound, "Got unexpected kind %s", out.Kind) expectedHooks := expect.hooks[out.Name] - if !reflect.DeepEqual(expectedHooks, out.Events) { - t.Errorf("expected events: %v but got: %v", expectedHooks, out.Events) - } + assert.True(t, reflect.DeepEqual(expectedHooks, out.Events), "expected events: %v but got: %v", expectedHooks, out.Events) } } - if !found { - t.Errorf("Result not found: %v", out) - } + assert.True(t, found, "Result not found: %v", out) } // Verify the sort order @@ -218,8 +204,6 @@ metadata: sorted = sortManifestsByKind(sorted, InstallOrder) for i, m := range generic { - if m.Content != sorted[i].Content { - t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content) - } + assert.Equal(t, sorted[i].Content, m.Content) } } diff --git a/pkg/release/v1/util/manifest_test.go b/pkg/release/v1/util/manifest_test.go index 516ac42d7..6bcf902d7 100644 --- a/pkg/release/v1/util/manifest_test.go +++ b/pkg/release/v1/util/manifest_test.go @@ -19,6 +19,8 @@ package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestSplitManifests(t *testing.T) { @@ -509,9 +511,7 @@ metadata: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := SplitManifests(tt.input) - if !reflect.DeepEqual(result, tt.expected) { - t.Errorf("SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) - } + assert.True(t, reflect.DeepEqual(result, tt.expected), "SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) }) } } diff --git a/pkg/release/v1/util/sorter_test.go b/pkg/release/v1/util/sorter_test.go index f47db7db8..0450b09e6 100644 --- a/pkg/release/v1/util/sorter_test.go +++ b/pkg/release/v1/util/sorter_test.go @@ -20,6 +20,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -45,9 +47,7 @@ func tsRelease(name string, vers int, dur time.Duration, status common.Status) * func check(t *testing.T, by string, fn func(int, int) bool) { t.Helper() for i := len(releases) - 1; i > 0; i-- { - if fn(i, i-1) { - t.Errorf("release at positions '(%d,%d)' not sorted by %s", i-1, i, by) - } + assert.False(t, fn(i, i-1), "release at positions '(%d,%d)' not sorted by %s", i-1, i, by) } } From 7ab295fcfcf3977599939d76ffbf7a266401bfbc Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 17 Jun 2026 21:48:26 +1200 Subject: [PATCH 09/10] refactor(internal/release): convert tests to testify assert/require Replace native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) with github.com/stretchr/testify equivalents (assert.X, require.X) for improved test readability and error messages. Signed-off-by: George Jenkins --- internal/release/v2/util/filter_test.go | 29 +++++--------- internal/release/v2/util/kind_sorter_test.go | 35 ++++++----------- .../release/v2/util/manifest_sorter_test.go | 39 ++++++------------- internal/release/v2/util/manifest_test.go | 7 ++-- internal/release/v2/util/sorter_test.go | 6 +-- pkg/release/v1/util/kind_sorter_test.go | 4 +- pkg/release/v1/util/manifest_sorter_test.go | 5 +-- pkg/release/v1/util/manifest_test.go | 3 +- 8 files changed, 43 insertions(+), 85 deletions(-) diff --git a/internal/release/v2/util/filter_test.go b/internal/release/v2/util/filter_test.go index 35236498a..8c7363b87 100644 --- a/internal/release/v2/util/filter_test.go +++ b/internal/release/v2/util/filter_test.go @@ -19,23 +19,19 @@ package util // import "helm.sh/helm/v4/internal/release/v2/util" import ( "testing" + "github.com/stretchr/testify/require" + rspb "helm.sh/helm/v4/internal/release/v2" "helm.sh/helm/v4/pkg/release/common" ) func TestFilterAny(t *testing.T) { ls := Any(StatusFilter(common.StatusUninstalled)).Filter(releases) - if len(ls) != 2 { - t.Fatalf("expected 2 results, got '%d'", len(ls)) - } + require.Len(t, ls, 2) r0, r1 := ls[0], ls[1] - switch { - case r0.Info.Status != common.StatusUninstalled: - t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) - case r1.Info.Status != common.StatusUninstalled: - t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) - } + require.Equal(t, common.StatusUninstalled, r0.Info.Status) + require.Equal(t, common.StatusUninstalled, r1.Info.Status) } func TestFilterAll(t *testing.T) { @@ -47,14 +43,9 @@ func TestFilterAll(t *testing.T) { }) ls := All(fn).Filter(releases) - if len(ls) != 1 { - t.Fatalf("expected 1 result, got '%d'", len(ls)) - } - - switch r0 := ls[0]; { - case r0.Version == 4: - t.Fatal("got release with status revision 4") - case r0.Info.Status == common.StatusUninstalled: - t.Fatal("got release with status UNINSTALLED") - } + require.Len(t, ls, 1) + + r0 := ls[0] + require.NotEqual(t, 4, r0.Version, "got release with status revision 4") + require.NotEqual(t, common.StatusUninstalled, r0.Info.Status, "got release with status UNINSTALLED") } diff --git a/internal/release/v2/util/kind_sorter_test.go b/internal/release/v2/util/kind_sorter_test.go index 06418a5fc..dba19e287 100644 --- a/internal/release/v2/util/kind_sorter_test.go +++ b/internal/release/v2/util/kind_sorter_test.go @@ -20,6 +20,9 @@ import ( "bytes" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + release "helm.sh/helm/v4/internal/release/v2" ) @@ -193,21 +196,15 @@ func TestKindSorter(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - if got, want := len(test.expected), len(manifests); got != want { - t.Fatalf("Expected %d names in order, got %d", want, got) - } + require.Len(t, manifests, len(test.expected), "Expected %d names in order", len(manifests)) defer buf.Reset() orig := manifests for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) - } + assert.Equal(t, test.expected, buf.String()) for i, manifest := range orig { - if manifest != manifests[i] { - t.Fatal("Expected input to sortManifestsByKind to stay the same") - } + require.Equal(t, manifest, manifests[i], "Expected input to sortManifestsByKind to stay the same") } }) } @@ -267,9 +264,7 @@ func TestKindSorterKeepOriginalOrder(t *testing.T) { for _, r := range sortManifestsByKind(manifests, test.order) { buf.WriteString(r.Name) } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) - } + assert.Equal(t, test.expected, buf.String()) }) } } @@ -289,9 +284,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) { expectedOrder := []Manifest{namespace, unknown} for i, manifest := range manifests { - if expectedOrder[i].Name != manifest.Name { - t.Errorf("Expected %s, got %s", expectedOrder[i].Name, manifest.Name) - } + assert.Equal(t, expectedOrder[i].Name, manifest.Name) } } @@ -326,22 +319,16 @@ func TestKindSorterForHooks(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - if got, want := len(test.expected), len(hooks); got != want { - t.Fatalf("Expected %d names in order, got %d", want, got) - } + require.Len(t, hooks, len(test.expected), "Expected %d names in order", len(hooks)) defer buf.Reset() orig := hooks for _, r := range sortHooksByKind(hooks, test.order) { buf.WriteString(r.Name) } for i, hook := range orig { - if hook != hooks[i] { - t.Fatal("Expected input to sortHooksByKind to stay the same") - } - } - if got := buf.String(); got != test.expected { - t.Errorf("Expected %q, got %q", test.expected, got) + require.Equal(t, hook, hooks[i], "Expected input to sortHooksByKind to stay the same") } + assert.Equal(t, test.expected, buf.String()) }) } } diff --git a/internal/release/v2/util/manifest_sorter_test.go b/internal/release/v2/util/manifest_sorter_test.go index c8851d678..1c932d70c 100644 --- a/internal/release/v2/util/manifest_sorter_test.go +++ b/internal/release/v2/util/manifest_sorter_test.go @@ -17,9 +17,10 @@ limitations under the License. package util // import "helm.sh/helm/v4/internal/release/v2/util" import ( - "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" release "helm.sh/helm/v4/internal/release/v2" @@ -138,55 +139,39 @@ metadata: } hs, generic, err := SortManifests(manifests, nil, InstallOrder) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + require.NoError(t, err) // This test will fail if 'six' or 'seven' was added. - if len(generic) != 2 { - t.Errorf("Expected 2 generic manifests, got %d", len(generic)) - } + assert.Len(t, generic, 2) - if len(hs) != 4 { - t.Errorf("Expected 4 hooks, got %d", len(hs)) - } + assert.Len(t, hs, 4) for _, out := range hs { found := false for _, expect := range data { if out.Path == expect.path { found = true - if out.Path != expect.path { - t.Errorf("Expected path %s, got %s", expect.path, out.Path) - } + assert.Equal(t, expect.path, out.Path) nameFound := false for _, expectedName := range expect.name { if out.Name == expectedName { nameFound = true } } - if !nameFound { - t.Errorf("Got unexpected name %s", out.Name) - } + assert.True(t, nameFound, "Got unexpected name %s", out.Name) kindFound := false for _, expectedKind := range expect.kind { if out.Kind == expectedKind { kindFound = true } } - if !kindFound { - t.Errorf("Got unexpected kind %s", out.Kind) - } + assert.True(t, kindFound, "Got unexpected kind %s", out.Kind) expectedHooks := expect.hooks[out.Name] - if !reflect.DeepEqual(expectedHooks, out.Events) { - t.Errorf("expected events: %v but got: %v", expectedHooks, out.Events) - } + assert.Equal(t, expectedHooks, out.Events, "expected events: %v but got: %v", expectedHooks, out.Events) } } - if !found { - t.Errorf("Result not found: %v", out) - } + assert.True(t, found, "Result not found: %v", out) } // Verify the sort order @@ -218,8 +203,6 @@ metadata: sorted = sortManifestsByKind(sorted, InstallOrder) for i, m := range generic { - if m.Content != sorted[i].Content { - t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content) - } + assert.Equal(t, m.Content, sorted[i].Content) } } diff --git a/internal/release/v2/util/manifest_test.go b/internal/release/v2/util/manifest_test.go index 72b095390..463ab5a4d 100644 --- a/internal/release/v2/util/manifest_test.go +++ b/internal/release/v2/util/manifest_test.go @@ -17,8 +17,9 @@ limitations under the License. package util // import "helm.sh/helm/v4/internal/release/v2/util" import ( - "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestSplitManifests(t *testing.T) { @@ -509,9 +510,7 @@ metadata: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := SplitManifests(tt.input) - if !reflect.DeepEqual(result, tt.expected) { - t.Errorf("SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) - } + assert.Equal(t, tt.expected, result, "SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) }) } } diff --git a/internal/release/v2/util/sorter_test.go b/internal/release/v2/util/sorter_test.go index 6cb876f69..82a53ae66 100644 --- a/internal/release/v2/util/sorter_test.go +++ b/internal/release/v2/util/sorter_test.go @@ -20,6 +20,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + rspb "helm.sh/helm/v4/internal/release/v2" "helm.sh/helm/v4/pkg/release/common" ) @@ -45,9 +47,7 @@ func tsRelease(name string, vers int, dur time.Duration, status common.Status) * func check(t *testing.T, by string, fn func(int, int) bool) { t.Helper() for i := len(releases) - 1; i > 0; i-- { - if fn(i, i-1) { - t.Errorf("release at positions '(%d,%d)' not sorted by %s", i-1, i, by) - } + assert.False(t, fn(i, i-1), "release at positions '(%d,%d)' not sorted by %s", i-1, i, by) } } diff --git a/pkg/release/v1/util/kind_sorter_test.go b/pkg/release/v1/util/kind_sorter_test.go index 793ef87fe..de8a79b25 100644 --- a/pkg/release/v1/util/kind_sorter_test.go +++ b/pkg/release/v1/util/kind_sorter_test.go @@ -196,7 +196,7 @@ func TestKindSorter(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - require.Equal(t, len(manifests), len(test.expected), "Expected %d names in order", len(manifests)) + require.Len(t, manifests, len(test.expected), "Expected %d names in order", len(manifests)) defer buf.Reset() orig := manifests for _, r := range sortManifestsByKind(manifests, test.order) { @@ -319,7 +319,7 @@ func TestKindSorterForHooks(t *testing.T) { } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { - require.Equal(t, len(hooks), len(test.expected), "Expected %d names in order", len(hooks)) + require.Len(t, hooks, len(test.expected), "Expected %d names in order", len(hooks)) defer buf.Reset() orig := hooks for _, r := range sortHooksByKind(hooks, test.order) { diff --git a/pkg/release/v1/util/manifest_sorter_test.go b/pkg/release/v1/util/manifest_sorter_test.go index 2cdb55dc3..b02400938 100644 --- a/pkg/release/v1/util/manifest_sorter_test.go +++ b/pkg/release/v1/util/manifest_sorter_test.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -169,7 +168,7 @@ metadata: assert.True(t, kindFound, "Got unexpected kind %s", out.Kind) expectedHooks := expect.hooks[out.Name] - assert.True(t, reflect.DeepEqual(expectedHooks, out.Events), "expected events: %v but got: %v", expectedHooks, out.Events) + assert.Equal(t, expectedHooks, out.Events, "expected events: %v but got: %v", expectedHooks, out.Events) } } assert.True(t, found, "Result not found: %v", out) @@ -204,6 +203,6 @@ metadata: sorted = sortManifestsByKind(sorted, InstallOrder) for i, m := range generic { - assert.Equal(t, sorted[i].Content, m.Content) + assert.Equal(t, m.Content, sorted[i].Content) } } diff --git a/pkg/release/v1/util/manifest_test.go b/pkg/release/v1/util/manifest_test.go index 6bcf902d7..d1efa21e1 100644 --- a/pkg/release/v1/util/manifest_test.go +++ b/pkg/release/v1/util/manifest_test.go @@ -17,7 +17,6 @@ limitations under the License. package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -511,7 +510,7 @@ metadata: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := SplitManifests(tt.input) - assert.True(t, reflect.DeepEqual(result, tt.expected), "SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) + assert.Equal(t, tt.expected, result, "SplitManifests() =\n%v\nwant:\n%v", result, tt.expected) }) } } From addbab2e6444954eb1a9bde86730ee7d11810b46 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Tue, 16 Jun 2026 15:49:51 +1200 Subject: [PATCH 10/10] refactor(internal): convert tests to testify assert/require Convert internal/urlutil, internal/cli/output, internal/monocular, and internal/sympath test files from native Go testing patterns (t.Errorf, t.Fatalf, t.Error, t.Fatal) to github.com/stretchr/testify equivalents (assert.X, require.X). Signed-off-by: George Jenkins --- internal/cli/output/color_test.go | 18 +++++++---------- internal/monocular/client_test.go | 11 +++++------ internal/monocular/search_test.go | 15 ++++++--------- internal/sympath/walk_test.go | 32 +++++++++++-------------------- internal/urlutil/urlutil_test.go | 23 +++++++++++----------- 5 files changed, 40 insertions(+), 59 deletions(-) diff --git a/internal/cli/output/color_test.go b/internal/cli/output/color_test.go index f0484fc3a..807086c62 100644 --- a/internal/cli/output/color_test.go +++ b/internal/cli/output/color_test.go @@ -20,6 +20,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/release/common" ) @@ -93,14 +95,12 @@ func TestColorizeStatus(t *testing.T) { // In test environment, term.IsTerminal will be false, so we won't get color // unless we're testing the logic without terminal detection - if hasColor && !tt.wantColor { - t.Errorf("ColorizeStatus() returned color when none expected: %q", result) + if hasColor { + assert.True(t, tt.wantColor, "ColorizeStatus() returned color when none expected: %q", result) } // Always check the status text is present - if !strings.Contains(result, tt.status.String()) { - t.Errorf("ColorizeStatus() = %q, want to contain %q", result, tt.status.String()) - } + assert.Contains(t, result, tt.status.String()) }) } } @@ -139,9 +139,7 @@ func TestColorizeHeader(t *testing.T) { result := ColorizeHeader(tt.header, tt.noColor) // Always check the header text is present - if !strings.Contains(result, tt.header) { - t.Errorf("ColorizeHeader() = %q, want to contain %q", result, tt.header) - } + assert.Contains(t, result, tt.header) }) } } @@ -180,9 +178,7 @@ func TestColorizeNamespace(t *testing.T) { result := ColorizeNamespace(tt.namespace, tt.noColor) // Always check the namespace text is present - if !strings.Contains(result, tt.namespace) { - t.Errorf("ColorizeNamespace() = %q, want to contain %q", result, tt.namespace) - } + assert.Contains(t, result, tt.namespace) }) } } diff --git a/internal/monocular/client_test.go b/internal/monocular/client_test.go index abf914ef5..6456557d8 100644 --- a/internal/monocular/client_test.go +++ b/internal/monocular/client_test.go @@ -18,14 +18,13 @@ package monocular import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNew(t *testing.T) { c, err := New("https://hub.helm.sh") - if err != nil { - t.Errorf("error creating client: %s", err) - } - if c.BaseURL != "https://hub.helm.sh" { - t.Errorf("incorrect BaseURL. Expected \"https://hub.helm.sh\" but got %q", c.BaseURL) - } + require.NoError(t, err) + assert.Equal(t, "https://hub.helm.sh", c.BaseURL) } diff --git a/internal/monocular/search_test.go b/internal/monocular/search_test.go index e4475f24e..85327cba7 100644 --- a/internal/monocular/search_test.go +++ b/internal/monocular/search_test.go @@ -21,6 +21,9 @@ import ( "net/http" "net/http/httptest" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // A search response for phpmyadmin containing 2 results @@ -33,16 +36,10 @@ func TestSearch(t *testing.T) { defer ts.Close() c, err := New(ts.URL) - if err != nil { - t.Errorf("unable to create monocular client: %s", err) - } + require.NoError(t, err, "unable to create monocular client") results, err := c.SearchWithContext(t.Context(), "phpmyadmin") - if err != nil { - t.Errorf("unable to search monocular: %s", err) - } + require.NoError(t, err, "unable to search monocular") - if len(results) != 2 { - t.Error("Did not receive the expected number of results") - } + assert.Len(t, results, 2) } diff --git a/internal/sympath/walk_test.go b/internal/sympath/walk_test.go index 1eba8b996..b5bffbdab 100644 --- a/internal/sympath/walk_test.go +++ b/internal/sympath/walk_test.go @@ -24,6 +24,9 @@ import ( "os" "path/filepath" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type Node struct { @@ -80,21 +83,14 @@ func makeTree(t *testing.T) { walkTree(tree, tree.name, func(path string, n *Node) { if n.entries == nil { if n.symLinkedTo != "" { - if err := os.Symlink(n.symLinkedTo, path); err != nil { - t.Fatalf("makeTree: %v", err) - } + require.NoError(t, os.Symlink(n.symLinkedTo, path), "makeTree") } else { fd, err := os.Create(path) - if err != nil { - t.Fatalf("makeTree: %v", err) - return - } + require.NoError(t, err, "makeTree") fd.Close() } } else { - if err := os.Mkdir(path, 0770); err != nil { - t.Fatalf("makeTree: %v", err) - } + require.NoError(t, os.Mkdir(path, 0770), "makeTree") } }) } @@ -102,8 +98,8 @@ func makeTree(t *testing.T) { func checkMarks(t *testing.T, report bool) { t.Helper() walkTree(tree, tree.name, func(path string, n *Node) { - if n.marks != n.expectedMarks && report { - t.Errorf("node %s mark = %d; expected %d", path, n.marks, n.expectedMarks) + if report { + assert.Equal(t, n.expectedMarks, n.marks, "node %s", path) } n.marks = 0 }) @@ -137,16 +133,10 @@ func TestWalk(t *testing.T) { } // Expect no errors. err := Walk(tree.name, markFn) - if err != nil { - t.Fatalf("no error expected, found: %s", err) - } - if len(errors) != 0 { - t.Fatalf("unexpected errors: %s", errors) - } + require.NoError(t, err) + require.Empty(t, errors, "unexpected errors") checkMarks(t, true) // cleanup - if err := os.RemoveAll(tree.name); err != nil { - t.Errorf("removeTree: %v", err) - } + assert.NoError(t, os.RemoveAll(tree.name), "removeTree") } diff --git a/internal/urlutil/urlutil_test.go b/internal/urlutil/urlutil_test.go index 82acc40fe..d6b3f5256 100644 --- a/internal/urlutil/urlutil_test.go +++ b/internal/urlutil/urlutil_test.go @@ -16,7 +16,11 @@ limitations under the License. package urlutil -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" +) func TestURLJoin(t *testing.T) { tests := []struct { @@ -31,11 +35,9 @@ func TestURLJoin(t *testing.T) { } for _, tt := range tests { - if got, err := URLJoin(tt.url, tt.paths...); err != nil { - t.Errorf("%s: error %q", tt.name, err) - } else if got != tt.expect { - t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got) - } + got, err := URLJoin(tt.url, tt.paths...) + assert.NoError(t, err, tt.name) + assert.Equal(t, tt.expect, got, tt.name) } } @@ -60,9 +62,7 @@ func TestEqual(t *testing.T) { {"%/1234", "%/123", false}, {"/1234", "%/1234", false}, } { - if tt.match != Equal(tt.a, tt.b) { - t.Errorf("Expected %q==%q to be %t", tt.a, tt.b, tt.match) - } + assert.Equal(t, tt.match, Equal(tt.a, tt.b), "Expected %q==%q to be %t", tt.a, tt.b, tt.match) } } @@ -74,8 +74,7 @@ func TestExtractHostname(t *testing.T) { "https://example.com:31337/not/with/a/bang/but/a/whimper": "example.com", } for start, expect := range tests { - if got, _ := ExtractHostname(start); got != expect { - t.Errorf("Got %q, expected %q", got, expect) - } + got, _ := ExtractHostname(start) + assert.Equal(t, expect, got) } }