diff --git a/internal/chart/v3/lint/rules/chartfile_test.go b/internal/chart/v3/lint/rules/chartfile_test.go index a7669a0aa..9ac4ce75e 100644 --- a/internal/chart/v3/lint/rules/chartfile_test.go +++ b/internal/chart/v3/lint/rules/chartfile_test.go @@ -18,11 +18,15 @@ package rules import ( "errors" + "fmt" "os" "path/filepath" - "strings" + "strconv" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/internal/chart/v3" "helm.sh/helm/v4/internal/chart/v3/lint/support" chartutil "helm.sh/helm/v4/internal/chart/v3/util" @@ -49,33 +53,23 @@ func TestValidateChartYamlNotDirectory(t *testing.T) { defer os.Remove(nonExistingChartFilePath) err := validateChartYamlNotDirectory(nonExistingChartFilePath) - if err == nil { - t.Error("validateChartYamlNotDirectory to return a linter error, got no error") - } + assert.Error(t, err, "validateChartYamlNotDirectory to return a linter error, got no error") } func TestValidateChartYamlFormat(t *testing.T) { err := validateChartYamlFormat(errors.New("Read error")) - if err == nil { - t.Error("validateChartYamlFormat to return a linter error, got no error") - } + require.Error(t, err, "validateChartYamlFormat to return a linter error, got no error") err = validateChartYamlFormat(nil) - if err != nil { - t.Error("validateChartYamlFormat to return no error, got a linter error") - } + assert.NoError(t, err, "validateChartYamlFormat to return no error, got a linter error") } func TestValidateChartName(t *testing.T) { err := validateChartName(badChart) - if err == nil { - t.Error("validateChartName to return a linter error, got no error") - } + require.Error(t, err, "validateChartName to return a linter error, got no error") err = validateChartName(badChartName) - if err == nil { - t.Error("expected validateChartName to return a linter error for an invalid name, got no error") - } + assert.Error(t, err, "expected validateChartName to return a linter error for an invalid name, got no error") } func TestValidateChartVersion(t *testing.T) { @@ -93,20 +87,18 @@ func TestValidateChartVersion(t *testing.T) { var successTest = []string{"0.0.1", "0.0.1+build", "0.0.1-beta"} - for _, test := range failTest { - badChart.Version = test.Version - err := validateChartVersion(badChart) - if err == nil || !strings.Contains(err.Error(), test.ErrorMsg) { - t.Errorf("validateChartVersion(%s) to return \"%s\", got no error", test.Version, test.ErrorMsg) - } + for i, test := range failTest { + t.Run(strconv.Itoa(i), func(t *testing.T) { + badChart.Version = test.Version + err := validateChartVersion(badChart) + require.ErrorContains(t, err, test.ErrorMsg, "validateChartVersion(%s) to return \"%s\", got no error", test.Version, test.ErrorMsg) + }) } for _, version := range successTest { badChart.Version = version err := validateChartVersion(badChart) - if err != nil { - t.Errorf("validateChartVersion(%s) to return no error, got a linter error", version) - } + assert.NoError(t, err, "validateChartVersion(%s) to return no error, got a linter error", version) } } @@ -130,49 +122,43 @@ func TestValidateChartMaintainer(t *testing.T) { } for _, test := range failTest { - badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} - err := validateChartMaintainer(badChart) - if err == nil || !strings.Contains(err.Error(), test.ErrorMsg) { - t.Errorf("validateChartMaintainer(%s, %s) to return \"%s\", got no error", test.Name, test.Email, test.ErrorMsg) - } + t.Run(fmt.Sprintf("%s, %s", test.Name, test.Email), func(t *testing.T) { + badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} + err := validateChartMaintainer(badChart) + require.ErrorContains(t, err, test.ErrorMsg, "validateChartMaintainer(%s, %s) to return \"%s\", got no error", test.Name, test.Email, test.ErrorMsg) + }) } for _, test := range successTest { - badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} - err := validateChartMaintainer(badChart) - if err != nil { - t.Errorf("validateChartMaintainer(%s, %s) to return no error, got %s", test.Name, test.Email, err.Error()) - } + t.Run(fmt.Sprintf("%s, %s", test.Name, test.Email), func(t *testing.T) { + badChart.Maintainers = []*chart.Maintainer{{Name: test.Name, Email: test.Email}} + err := validateChartMaintainer(badChart) + require.NoError(t, err, "validateChartMaintainer(%s, %s) to return no error", test.Name, test.Email) + }) } // Testing for an empty maintainer badChart.Maintainers = []*chart.Maintainer{nil} err := validateChartMaintainer(badChart) - if err == nil { - t.Error("validateChartMaintainer did not return error for nil maintainer as expected") - } - if err.Error() != "a maintainer entry is empty" { - t.Errorf("validateChartMaintainer returned unexpected error for nil maintainer: %s", err.Error()) - } + require.Error(t, err, "validateChartMaintainer did not return error for nil maintainer as expected") + assert.EqualError(t, err, "a maintainer entry is empty", "validateChartMaintainer returned unexpected error for nil maintainer") } func TestValidateChartSources(t *testing.T) { var failTest = []string{"", "RiverRun", "john@winterfell", "riverrun.io"} var successTest = []string{"http://riverrun.io", "https://riverrun.io", "https://riverrun.io/blackfish"} for _, test := range failTest { - badChart.Sources = []string{test} - err := validateChartSources(badChart) - if err == nil || !strings.Contains(err.Error(), "invalid source URL") { - t.Errorf("validateChartSources(%s) to return \"invalid source URL\", got no error", test) - } + t.Run(test, func(t *testing.T) { + badChart.Sources = []string{test} + err := validateChartSources(badChart) + require.ErrorContains(t, err, "invalid source URL", "validateChartSources(%s) to return \"invalid source URL\", got no error", test) + }) } for _, test := range successTest { badChart.Sources = []string{test} err := validateChartSources(badChart) - if err != nil { - t.Errorf("validateChartSources(%s) to return no error, got %s", test, err.Error()) - } + assert.NoError(t, err, "validateChartSources(%s) to return no error", test) } } @@ -184,11 +170,8 @@ func TestValidateChartIconPresence(t *testing.T) { err := validateChartIconPresence(testChart) - if err == nil { - t.Error("validateChartIconPresence to return a linter error, got no error") - } else if !strings.Contains(err.Error(), "icon is recommended") { - t.Errorf("expected %q, got %q", "icon is recommended", err.Error()) - } + require.Error(t, err, "validateChartIconPresence to return a linter error, got no error") + assert.ErrorContains(t, err, "icon is recommended", "expected %q", "icon is recommended") }) t.Run("Icon present", func(t *testing.T) { testChart := &chart.Metadata{ @@ -197,9 +180,7 @@ func TestValidateChartIconPresence(t *testing.T) { err := validateChartIconPresence(testChart) - if err != nil { - t.Errorf("Unexpected error: %q", err.Error()) - } + assert.NoError(t, err, "Unexpected error") }) } @@ -207,19 +188,17 @@ func TestValidateChartIconURL(t *testing.T) { var failTest = []string{"RiverRun", "john@winterfell", "riverrun.io"} var successTest = []string{"http://riverrun.io", "https://riverrun.io", "https://riverrun.io/blackfish.png"} for _, test := range failTest { - badChart.Icon = test - err := validateChartIconURL(badChart) - if err == nil || !strings.Contains(err.Error(), "invalid icon URL") { - t.Errorf("validateChartIconURL(%s) to return \"invalid icon URL\", got no error", test) - } + t.Run(test, func(t *testing.T) { + badChart.Icon = test + err := validateChartIconURL(badChart) + require.ErrorContains(t, err, "invalid icon URL", "validateChartIconURL(%s) to return \"invalid icon URL\", got no error", test) + }) } for _, test := range successTest { badChart.Icon = test - err := validateChartSources(badChart) - if err != nil { - t.Errorf("validateChartIconURL(%s) to return no error, got %s", test, err.Error()) - } + err := validateChartIconURL(badChart) + assert.NoError(t, err, "validateChartIconURL(%s) to return no error", test) } } @@ -230,26 +209,11 @@ func TestV3Chartfile(t *testing.T) { msgs := linter.Messages expectedNumberOfErrorMessages := 6 - if len(msgs) != expectedNumberOfErrorMessages { - t.Errorf("Expected %d errors, got %d", expectedNumberOfErrorMessages, len(msgs)) - return - } - - if !strings.Contains(msgs[0].Err.Error(), "name is required") { - t.Errorf("Unexpected message 0: %s", msgs[0].Err) - } - - if !strings.Contains(msgs[1].Err.Error(), "apiVersion is required. The value must be \"v3\"") { - t.Errorf("Unexpected message 1: %s", msgs[1].Err) - } - - if !strings.Contains(msgs[2].Err.Error(), "version '0.0.0.0' is not a valid SemVer") { - t.Errorf("Unexpected message 2: %s", msgs[2].Err) - } - - if !strings.Contains(msgs[3].Err.Error(), "icon is recommended") { - t.Errorf("Unexpected message 3: %s", msgs[3].Err) - } + require.Lenf(t, msgs, expectedNumberOfErrorMessages, "Expected %d errors", expectedNumberOfErrorMessages) + require.ErrorContains(t, msgs[0].Err, "name is required", "Unexpected message 0") + require.ErrorContains(t, msgs[1].Err, "apiVersion is required. The value must be \"v3\"", "Unexpected message 1") + require.ErrorContains(t, msgs[2].Err, "version '0.0.0.0' is not a valid SemVer", "Unexpected message 2") + assert.ErrorContains(t, msgs[3].Err, "icon is recommended", "Unexpected message 3") }) t.Run("Chart.yaml validity issues due to type mismatch", func(t *testing.T) { @@ -258,21 +222,9 @@ func TestV3Chartfile(t *testing.T) { msgs := linter.Messages expectedNumberOfErrorMessages := 3 - if len(msgs) != expectedNumberOfErrorMessages { - t.Errorf("Expected %d errors, got %d", expectedNumberOfErrorMessages, len(msgs)) - return - } - - if !strings.Contains(msgs[0].Err.Error(), "version should be of type string") { - t.Errorf("Unexpected message 0: %s", msgs[0].Err) - } - - if !strings.Contains(msgs[1].Err.Error(), "version '7.2445e+06' is not a valid SemVer") { - t.Errorf("Unexpected message 1: %s", msgs[1].Err) - } - - if !strings.Contains(msgs[2].Err.Error(), "appVersion should be of type string") { - t.Errorf("Unexpected message 2: %s", msgs[2].Err) - } + require.Lenf(t, msgs, expectedNumberOfErrorMessages, "Expected %d errors", expectedNumberOfErrorMessages) + require.ErrorContains(t, msgs[0].Err, "version should be of type string", "Unexpected message 0") + require.ErrorContains(t, msgs[1].Err, "version '7.2445e+06' is not a valid SemVer", "Unexpected message 1") + assert.ErrorContains(t, msgs[2].Err, "appVersion should be of type string", "Unexpected message 2") }) } diff --git a/internal/chart/v3/lint/rules/deprecations_test.go b/internal/chart/v3/lint/rules/deprecations_test.go index 2b12a88f9..a255abf50 100644 --- a/internal/chart/v3/lint/rules/deprecations_test.go +++ b/internal/chart/v3/lint/rules/deprecations_test.go @@ -16,7 +16,12 @@ limitations under the License. package rules // import "helm.sh/helm/v4/internal/chart/v3/lint/rules" -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) func TestValidateNoDeprecations(t *testing.T) { deprecated := &k8sYamlStruct{ @@ -24,18 +29,14 @@ func TestValidateNoDeprecations(t *testing.T) { Kind: "Deployment", } err := validateNoDeprecations(deprecated, nil) - if err == nil { - t.Fatal("Expected deprecated extension to be flagged") - } - depErr := err.(deprecatedAPIError) - if depErr.Message == "" { - t.Fatalf("Expected error message to be non-blank: %v", err) - } + var depErr deprecatedAPIError + require.Error(t, err, "Expected deprecated extension to be flagged") + require.ErrorAs(t, err, &depErr, "Expected error to be of type deprecatedAPIError") + require.NotEmpty(t, depErr.Message, "Expected error message to be non-blank: %v", err) - if err := validateNoDeprecations(&k8sYamlStruct{ + err = validateNoDeprecations(&k8sYamlStruct{ APIVersion: "v1", Kind: "Pod", - }, nil); err != nil { - t.Error("Expected a v1 Pod to not be deprecated") - } + }, nil) + assert.NoError(t, err, "Expected a v1 Pod to not be deprecated") } diff --git a/internal/chart/v3/util/dependencies_test.go b/internal/chart/v3/util/dependencies_test.go index 0a0937e4a..f6ca06da3 100644 --- a/internal/chart/v3/util/dependencies_test.go +++ b/internal/chart/v3/util/dependencies_test.go @@ -21,6 +21,9 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/internal/chart/v3" "helm.sh/helm/v4/internal/chart/v3/loader" "helm.sh/helm/v4/pkg/chart/common" @@ -29,9 +32,7 @@ import ( func loadChart(t *testing.T, path string) *chart.Chart { t.Helper() c, err := loader.Load(path) - if err != nil { - t.Fatalf("failed to load testdata: %s", err) - } + require.NoError(t, err, "failed to load testdata") return c } @@ -42,19 +43,11 @@ func TestLoadDependency(t *testing.T) { } check := func(deps []*chart.Dependency) { - if len(deps) != 2 { - t.Errorf("expected 2 dependencies, got %d", len(deps)) - } + require.Len(t, deps, 2, "expected 2 dependencies") for i, tt := range tests { - if deps[i].Name != tt.Name { - t.Errorf("expected dependency named %q, got %q", tt.Name, deps[i].Name) - } - if deps[i].Version != tt.Version { - t.Errorf("expected dependency named %q to have version %q, got %q", tt.Name, tt.Version, deps[i].Version) - } - if deps[i].Repository != tt.Repository { - t.Errorf("expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, deps[i].Repository) - } + assert.Equal(t, tt.Name, deps[i].Name, "expected dependency named %q, got %q", tt.Name, deps[i].Name) + assert.Equal(t, tt.Version, deps[i].Version, "expected dependency named %q to have version %q, got %q", tt.Name, tt.Version, deps[i].Version) + assert.Equal(t, tt.Repository, deps[i].Repository, "expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, deps[i].Repository) } } c := loadChart(t, "testdata/frobnitz") @@ -117,18 +110,12 @@ func TestDependencyEnabled(t *testing.T) { for _, tc := range tests { c := loadChart(t, "testdata/subpop") t.Run(tc.name, func(t *testing.T) { - if err := processDependencyEnabled(c, tc.v, ""); err != nil { - t.Fatalf("error processing enabled dependencies %v", err) - } + require.NoErrorf(t, processDependencyEnabled(c, tc.v, ""), "error processing enabled dependencies") names := extractChartNames(c) - if len(names) != len(tc.e) { - t.Fatalf("slice lengths do not match got %v, expected %v", len(names), len(tc.e)) - } + require.Len(t, names, len(tc.e), "slice lengths do not match got %v, expected %v", len(names), len(tc.e)) for i := range names { - if names[i] != tc.e[i] { - t.Fatalf("slice values do not match got %v, expected %v", names, tc.e) - } + require.Equal(t, tc.e[i], names[i], "slice values do not match got %v, expected %v", names, tc.e) } }) } @@ -219,68 +206,44 @@ func TestProcessDependencyImportValues(t *testing.T) { e["SCBexported2A"] = "blaster" e["global.SC1exported2.all.SC1exported3"] = "SC1expstr" - if err := processDependencyImportValues(c, false); err != nil { - t.Fatalf("processing import values dependencies %v", err) - } + require.NoError(t, processDependencyImportValues(c, false), "processing import values dependencies") cc := common.Values(c.Values) for kk, vv := range e { pv, err := cc.PathValue(kk) - if err != nil { - t.Fatalf("retrieving import values table %v %v", kk, err) - } + require.NoError(t, err, "retrieving import values table %v", kk) switch pv := pv.(type) { case float64: - if s := strconv.FormatFloat(pv, 'f', -1, 64); s != vv { - t.Errorf("failed to match imported float value %v with expected %v for key %q", s, vv, kk) - } + s := strconv.FormatFloat(pv, 'f', -1, 64) + assert.Equalf(t, vv, s, "failed to match imported float value %v with expected %v for key %q", s, vv, kk) case bool: - if b := strconv.FormatBool(pv); b != vv { - t.Errorf("failed to match imported bool value %v with expected %v for key %q", b, vv, kk) - } + b := strconv.FormatBool(pv) + assert.Equalf(t, vv, b, "failed to match imported bool value %v with expected %v for key %q", b, vv, kk) default: - if pv != vv { - t.Errorf("failed to match imported string value %q with expected %q for key %q", pv, vv, kk) - } + assert.Equalf(t, vv, pv, "failed to match imported string value %q with expected %q for key %q", pv, vv, kk) } } // Since this was processed with coalescing there should be no null values. // Here we verify that. _, err := cc.PathValue("ensurenull") - if err == nil { - t.Error("expect nil value not found but found it") - } - switch xerr := err.(type) { - case common.ErrNoValue: - // We found what we expected - default: - t.Errorf("expected an ErrNoValue but got %q instead", xerr) - } + require.Error(t, err, "expect nil value not found but found it") + var xerr common.ErrNoValue + require.ErrorAs(t, err, &xerr, "expected an ErrNoValue") c = loadChart(t, "testdata/subpop") - if err := processDependencyImportValues(c, true); err != nil { - t.Fatalf("processing import values dependencies %v", err) - } + require.NoError(t, processDependencyImportValues(c, true), "processing import values dependencies") cc = common.Values(c.Values) val, err := cc.PathValue("ensurenull") - if err != nil { - t.Error("expect value but ensurenull was not found") - } - if val != nil { - t.Errorf("expect nil value but got %q instead", val) - } + require.NoError(t, err, "expect value but ensurenull was not found") + assert.Nil(t, val, "expect nil value but got %v instead", val) } func TestProcessDependencyImportValuesFromSharedDependencyToAliases(t *testing.T) { c := loadChart(t, "testdata/chart-with-import-from-aliased-dependencies") - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } - if err := processDependencyImportValues(c, true); err != nil { - t.Fatalf("processing import values dependencies %v", err) - } + require.NoError(t, processDependencyEnabled(c, c.Values, ""), "expected no errors") + require.NoError(t, processDependencyImportValues(c, true), "processing import values dependencies") e := make(map[string]string) e["foo-defaults.defaultValue"] = "42" @@ -295,12 +258,8 @@ func TestProcessDependencyImportValuesFromSharedDependencyToAliases(t *testing.T cValues := common.Values(c.Values) for kk, vv := range e { pv, err := cValues.PathValue(kk) - if err != nil { - t.Fatalf("retrieving import values table %v %v", kk, err) - } - if pv != vv { - t.Errorf("failed to match imported value %v with expected %v", pv, vv) - } + require.NoError(t, err, "retrieving import values table %v", kk) + assert.Equal(t, vv, pv, "failed to match imported value %v with expected %v", pv, vv) } } @@ -327,25 +286,18 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) { e["app2.service.port"] = "8080" e["app3.service.port"] = "9090" e["app4.service.port"] = "1234" - if err := processDependencyImportValues(c, true); err != nil { - t.Fatalf("processing import values dependencies %v", err) - } + require.NoError(t, processDependencyImportValues(c, true), "processing import values dependencies") cc := common.Values(c.Values) for kk, vv := range e { pv, err := cc.PathValue(kk) - if err != nil { - t.Fatalf("retrieving import values table %v %v", kk, err) - } + require.NoError(t, err, "retrieving import values table %v", kk) switch pv := pv.(type) { case float64: - if s := strconv.FormatFloat(pv, 'f', -1, 64); s != vv { - t.Errorf("failed to match imported float value %v with expected %v", s, vv) - } + s := strconv.FormatFloat(pv, 'f', -1, 64) + assert.Equalf(t, vv, s, "failed to match imported float value %v with expected %v", s, vv) default: - if pv != vv { - t.Errorf("failed to match imported string value %q with expected %q", pv, vv) - } + assert.Equalf(t, vv, pv, "failed to match imported string value %q with expected %q", pv, vv) } } } @@ -354,127 +306,72 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { c := loadChart(t, "testdata/import-values-from-enabled-subchart/parent-chart") nameOverride := "parent-chart-prod" - if err := processDependencyImportValues(c, true); err != nil { - t.Fatalf("processing import values dependencies %v", err) - } - - if len(c.Dependencies()) != 2 { - t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - } - - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } - - if len(c.Dependencies()) != 1 { - t.Fatal("expected no changes in dependencies") - } - - if len(c.Metadata.Dependencies) != 1 { - t.Fatalf("expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies)) - } + require.NoError(t, processDependencyImportValues(c, true), "processing import values dependencies") + require.Len(t, c.Dependencies(), 2, "expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) + require.NoError(t, processDependencyEnabled(c, c.Values, ""), "expected no errors") + require.Len(t, c.Dependencies(), 1, "expected no changes in dependencies") + require.Len(t, c.Metadata.Dependencies, 1, "expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies)) prodDependencyValues := c.Dependencies()[0].Values - if prodDependencyValues["nameOverride"] != nameOverride { - t.Fatalf("dependency chart name should be %s but got %s", nameOverride, prodDependencyValues["nameOverride"]) - } + require.Equal(t, nameOverride, prodDependencyValues["nameOverride"], "dependency chart name should be %s but got %s", nameOverride, prodDependencyValues["nameOverride"]) } func TestGetAliasDependency(t *testing.T) { c := loadChart(t, "testdata/frobnitz") req := c.Metadata.Dependencies - if len(req) == 0 { - t.Fatal("there are no dependencies to test") - } + require.NotEmpty(t, req, "there are no dependencies to test") // Success case aliasChart := getAliasDependency(c.Dependencies(), req[0]) - if aliasChart == nil { - t.Fatalf("failed to get dependency chart for alias %s", req[0].Name) - } + require.NotNil(t, aliasChart, "failed to get dependency chart for alias %s", req[0].Name) if req[0].Alias != "" { - if aliasChart.Name() != req[0].Alias { - t.Fatalf("dependency chart name should be %s but got %s", req[0].Alias, aliasChart.Name()) - } - } else if aliasChart.Name() != req[0].Name { - t.Fatalf("dependency chart name should be %s but got %s", req[0].Name, aliasChart.Name()) + require.Equal(t, req[0].Alias, aliasChart.Name(), "dependency chart name should be %s but got %s", req[0].Alias, aliasChart.Name()) + } else { + require.Equal(t, req[0].Name, aliasChart.Name(), "dependency chart name should be %s but got %s", req[0].Name, aliasChart.Name()) } - if req[0].Version != "" { - if !IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version) { - t.Fatal("dependency chart version is not in the compatible range") - } + require.True(t, IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version), "dependency chart version is not in the compatible range") } // Failure case req[0].Name = "something-else" - if aliasChart := getAliasDependency(c.Dependencies(), req[0]); aliasChart != nil { - t.Fatalf("expected no chart but got %s", aliasChart.Name()) - } + require.Nil(t, getAliasDependency(c.Dependencies(), req[0]), "expected no chart") req[0].Version = "something else which is not in the compatible range" - if IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version) { - t.Fatal("dependency chart version outside the compatible range should not be considered compatible") - } + require.False(t, IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version), "dependency chart version outside the compatible range should not be considered compatible") } func TestDependentChartAliases(t *testing.T) { c := loadChart(t, "testdata/dependent-chart-alias") req := c.Metadata.Dependencies - if len(c.Dependencies()) != 2 { - t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - } - - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } - - if len(c.Dependencies()) != 3 { - t.Fatal("expected alias dependencies to be added") - } - - if len(c.Dependencies()) != len(c.Metadata.Dependencies) { - t.Fatalf("expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies())) - } + require.Len(t, c.Dependencies(), 2, "expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) + require.NoError(t, processDependencyEnabled(c, c.Values, ""), "expected no errors") + require.Len(t, c.Dependencies(), 3, "expected alias dependencies to be added") + require.Len(t, c.Dependencies(), len(c.Metadata.Dependencies), "expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies())) aliasChart := getAliasDependency(c.Dependencies(), req[2]) - if aliasChart == nil { - t.Fatalf("failed to get dependency chart for alias %s", req[2].Name) - } - if aliasChart.Parent() != c { - t.Fatalf("dependency chart has wrong parent, expected %s but got %s", c.Name(), aliasChart.Parent().Name()) - } + require.NotNil(t, aliasChart, "failed to get dependency chart for alias %s", req[2].Name) + require.Equal(t, c, aliasChart.Parent(), "dependency chart has wrong parent, expected %s but got %s", c.Name(), aliasChart.Parent().Name()) if req[2].Alias != "" { - if aliasChart.Name() != req[2].Alias { - t.Fatalf("dependency chart name should be %s but got %s", req[2].Alias, aliasChart.Name()) - } - } else if aliasChart.Name() != req[2].Name { - t.Fatalf("dependency chart name should be %s but got %s", req[2].Name, aliasChart.Name()) + require.Equal(t, req[2].Alias, aliasChart.Name(), "dependency chart name should be %s but got %s", req[2].Alias, aliasChart.Name()) + } else { + require.Equal(t, req[2].Name, aliasChart.Name(), "dependency chart name should be %s but got %s", req[2].Name, aliasChart.Name()) } - req[2].Name = "dummy-name" - if aliasChart := getAliasDependency(c.Dependencies(), req[2]); aliasChart != nil { - t.Fatalf("expected no chart but got %s", aliasChart.Name()) - } + require.Nil(t, getAliasDependency(c.Dependencies(), req[2]), "expected no chart") } func TestDependentChartWithSubChartsAbsentInDependency(t *testing.T) { c := loadChart(t, "testdata/dependent-chart-no-requirements-yaml") - if len(c.Dependencies()) != 2 { - t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - } - - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } + require.Len(t, c.Dependencies(), 2, "expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - if len(c.Dependencies()) != 2 { - t.Fatal("expected no changes in dependencies") - } + err := processDependencyEnabled(c, c.Values, "") + require.NoError(t, err, "expected no errors") + require.Len(t, c.Dependencies(), 2, "expected no changes in dependencies") } func TestDependentChartWithSubChartsHelmignore(t *testing.T) { @@ -484,86 +381,53 @@ func TestDependentChartWithSubChartsHelmignore(t *testing.T) { func TestDependentChartsWithSubChartsSymlink(t *testing.T) { joonix := filepath.Join("testdata", "joonix") - if err := os.Symlink(filepath.Join("..", "..", "frobnitz"), filepath.Join(joonix, "charts", "frobnitz")); err != nil { - t.Fatal(err) - } + err := os.Symlink(filepath.Join("..", "..", "frobnitz"), filepath.Join(joonix, "charts", "frobnitz")) + require.NoError(t, err, "failed to create symlink") defer os.RemoveAll(filepath.Join(joonix, "charts", "frobnitz")) c := loadChart(t, joonix) - if c.Name() != "joonix" { - t.Fatalf("unexpected chart name: %s", c.Name()) - } - if n := len(c.Dependencies()); n != 1 { - t.Fatalf("expected 1 dependency for this chart, but got %d", n) - } + require.Equal(t, "joonix", c.Name(), "unexpected chart name: %s", c.Name()) + require.Len(t, c.Dependencies(), 1, "expected 1 dependency for this chart") } func TestDependentChartsWithSubchartsAllSpecifiedInDependency(t *testing.T) { c := loadChart(t, "testdata/dependent-chart-with-all-in-requirements-yaml") - if len(c.Dependencies()) != 2 { - t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - } + require.Len(t, c.Dependencies(), 2, "expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } + err := processDependencyEnabled(c, c.Values, "") + require.NoError(t, err, "expected no errors") - if len(c.Dependencies()) != 2 { - t.Fatal("expected no changes in dependencies") - } + require.Len(t, c.Dependencies(), 2, "expected no changes in dependencies") - if len(c.Dependencies()) != len(c.Metadata.Dependencies) { - t.Fatalf("expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies())) - } + require.Len(t, c.Dependencies(), len(c.Metadata.Dependencies), "expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies())) } func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) { c := loadChart(t, "testdata/dependent-chart-with-mixed-requirements-yaml") - if len(c.Dependencies()) != 2 { - t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) - } - - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } - - if len(c.Dependencies()) != 2 { - t.Fatal("expected no changes in dependencies") - } - - if len(c.Metadata.Dependencies) != 1 { - t.Fatalf("expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies)) - } + require.Len(t, c.Dependencies(), 2, "expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) + require.NoError(t, processDependencyEnabled(c, c.Values, ""), "expected no errors") + require.Len(t, c.Dependencies(), 2, "expected no changes in dependencies") + require.Len(t, c.Metadata.Dependencies, 1, "expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies)) } func validateDependencyTree(t *testing.T, c *chart.Chart) { t.Helper() for _, dependency := range c.Dependencies() { - if dependency.Parent() != c { - if dependency.Parent() != c { - t.Fatalf("dependency chart %s has wrong parent, expected %s but got %s", dependency.Name(), c.Name(), dependency.Parent().Name()) - } - } - // recurse entire tree - validateDependencyTree(t, dependency) + t.Run(dependency.Name(), func(t *testing.T) { + require.Equal(t, c, dependency.Parent(), "dependency chart %s has wrong parent, expected %s but got %s", dependency.Name(), c.Name(), dependency.Parent().Name()) + // recurse entire tree + validateDependencyTree(t, dependency) + }) } } func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *testing.T) { c := loadChart(t, "testdata/chart-with-dependency-aliased-twice") - if len(c.Dependencies()) != 1 { - t.Fatalf("expected one dependency for this chart, but got %d", len(c.Dependencies())) - } - - if err := processDependencyEnabled(c, c.Values, ""); err != nil { - t.Fatalf("expected no errors but got %q", err) - } - - if len(c.Dependencies()) != 2 { - t.Fatal("expected two dependencies after processing aliases") - } + require.Len(t, c.Dependencies(), 1, "expected one dependency for this chart, but got %d", len(c.Dependencies())) + require.NoError(t, processDependencyEnabled(c, c.Values, ""), "expected no errors") + require.Len(t, c.Dependencies(), 2, "expected two dependencies after processing aliases") validateDependencyTree(t, c) } diff --git a/internal/plugin/metadata_test.go b/internal/plugin/metadata_test.go index 6113f30c7..b4994cf68 100644 --- a/internal/plugin/metadata_test.go +++ b/internal/plugin/metadata_test.go @@ -16,10 +16,12 @@ limitations under the License. package plugin import ( + "strconv" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidatePluginData(t *testing.T) { @@ -61,15 +63,15 @@ func TestValidatePluginData(t *testing.T) { {true, mockNoCommand, ""}, // Test no command metadata works {true, mockLegacyCommand, ""}, // Test legacy command metadata works } { - err := item.plug.Metadata().Validate() - if item.pass && err != nil { - t.Errorf("failed to validate case %d: %s", i, err) - } else if !item.pass && err == nil { - t.Errorf("expected case %d to fail", i) - } - if !item.pass && !strings.Contains(err.Error(), item.errString) { - t.Errorf("index [%d]: expected error to contain: %s, but got: %s", i, item.errString, err.Error()) - } + t.Run(strconv.Itoa(i), func(t *testing.T) { + err := item.plug.Metadata().Validate() + if item.pass { + require.NoError(t, err, "failed to validate case %d: %s", i, err) + } else { + require.Error(t, err, "expected case %d to fail", i) + assert.ErrorContains(t, err, item.errString, "expected case %d error to contain %q", i, item.errString) + } + }) } } @@ -122,9 +124,7 @@ func TestMetadataValidateMultipleErrors(t *testing.T) { } err := metadata.Validate() - if err == nil { - t.Fatal("expected validation to fail with multiple errors") - } + require.Error(t, err, "expected validation to fail with multiple errors") errStr := err.Error() @@ -139,9 +139,7 @@ func TestMetadataValidateMultipleErrors(t *testing.T) { } for _, expectedErr := range expectedErrors { - if !strings.Contains(errStr, expectedErr) { - t.Errorf("expected error to contain %q, but got: %v", expectedErr, errStr) - } + assert.Contains(t, errStr, expectedErr, "expected error to contain %q, but got: %v", expectedErr, errStr) } // Verify that the error contains the correct number of error messages @@ -152,7 +150,5 @@ func TestMetadataValidateMultipleErrors(t *testing.T) { } } - if errorCount < len(expectedErrors) { - t.Errorf("expected %d errors, but only found %d in: %v", len(expectedErrors), errorCount, errStr) - } + assert.GreaterOrEqual(t, errorCount, len(expectedErrors), "expected %d errors, but only found %d in: %v", len(expectedErrors), errorCount, errStr) } diff --git a/internal/plugin/verify_test.go b/internal/plugin/verify_test.go index 2828d30b6..5721e0414 100644 --- a/internal/plugin/verify_test.go +++ b/internal/plugin/verify_test.go @@ -20,6 +20,9 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/provenance" ) @@ -41,20 +44,16 @@ func TestVerifyPlugin(t *testing.T) { // Create plugin directory pluginDir := filepath.Join(tempDir, "verify-test-plugin") - if err := os.MkdirAll(pluginDir, 0o755); err != nil { - t.Fatal(err) - } + err := os.MkdirAll(pluginDir, 0o755) + require.NoError(t, err) - if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0o644); err != nil { - t.Fatal(err) - } + err = os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0o644) + require.NoError(t, err) // Create tarball tarballPath := filepath.Join(tempDir, "verify-test-plugin.tar.gz") tarFile, err := os.Create(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) if err := CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { tarFile.Close() @@ -64,61 +63,41 @@ func TestVerifyPlugin(t *testing.T) { // Sign the plugin with source directory signer, err := provenance.NewFromKeyring(testKeyFile, "helm-test") - if err != nil { - t.Fatal(err) - } - if err := signer.DecryptKey(func(_ string) ([]byte, error) { + require.NoError(t, err) + err = signer.DecryptKey(func(_ string) ([]byte, error) { return []byte(""), nil - }); err != nil { - t.Fatal(err) - } + }) + require.NoError(t, err) // Read the tarball data tarballData, err := os.ReadFile(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) sig, err := SignPlugin(tarballData, filepath.Base(tarballPath), signer) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // Write the signature to .prov file provFile := tarballPath + ".prov" - if err := os.WriteFile(provFile, []byte(sig), 0o644); err != nil { - t.Fatal(err) - } + err = os.WriteFile(provFile, []byte(sig), 0o644) + require.NoError(t, err) // Read the files for verification archiveData, err := os.ReadFile(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) provData, err := os.ReadFile(provFile) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // Now verify the plugin verification, err := VerifyPlugin(archiveData, provData, filepath.Base(tarballPath), testPubFile) - if err != nil { - t.Fatalf("Failed to verify plugin: %v", err) - } + require.NoError(t, err, "Failed to verify plugin") // Check verification results - if verification.SignedBy == nil { - t.Error("SignedBy is nil") - } + assert.NotNil(t, verification.SignedBy, "SignedBy is nil") - if verification.FileName != "verify-test-plugin.tar.gz" { - t.Errorf("Expected filename 'verify-test-plugin.tar.gz', got %s", verification.FileName) - } + assert.Equal(t, "verify-test-plugin.tar.gz", verification.FileName, "Expected filename 'verify-test-plugin.tar.gz', got %s", verification.FileName) - if verification.FileHash == "" { - t.Error("FileHash is empty") - } + assert.NotEmpty(t, verification.FileHash, "FileHash is empty") } func TestVerifyPluginBadSignature(t *testing.T) { @@ -126,19 +105,15 @@ func TestVerifyPluginBadSignature(t *testing.T) { // Create a plugin tarball pluginDir := filepath.Join(tempDir, "bad-plugin") - if err := os.MkdirAll(pluginDir, 0o755); err != nil { - t.Fatal(err) - } + err := os.MkdirAll(pluginDir, 0o755) + require.NoError(t, err) - if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0o644); err != nil { - t.Fatal(err) - } + err = os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0o644) + require.NoError(t, err) tarballPath := filepath.Join(tempDir, "bad-plugin.tar.gz") tarFile, err := os.Create(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) if err := CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { tarFile.Close() @@ -158,26 +133,19 @@ InvalidSignatureData -----END PGP SIGNATURE-----` provFile := tarballPath + ".prov" - if err := os.WriteFile(provFile, []byte(badSig), 0o644); err != nil { - t.Fatal(err) - } + err = os.WriteFile(provFile, []byte(badSig), 0o644) + require.NoError(t, err) // Read the files archiveData, err := os.ReadFile(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) provData, err := os.ReadFile(provFile) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // Try to verify - should fail _, err = VerifyPlugin(archiveData, provData, filepath.Base(tarballPath), testPubFile) - if err == nil { - t.Error("Expected verification to fail with bad signature") - } + assert.Error(t, err, "Expected verification to fail with bad signature") } func TestVerifyPluginMissingProvenance(t *testing.T) { @@ -185,21 +153,16 @@ func TestVerifyPluginMissingProvenance(t *testing.T) { tarballPath := filepath.Join(tempDir, "no-prov.tar.gz") // Create a minimal tarball - if err := os.WriteFile(tarballPath, []byte("dummy"), 0o644); err != nil { - t.Fatal(err) - } + err := os.WriteFile(tarballPath, []byte("dummy"), 0o644) + require.NoError(t, err) // Read the tarball data archiveData, err := os.ReadFile(tarballPath) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) // Try to verify with empty provenance data _, err = VerifyPlugin(archiveData, nil, filepath.Base(tarballPath), testPubFile) - if err == nil { - t.Error("Expected verification to fail with empty provenance data") - } + assert.Error(t, err, "Expected verification to fail with empty provenance data") } func TestVerifyPluginMalformedData(t *testing.T) { @@ -208,7 +171,5 @@ func TestVerifyPluginMalformedData(t *testing.T) { provData := []byte("fake provenance") _, err := VerifyPlugin(malformedData, provData, "malformed.tar.gz", testPubFile) - if err == nil { - t.Error("Expected malformed data verification to fail, but it succeeded") - } + assert.Error(t, err, "Expected malformed data verification to fail, but it succeeded") }