From 7ab295fcfcf3977599939d76ffbf7a266401bfbc Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 17 Jun 2026 21:48:26 +1200 Subject: [PATCH] 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) }) } }