From 1d4665e54c28a41a5631c19f3bff14ef3d4e058f Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 20 Jul 2026 20:50:56 +0200 Subject: [PATCH] chore(pkg): refactor: convert tests to testify assert/require part 20 #### Description refactor: convert tests to testify assert/require in pkg/cmd Signed-off-by: Matthieu MOREL --- pkg/chart/common/util/values_test.go | 52 ++++++-------- pkg/chart/loader/archive/archive_test.go | 29 +++----- pkg/chart/v2/lint/lint_test.go | 43 +++--------- pkg/cmd/list_test.go | 46 +++---------- pkg/cmd/package_test.go | 44 +++++------- pkg/cmd/release_testing_test.go | 12 ++-- pkg/cmd/source_date_epoch_test.go | 44 ++++-------- .../chart_downloader_windows_test.go | 67 +++++++++---------- pkg/kube/wait_test.go | 36 +++------- pkg/registry/client_tls_test.go | 23 +++---- pkg/release/v1/info_test.go | 43 +++++------- 11 files changed, 151 insertions(+), 288 deletions(-) diff --git a/pkg/chart/common/util/values_test.go b/pkg/chart/common/util/values_test.go index 1882fd0c4..e87a69237 100644 --- a/pkg/chart/common/util/values_test.go +++ b/pkg/chart/common/util/values_test.go @@ -20,6 +20,9 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" ) @@ -61,42 +64,26 @@ func TestToRenderValues(t *testing.T) { } res, err := ToRenderValuesWithSchemaValidation(c, overrideValues, o, nil, false) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // Ensure that the top-level values are all set. metamap := res["Chart"].(map[string]any) - if name := metamap["Name"]; name.(string) != "test" { - t.Errorf("Expected chart name 'test', got %q", name) - } + name := metamap["Name"] + assert.Equalf(t, "test", name.(string), "Expected chart name 'test', got %q", name) relmap := res["Release"].(map[string]any) - if name := relmap["Name"]; name.(string) != "Seven Voyages" { - t.Errorf("Expected release name 'Seven Voyages', got %q", name) - } - if namespace := relmap["Namespace"]; namespace.(string) != "default" { - t.Errorf("Expected namespace 'default', got %q", namespace) - } - if revision := relmap["Revision"]; revision.(int) != 1 { - t.Errorf("Expected revision '1', got %d", revision) - } - if relmap["IsUpgrade"].(bool) { - t.Error("Expected upgrade to be false.") - } - if !relmap["IsInstall"].(bool) { - t.Error("Expected install to be true.") - } - if !res["Capabilities"].(*common.Capabilities).APIVersions.Has("v1") { - t.Error("Expected Capabilities to have v1 as an API") - } - if res["Capabilities"].(*common.Capabilities).KubeVersion.Major != "1" { - t.Error("Expected Capabilities to have a Kube version") - } + name = relmap["Name"] + assert.Equalf(t, "Seven Voyages", name.(string), "Expected release name 'Seven Voyages', got %q", name) + namespace := relmap["Namespace"] + assert.Equalf(t, "default", namespace.(string), "Expected namespace 'default', got %q", namespace) + revision := relmap["Revision"] + assert.Equalf(t, 1, revision.(int), "Expected revision '1', got %d", revision) + assert.False(t, relmap["IsUpgrade"].(bool), "Expected upgrade to be false.") + assert.True(t, relmap["IsInstall"].(bool), "Expected install to be true.") + assert.True(t, res["Capabilities"].(*common.Capabilities).APIVersions.Has("v1"), "Expected Capabilities to have v1 as an API") + assert.Equal(t, "1", res["Capabilities"].(*common.Capabilities).KubeVersion.Major, "Expected Capabilities to have a Kube version") vals := res["Values"].(common.Values) - if vals["name"] != "Haroun" { - t.Errorf("Expected 'Haroun', got %q (%v)", vals["name"], vals) - } + assert.Equal(t, "Haroun", vals["name"], "Expected 'Haroun', got %q (%v)", vals["name"], vals) where := vals["where"].(map[string]any) expects := map[string]string{ "city": "Baghdad", @@ -104,8 +91,7 @@ func TestToRenderValues(t *testing.T) { "title": "caliph", } for field, expect := range expects { - if got := where[field]; got != expect { - t.Errorf("Expected %q, got %q (%v)", expect, got, where) - } + got := where[field] + assert.Equalf(t, got, expect, "Expected %q, got %q (%v)", expect, got, where) } } diff --git a/pkg/chart/loader/archive/archive_test.go b/pkg/chart/loader/archive/archive_test.go index 2fe09e9b2..3c3d336c3 100644 --- a/pkg/chart/loader/archive/archive_test.go +++ b/pkg/chart/loader/archive/archive_test.go @@ -21,6 +21,8 @@ import ( "bytes" "compress/gzip" "testing" + + "github.com/stretchr/testify/require" ) func TestLoadArchiveFiles(t *testing.T) { @@ -34,9 +36,7 @@ func TestLoadArchiveFiles(t *testing.T) { generate: func(_ *tar.Writer) {}, check: func(t *testing.T, _ []*BufferedFile, err error) { t.Helper() - if err.Error() != "no files in chart archive" { - t.Fatalf(`expected "no files in chart archive", got [%#v]`, err) - } + require.EqualError(t, err, "no files in chart archive") }, }, { @@ -44,32 +44,21 @@ func TestLoadArchiveFiles(t *testing.T) { generate: func(w *tar.Writer) { // simulate the presence of a `pax_global_header` file like you would get when // processing a GitHub release archive. - err := w.WriteHeader(&tar.Header{ + require.NoError(t, w.WriteHeader(&tar.Header{ Typeflag: tar.TypeXGlobalHeader, Name: "pax_global_header", - }) - if err != nil { - t.Fatal(err) - } + })) // we need to have at least one file, otherwise we'll get the "no files in chart archive" error - err = w.WriteHeader(&tar.Header{ + require.NoError(t, w.WriteHeader(&tar.Header{ Typeflag: tar.TypeReg, Name: "dir/empty", - }) - if err != nil { - t.Fatal(err) - } + })) }, check: func(t *testing.T, files []*BufferedFile, err error) { t.Helper() - if err != nil { - t.Fatalf(`got unwanted error [%#v] for tar file with pax_global_header content`, err) - } - - if len(files) != 1 { - t.Fatalf(`expected to get one file but got [%v]`, files) - } + require.NoErrorf(t, err, `got unwanted error for tar file with pax_global_header content`) + require.Lenf(t, files, 1, `expected to get one file but got [%v]`, files) }, }, } diff --git a/pkg/chart/v2/lint/lint_test.go b/pkg/chart/v2/lint/lint_test.go index 85960131b..1782c01d4 100644 --- a/pkg/chart/v2/lint/lint_test.go +++ b/pkg/chart/v2/lint/lint_test.go @@ -97,34 +97,22 @@ func TestBadChart(t *testing.T) { func TestInvalidYaml(t *testing.T) { var values map[string]any m := RunAll(badYamlFileDir, values, namespace).Messages - if len(m) != 1 { - t.Fatalf("All didn't fail with expected errors, got %#v", m) - } - if !strings.Contains(m[0].Err.Error(), "deliberateSyntaxError") { - t.Error("All didn't have the error for deliberateSyntaxError") - } + require.Len(t, m, 1, "All didn't fail with expected errors, got %#v", m) + assert.ErrorContains(t, m[0].Err, "deliberateSyntaxError", "All didn't have the error for deliberateSyntaxError") } func TestInvalidChartYaml(t *testing.T) { var values map[string]any m := RunAll(invalidChartFileDir, values, namespace).Messages - if len(m) != 2 { - t.Fatalf("All didn't fail with expected errors, got %#v", m) - } - if !strings.Contains(m[0].Err.Error(), "failed to strictly parse chart metadata file") { - t.Error("All didn't have the error for duplicate YAML keys") - } + require.Len(t, m, 2, "All didn't fail with expected errors, got %#v", m) + assert.ErrorContains(t, m[0].Err, "failed to strictly parse chart metadata file", "All didn't have the error for duplicate YAML keys") } func TestBadValues(t *testing.T) { var values map[string]any m := RunAll(badValuesFileDir, values, namespace).Messages - if len(m) < 1 { - t.Fatalf("All didn't fail with expected errors, got %#v", m) - } - if !strings.Contains(m[0].Err.Error(), "unable to parse YAML") { - t.Errorf("All didn't have the error for invalid key format: %s", m[0].Err) - } + require.GreaterOrEqualf(t, len(m), 1, "All didn't fail with expected errors, got %#v", m) + assert.ErrorContains(t, m[0].Err, "unable to parse YAML", "All didn't have the error for invalid key format: %s", m[0].Err) } func TestBadCrdFile(t *testing.T) { @@ -154,11 +142,7 @@ func TestHelmCreateChart(t *testing.T) { dir := t.TempDir() createdChart, err := chartutil.Create("testhelmcreatepasseslint", dir) - if err != nil { - t.Error(err) - // Fatal is bad because of the defer. - return - } + require.NoError(t, err) // Note: we test with strict=true here, even though others have // strict = false. @@ -182,10 +166,7 @@ func TestHelmCreateChart(t *testing.T) { // of the `--set` flag. func TestHelmCreateChart_CheckDeprecatedWarnings(t *testing.T) { createdChart, err := chartutil.Create("checkdeprecatedwarnings", t.TempDir()) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) // Add values to enable hpa, and ingress which are disabled by default. // This is the equivalent of: @@ -238,11 +219,7 @@ func TestMalformedTemplate(t *testing.T) { case <-c: t.Fatal("lint malformed template timeout") case <-ch: - if len(m) != 1 { - t.Fatalf("All didn't fail with expected errors, got %#v", m) - } - if !strings.Contains(m[0].Err.Error(), "invalid character '{'") { - t.Error("All didn't have the error for invalid character '{'") - } + require.Len(t, m, 1, "All didn't fail with expected errors, got %#v", m) + assert.ErrorContains(t, m[0].Err, "invalid character '{'", "All didn't have the error for invalid character '{'") } } diff --git a/pkg/cmd/list_test.go b/pkg/cmd/list_test.go index f45616250..41426fedb 100644 --- a/pkg/cmd/list_test.go +++ b/pkg/cmd/list_test.go @@ -426,20 +426,9 @@ func TestReleaseListWriterMethods(t *testing.T) { var buf []byte out := &bytesWriter{buf: &buf} - err := writer.WriteJSON(out) - if err != nil { - t.Errorf("WriteJSON failed: %v", err) - } - - err = writer.WriteYAML(out) - if err != nil { - t.Errorf("WriteYAML failed: %v", err) - } - - err = writer.WriteTable(out) - if err != nil { - t.Errorf("WriteTable failed: %v", err) - } + require.NoError(t, writer.WriteJSON(out), "WriteJSON failed") + require.NoError(t, writer.WriteYAML(out), "WriteYAML failed") + assert.NoError(t, writer.WriteTable(out), "WriteTable failed") }) } @@ -448,20 +437,9 @@ func TestReleaseListWriterMethods(t *testing.T) { var buf []byte out := &bytesWriter{buf: &buf} - err := writer.WriteJSON(out) - if err != nil { - t.Errorf("WriteJSON failed: %v", err) - } - - err = writer.WriteYAML(out) - if err != nil { - t.Errorf("WriteYAML failed: %v", err) - } - - err = writer.WriteTable(out) - if err != nil { - t.Errorf("WriteTable failed: %v", err) - } + require.NoError(t, writer.WriteJSON(out), "WriteJSON failed") + require.NoError(t, writer.WriteYAML(out), "WriteYAML failed") + assert.NoError(t, writer.WriteTable(out), "WriteTable failed") } func TestFilterReleases(t *testing.T) { @@ -512,9 +490,7 @@ func TestFilterReleases(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := filterReleases(tt.releases, tt.ignoredReleaseNames) - if len(result) != tt.expectedCount { - t.Errorf("Expected %d releases, got %d", tt.expectedCount, len(result)) - } + assert.Len(t, result, tt.expectedCount, "Expected %d releases, got %d", tt.expectedCount, len(result)) }) } } @@ -603,13 +579,9 @@ func TestListStatusMapping(t *testing.T) { } writer := newReleaseListWriter(releaseFixture, "", false, false) - if len(writer.releases) != 1 { - t.Errorf("Expected 1 release, got %d", len(writer.releases)) - } + assert.Len(t, writer.releases, 1, "Expected 1 release, got %d", len(writer.releases)) - if writer.releases[0].Status != tc.status.String() { - t.Errorf("Expected status %s, got %s", tc.status.String(), writer.releases[0].Status) - } + assert.Equal(t, tc.status.String(), writer.releases[0].Status, "Expected status %s, got %s", tc.status.String(), writer.releases[0].Status) }) } } diff --git a/pkg/cmd/package_test.go b/pkg/cmd/package_test.go index 5705c9b9e..d78f5d88d 100644 --- a/pkg/cmd/package_test.go +++ b/pkg/cmd/package_test.go @@ -23,6 +23,9 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/internal/test/ensure" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" @@ -105,18 +108,14 @@ func TestPackage(t *testing.T) { } origDir, err := os.Getwd() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Chdir(t.TempDir()) ensure.HelmHome(t) - if err := os.MkdirAll("toot", 0o777); err != nil { - t.Fatal(err) - } + require.NoError(t, os.MkdirAll("toot", 0o777)) // This is an unfortunate byproduct of the tmpdir if v, ok := tt.flags["keyring"]; ok && v != "" { @@ -140,6 +139,7 @@ func TestPackage(t *testing.T) { } } _, _, err = executeActionCommand(strings.Join(cmd, " ")) + if err != nil { if tt.err && re.MatchString(err.Error()) { return @@ -148,19 +148,15 @@ func TestPackage(t *testing.T) { } if tt.hasfile != "" { - if fi, err := os.Stat(tt.hasfile); err != nil { - t.Errorf("%q: expected file %q, got err %q", tt.name, tt.hasfile, err) - } else if fi.Size() == 0 { - t.Errorf("%q: file %q has zero bytes.", tt.name, tt.hasfile) - } + fi, err := os.Stat(tt.hasfile) + require.NoErrorf(t, err, "%q: expected file %q", tt.name, tt.hasfile) + assert.NotEqualf(t, 0, fi.Size(), "%q: file %q has zero bytes.", tt.name, tt.hasfile) } if v, ok := tt.flags["sign"]; ok && v == "1" { - if fi, err := os.Stat(tt.hasfile + ".prov"); err != nil { - t.Errorf("%q: expected provenance file", tt.name) - } else if fi.Size() == 0 { - t.Errorf("%q: provenance file is empty", tt.name) - } + fi, err := os.Stat(tt.hasfile + ".prov") + require.NoErrorf(t, err, "%q: expected provenance file", tt.name) + assert.NotEqualf(t, 0, fi.Size(), "%q: provenance file is empty", tt.name) } }) } @@ -178,18 +174,12 @@ func TestSetAppVersion(t *testing.T) { t.Fatal(err) } chartPath := filepath.Join(dir, "alpine-0.1.0.tgz") - if fi, err := os.Stat(chartPath); err != nil { - t.Errorf("expected file %q, got err %q", chartPath, err) - } else if fi.Size() == 0 { - t.Errorf("file %q has zero bytes.", chartPath) - } + fi, err := os.Stat(chartPath) + require.NoErrorf(t, err, "expected file %q", chartPath) + assert.NotEqualf(t, 0, fi.Size(), "file %q has zero bytes.", chartPath) ch, err = loader.Load(chartPath) - if err != nil { - t.Fatalf("unexpected error loading packaged chart: %v", err) - } - if ch.Metadata.AppVersion != expectedAppVersion { - t.Errorf("expected app-version %q, found %q", expectedAppVersion, ch.Metadata.AppVersion) - } + require.NoError(t, err, "unexpected error loading packaged chart") + assert.Equal(t, expectedAppVersion, ch.Metadata.AppVersion, "expected app-version %q, found %q", expectedAppVersion, ch.Metadata.AppVersion) } func TestPackageFileCompletion(t *testing.T) { diff --git a/pkg/cmd/release_testing_test.go b/pkg/cmd/release_testing_test.go index fdb5df1e9..a39816843 100644 --- a/pkg/cmd/release_testing_test.go +++ b/pkg/cmd/release_testing_test.go @@ -19,9 +19,11 @@ package cmd import ( "bytes" "io" - "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/action" "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" @@ -71,11 +73,7 @@ func TestReleaseTestNotesHandling(t *testing.T) { cmd1 := newReleaseTestCmd(actionConfig, &buf1) cmd1.SetArgs([]string{"test-release"}) err1 := cmd1.Execute() - if err1 != nil { - t.Fatalf("Unexpected error for default test: %v", err1) - } + require.NoError(t, err1, "Unexpected error for default test") output1 := buf1.String() - if strings.Contains(output1, "NOTES:") { - t.Errorf("Expected notes to be hidden by default, but found NOTES section in output: %s", output1) - } + assert.NotContains(t, output1, "NOTES:", "Expected notes to be hidden by default, but found NOTES section in output: %s", output1) } diff --git a/pkg/cmd/source_date_epoch_test.go b/pkg/cmd/source_date_epoch_test.go index 719de7c90..fa64c08b0 100644 --- a/pkg/cmd/source_date_epoch_test.go +++ b/pkg/cmd/source_date_epoch_test.go @@ -19,64 +19,48 @@ package cmd import ( "testing" "time" + + "github.com/stretchr/testify/require" ) func TestSourceDateEpochFromEnv(t *testing.T) { t.Setenv("SOURCE_DATE_EPOCH", "1609459200") got, err := sourceDateEpochFromEnv() - if err != nil { - t.Fatalf("sourceDateEpochFromEnv() error: %v", err) - } - if got == nil { - t.Fatal("expected non-nil epoch") - } + require.NoError(t, err, "sourceDateEpochFromEnv()") + require.NotNil(t, got, "expected non-nil epoch") want := time.Unix(1609459200, 0).UTC() - if !got.Equal(want) { - t.Fatalf("expected %v, got %v", want, *got) - } + require.Truef(t, got.Equal(want), "expected %v, got %v", want, *got) } func TestSourceDateEpochFromEnvUnset(t *testing.T) { t.Setenv("SOURCE_DATE_EPOCH", "") got, err := sourceDateEpochFromEnv() - if err != nil { - t.Fatalf("sourceDateEpochFromEnv() error: %v", err) - } - if got != nil { - t.Fatalf("expected nil epoch, got %v", *got) - } + require.NoError(t, err, "sourceDateEpochFromEnv()") + require.Nil(t, got, "expected nil epoch") } func TestSourceDateEpochFromEnvInvalid(t *testing.T) { t.Setenv("SOURCE_DATE_EPOCH", "not-a-number") - if _, err := sourceDateEpochFromEnv(); err == nil { - t.Fatal("expected error for invalid SOURCE_DATE_EPOCH") - } + _, err := sourceDateEpochFromEnv() + require.Error(t, err, "expected error for invalid SOURCE_DATE_EPOCH") } func TestSourceDateEpochFromEnvNegative(t *testing.T) { t.Setenv("SOURCE_DATE_EPOCH", "-1") - if _, err := sourceDateEpochFromEnv(); err == nil { - t.Fatal("expected error for negative SOURCE_DATE_EPOCH") - } + _, err := sourceDateEpochFromEnv() + require.Error(t, err, "expected error for negative SOURCE_DATE_EPOCH") } func TestSourceDateEpochFromEnvZero(t *testing.T) { t.Setenv("SOURCE_DATE_EPOCH", "0") got, err := sourceDateEpochFromEnv() - if err != nil { - t.Fatalf("sourceDateEpochFromEnv() error: %v", err) - } - if got == nil { - t.Fatal("expected non-nil epoch") - } + require.NoError(t, err, "sourceDateEpochFromEnv() error") + require.NotNil(t, got, "expected non-nil epoch") want := time.Unix(0, 0).UTC() - if !got.Equal(want) { - t.Fatalf("expected %v, got %v", want, *got) - } + require.Truef(t, got.Equal(want), "expected %v, got %v", want, *got) } diff --git a/pkg/downloader/chart_downloader_windows_test.go b/pkg/downloader/chart_downloader_windows_test.go index 732416701..2b6bea5eb 100644 --- a/pkg/downloader/chart_downloader_windows_test.go +++ b/pkg/downloader/chart_downloader_windows_test.go @@ -23,6 +23,9 @@ import ( "sync" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/repo/v1/repotest" @@ -36,9 +39,7 @@ func TestParallelDownloadTo(t *testing.T) { srv := repotest.NewTempServer(t, repotest.WithChartSourceGlob("testdata/*.tgz")) defer srv.Stop() - if err := srv.CreateIndex(); err != nil { - t.Fatal(err) - } + require.NoError(t, srv.CreateIndex()) dest := t.TempDir() cacheDir := t.TempDir() @@ -86,46 +87,38 @@ func TestParallelDownloadTo(t *testing.T) { } // With the file locking fix, all parallel downloads should succeed - if failedCount > 0 { - t.Errorf("Parallel downloads failed: %d out of %d downloads failed due to concurrent file access", failedCount, numDownloads) - } + assert.Falsef(t, failedCount > 0, "Parallel downloads failed: %d out of %d downloads failed due to concurrent file access", failedCount, numDownloads) // Verify the file exists and is valid expectedFile := filepath.Join(dest, "local-subchart-0.1.0.tgz") info, err := os.Stat(expectedFile) - if err != nil { - t.Errorf("Expected file %s does not exist: %v", expectedFile, err) - } else { - // Verify the file is not empty - if info.Size() == 0 { - t.Errorf("Downloaded file %s is empty (0 bytes)", expectedFile) - } - - // Verify the file has the expected size (should match the source file) - sourceFile := "testdata/local-subchart-0.1.0.tgz" - sourceInfo, err := os.Stat(sourceFile) - if err == nil && info.Size() != sourceInfo.Size() { - t.Errorf("Downloaded file size (%d bytes) doesn't match source file size (%d bytes)", - info.Size(), sourceInfo.Size()) - } + require.NoError(t, err, "Expected file %s does not exist: %v", expectedFile) + // Verify the file is not empty + assert.Falsef(t, info.Size() == 0, "Downloaded file %s is empty (0 bytes)", expectedFile) + + // Verify the file has the expected size (should match the source file) + sourceFile := "testdata/local-subchart-0.1.0.tgz" + sourceInfo, err := os.Stat(sourceFile) + if err == nil && info.Size() != sourceInfo.Size() { + t.Errorf("Downloaded file size (%d bytes) doesn't match source file size (%d bytes)", + info.Size(), sourceInfo.Size()) + } - // Verify it's a valid tar.gz file by checking the magic bytes - file, err := os.Open(expectedFile) - if err == nil { - defer file.Close() - // gzip magic bytes are 0x1f 0x8b - magic := make([]byte, 2) - if n, err := file.Read(magic); err == nil && n == 2 { - if magic[0] != 0x1f || magic[1] != 0x8b { - t.Errorf("Downloaded file is not a valid gzip file (magic bytes: %x)", magic) - } + // Verify it's a valid tar.gz file by checking the magic bytes + file, err := os.Open(expectedFile) + if err == nil { + defer file.Close() + // gzip magic bytes are 0x1f 0x8b + magic := make([]byte, 2) + if n, err := file.Read(magic); err == nil && n == 2 { + if magic[0] != 0x1f || magic[1] != 0x8b { + t.Errorf("Downloaded file is not a valid gzip file (magic bytes: %x)", magic) } } - - // Verify no lock file was left behind - lockFile := expectedFile + ".lock" - if _, err := os.Stat(lockFile); err == nil { - t.Errorf("Lock file %s was not cleaned up", lockFile) - } } + + // Verify no lock file was left behind + lockFile := expectedFile + ".lock" + _, err = os.Stat(lockFile) + assert.Falsef(t, err == nil, "Lock file %s was not cleaned up", lockFile) } diff --git a/pkg/kube/wait_test.go b/pkg/kube/wait_test.go index acdfffa23..9cf1e5506 100644 --- a/pkg/kube/wait_test.go +++ b/pkg/kube/wait_test.go @@ -19,7 +19,6 @@ package kube import ( "errors" "net/http" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -298,17 +297,12 @@ func TestLegacyWaiter_waitForPodSuccess(t *testing.T) { t.Run(tt.name, func(t *testing.T) { done, err := lw.waitForPodSuccess(tt.obj, "foo") if tt.wantErr { - if err == nil { - t.Error("expected error, got none") - } else if !strings.Contains(err.Error(), tt.errMessage) { - t.Errorf("expected error to contain %q, got %q", tt.errMessage, err.Error()) - } - } else if err != nil { - t.Errorf("unexpected error: %v", err) - } - if done != tt.wantDone { - t.Errorf("got done=%v, want %v", done, tt.wantDone) + require.Error(t, err, "expected error, got none") + require.ErrorContains(t, err, tt.errMessage, "expected error to contain %q, got %q", tt.errMessage, err.Error()) + } else { + require.NoError(t, err) } + assert.Equal(t, tt.wantDone, done, "got done=%v, want %v", done, tt.wantDone) }) } } @@ -390,17 +384,11 @@ func TestLegacyWaiter_waitForJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { done, err := lw.waitForJob(tt.obj, "test-job") if tt.wantErr { - if err == nil { - t.Error("expected error, got none") - } else if !strings.Contains(err.Error(), tt.errMessage) { - t.Errorf("expected error to contain %q, got %q", tt.errMessage, err.Error()) - } - } else if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if done != tt.wantDone { - t.Errorf("got done=%v, want %v", done, tt.wantDone) + require.Error(t, err, "expected error, got none") + require.ErrorContainsf(t, err, tt.errMessage, "expected error to contain %q, got %q", tt.errMessage, err.Error()) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantDone, done, "got done=%v, want %v", done, tt.wantDone) } }) } @@ -459,9 +447,7 @@ func TestLegacyWaiter_isRetryableError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := lw.isRetryableError(tt.err, info) - if got != tt.wantRetry { - t.Errorf("isRetryableError() = %v, want %v", got, tt.wantRetry) - } + assert.Equal(t, tt.wantRetry, got, "isRetryableError() = %v, want %v", got, tt.wantRetry) }) } } diff --git a/pkg/registry/client_tls_test.go b/pkg/registry/client_tls_test.go index 45eff232c..0bad5df64 100644 --- a/pkg/registry/client_tls_test.go +++ b/pkg/registry/client_tls_test.go @@ -40,22 +40,19 @@ func (suite *TLSRegistryClientTestSuite) TearDownSuite() { } func (suite *TLSRegistryClientTestSuite) Test_0_Login() { - err := suite.RegistryClient.Login(suite.DockerRegistryHost, + suite.Require().Error(suite.RegistryClient.Login(suite.DockerRegistryHost, LoginOptBasicAuth("badverybad", "ohsobad"), - LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)) - suite.Require().Error(err, "error logging into registry with bad credentials") + LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)), "error logging into registry with bad credentials") - err = suite.RegistryClient.Login(suite.DockerRegistryHost, + suite.Require().NoError(suite.RegistryClient.Login(suite.DockerRegistryHost, LoginOptBasicAuth(testUsername, testPassword), - LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)) - suite.Require().NoError(err, "no error logging into registry with good credentials") + LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)), "no error logging into registry with good credentials") } func (suite *TLSRegistryClientTestSuite) Test_1_Login() { - err := suite.RegistryClient.Login(suite.DockerRegistryHost, + suite.Require().Error(suite.RegistryClient.Login(suite.DockerRegistryHost, LoginOptBasicAuth("badverybad", "ohsobad"), - LoginOptTLSClientConfigFromConfig(&tls.Config{})) - suite.Require().Error(err, "error logging into registry with bad credentials") + LoginOptTLSClientConfigFromConfig(&tls.Config{})), "error logging into registry with bad credentials") // Create a *tls.Config from tlsCert, tlsKey, and tlsCA. cert, err := tls.LoadX509KeyPair(tlsCert, tlsKey) @@ -69,10 +66,9 @@ func (suite *TLSRegistryClientTestSuite) Test_1_Login() { RootCAs: rootCAs, } - err = suite.RegistryClient.Login(suite.DockerRegistryHost, + suite.Require().NoError(suite.RegistryClient.Login(suite.DockerRegistryHost, LoginOptBasicAuth(testUsername, testPassword), - LoginOptTLSClientConfigFromConfig(conf)) - suite.Require().NoError(err, "no error logging into registry with good credentials") + LoginOptTLSClientConfigFromConfig(conf)), "no error logging into registry with good credentials") } func (suite *TLSRegistryClientTestSuite) Test_1_Push() { @@ -94,8 +90,7 @@ func (suite *TLSRegistryClientTestSuite) Test_4_Logout() { suite.Require().Error(err, "failed to delete the credential for this-host-aint-real:5000") } - err = suite.RegistryClient.Logout(suite.DockerRegistryHost) - suite.Require().NoError(err, "no error logging out of registry") + suite.Require().NoError(suite.RegistryClient.Logout(suite.DockerRegistryHost), "no error logging out of registry") } func TestTLSRegistryClientTestSuite(t *testing.T) { diff --git a/pkg/release/v1/info_test.go b/pkg/release/v1/info_test.go index 12b3014f6..61a888cb8 100644 --- a/pkg/release/v1/info_test.go +++ b/pkg/release/v1/info_test.go @@ -253,17 +253,17 @@ func TestInfoUnmarshalJSON(t *testing.T) { err := json.Unmarshal([]byte(tt.input), &info) if tt.wantErr { assert.Error(t, err) - return + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected.FirstDeployed.Unix(), info.FirstDeployed.Unix()) + assert.Equal(t, tt.expected.LastDeployed.Unix(), info.LastDeployed.Unix()) + assert.Equal(t, tt.expected.Deleted.Unix(), info.Deleted.Unix()) + assert.Equal(t, tt.expected.Description, info.Description) + assert.Equal(t, tt.expected.Status, info.Status) + assert.Equal(t, tt.expected.RollbackRevision, info.RollbackRevision) + assert.Equal(t, tt.expected.Notes, info.Notes) + assert.Equal(t, tt.expected.Resources, info.Resources) } - require.NoError(t, err) - assert.Equal(t, tt.expected.FirstDeployed.Unix(), info.FirstDeployed.Unix()) - assert.Equal(t, tt.expected.LastDeployed.Unix(), info.LastDeployed.Unix()) - assert.Equal(t, tt.expected.Deleted.Unix(), info.Deleted.Unix()) - assert.Equal(t, tt.expected.Description, info.Description) - assert.Equal(t, tt.expected.Status, info.Status) - assert.Equal(t, tt.expected.RollbackRevision, info.RollbackRevision) - assert.Equal(t, tt.expected.Notes, info.Notes) - assert.Equal(t, tt.expected.Resources, info.Resources) }) } } @@ -284,9 +284,7 @@ func TestInfoRoundTrip(t *testing.T) { require.NoError(t, err) var decoded Info - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - + require.NoError(t, json.Unmarshal(data, &decoded)) assert.Equal(t, original.FirstDeployed.Unix(), decoded.FirstDeployed.Unix()) assert.Equal(t, original.LastDeployed.Unix(), decoded.LastDeployed.Unix()) assert.Equal(t, original.Deleted.Unix(), decoded.Deleted.Unix()) @@ -330,9 +328,7 @@ func TestInfoRollbackRevisionRoundTrip(t *testing.T) { require.NoError(t, err) var decoded Info - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - + require.NoError(t, json.Unmarshal(data, &decoded)) assert.Equal(t, tt.info.RollbackRevision, decoded.RollbackRevision) assert.Equal(t, tt.info.FirstDeployed.Unix(), decoded.FirstDeployed.Unix()) assert.Equal(t, tt.info.LastDeployed.Unix(), decoded.LastDeployed.Unix()) @@ -342,8 +338,7 @@ func TestInfoRollbackRevisionRoundTrip(t *testing.T) { // Verify omitempty behavior: zero rollback_revision should not appear in JSON if tt.info.RollbackRevision == 0 { var raw map[string]any - err = json.Unmarshal(data, &raw) - require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, &raw)) assert.NotContains(t, raw, "rollback_revision") } }) @@ -356,13 +351,12 @@ func TestInfoEmptyStringRoundTrip(t *testing.T) { input := `{"first_deployed":"","last_deployed":"","deleted":"","status":"deployed","description":"test"}` var info Info - err := json.Unmarshal([]byte(input), &info) - require.NoError(t, err) + require.NoError(t, json.Unmarshal([]byte(input), &info)) // Verify time fields are zero values - assert.True(t, info.FirstDeployed.IsZero()) - assert.True(t, info.LastDeployed.IsZero()) - assert.True(t, info.Deleted.IsZero()) + assert.Zero(t, info.FirstDeployed) + assert.Zero(t, info.LastDeployed) + assert.Zero(t, info.Deleted) assert.Equal(t, common.StatusDeployed, info.Status) assert.Equal(t, "test", info.Description) @@ -371,8 +365,7 @@ func TestInfoEmptyStringRoundTrip(t *testing.T) { require.NoError(t, err) var result map[string]any - err = json.Unmarshal(data, &result) - require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, &result)) // Zero time values should be omitted due to omitzero tag assert.NotContains(t, result, "first_deployed")