Merge pull request #32254 from gjenkins8/refactor/testify-pkg-registry

refactor(pkg/registry): convert tests to testify assert/require

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)
-
+		assert.Equal(t, tt.expect, result, tt.name)
-		if !reflect.DeepEqual(tt.expect, result) {
-			t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, result)
-		}
 	}
 }
 
@@ -218,10 +216,7 @@ func TestGenerateOCIAnnotations(t *testing.T) {
 
 	for _, tt := range tests {
 		result := generateOCIAnnotations(tt.chart, nowString)
-
+		assert.Equal(t, tt.expect, result, tt.name)
-		if !reflect.DeepEqual(tt.expect, result) {
-			t.Errorf("%s: expected map %v, got %v", tt.name, tt.expect, result)
-		}
 	}
 }
 
@@ -237,29 +232,23 @@ func TestGenerateOCICreatedAnnotations(t *testing.T) {
 	result := generateOCIAnnotations(testChart, nowTimeString)
 
 	// Check that created annotation exists
-	if _, ok := result[ocispec.AnnotationCreated]; !ok {
+	_, ok := result[ocispec.AnnotationCreated]
-		t.Errorf("%s annotation not created", 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 {
+	_, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated])
-		t.Errorf("%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, 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 {
+	_, ok = result[ocispec.AnnotationCreated]
-		t.Errorf("%s annotation not created", ocispec.AnnotationCreated)
+	require.True(t, ok, "%s annotation not created", ocispec.AnnotationCreated)
-	}
 
-	if createdTimeAnnotation, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated]); err != nil {
+	createdTimeAnnotation, err := time.Parse(time.RFC3339, result[ocispec.AnnotationCreated])
-		t.Errorf("%s annotation with value '%s' not in RFC3339 format", ocispec.AnnotationCreated, 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
+	// Verify creation annotation after (or equals) time test began
-		if !nowTime.Before(createdTimeAnnotation) {
+	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)
-			t.Errorf("%s annotation with value '%s' not configured properly. Annotation value is not after %s", ocispec.AnnotationCreated, result[ocispec.AnnotationCreated], nowTimeString)
-		}
-	}
 }

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 {
+	require.NoError(t, err, "NewClient error")
-		t.Fatalf("NewClient error: %v", err)
-	}
 
-	if c.authorizer == nil || c.authorizer.ForceAttemptOAuth2 {
+	require.NotNil(t, c.authorizer)
-		t.Fatal("expected ForceAttemptOAuth2 default to be false")
+	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 {
+	require.NoError(t, c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")), "Login error")
-		t.Fatalf("Login error: %v", err)
-	}
 
-	if c.authorizer.ForceAttemptOAuth2 {
+	assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after successful Login")
-		t.Error("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 {
+	require.NoError(t, err, "NewClient error")
-		t.Fatalf("NewClient error: %v", err)
-	}
 
 	// Invoke Login, expect an error but ForceAttemptOAuth2 must end false
 	_ = c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p"))
 
-	if c.authorizer.ForceAttemptOAuth2 {
+	assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after failed Login")
-		t.Error("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)
+			assert.Equal(t, tt.wantWarn, warnIfHostHasPath(tt.host))
-			if got != tt.wantWarn {
-				t.Errorf("warnIfHostHasPath(%q) = %v, want %v", tt.host, got, tt.wantWarn)
-			}
 		})
 	}
 }

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 {
+				assert.Error(t, err, "expected error but got none")
-					t.Error("expected error but got none")
-				}
-				return
-			}
-
-			if err != nil {
-				t.Errorf("unexpected error: %v", err)
 				return
 			}
 
-			if pluginName != tt.expected {
+			require.NoError(t, err)
-				t.Errorf("expected plugin name %q, got %q", tt.expected, pluginName)
+			assert.Equal(t, tt.expected, pluginName)
-			}
 		})
 	}
 }

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 {
+	assert.Equal(t, registry, actual.orasReference.Registry, "Oras reference registry")
-		t.Errorf("Oras reference registry expected %v actual %v", registry, actual.Registry)
+	assert.Equal(t, repository, actual.orasReference.Repository, "Oras reference repository")
-	}
+	assert.Equal(t, tag, actual.orasReference.Reference, "Oras reference reference")
-	if repository != actual.orasReference.Repository {
+	assert.Equal(t, registry, actual.Registry, "Registry")
-		t.Errorf("Oras reference repository expected %v actual %v", repository, actual.Repository)
+	assert.Equal(t, repository, actual.Repository, "Repository")
-	}
+	assert.Equal(t, tag, actual.Tag, "Tag")
-	if tag != actual.orasReference.Reference {
+	assert.Equal(t, digest, actual.Digest, "Digest")
-		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)
-	}
 	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 {
+	assert.Equal(t, expectedString, actual.String(), "String")
-		t.Errorf("String expected %s actual %s", expectedString, actual.String())
-	}
 }
 
 func TestNewReference(t *testing.T) {
 	actual, err := newReference("registry.example.com/repository:1.0@sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888")
-	if err != nil {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", 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 {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", err)
-	}
 	verify(t, actual, "registry.example.com", "repository", "1.0", "sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888")
 
 	actual, err = newReference("a/b:1@c")
-	if err != nil {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", err)
-	}
 	verify(t, actual, "a", "b", "1", "c")
 
 	actual, err = newReference("a/b:@")
-	if err != nil {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", err)
-	}
 	verify(t, actual, "a", "b", "", "")
 
 	actual, err = newReference("registry.example.com/repository:1.0+001")
-	if err != nil {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", err)
-	}
 	verify(t, actual, "registry.example.com", "repository", "1.0_001", "")
 
 	actual, err = newReference("thing:1.0")
-	if err == nil {
+	assert.Error(t, err)
-		t.Errorf("Expect error error %v", err)
-	}
 	verify(t, actual, "", "", "", "")
 
 	actual, err = newReference("registry.example.com/the/repository@sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888")
-	if err != nil {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error %v", err)
-	}
 	verify(t, actual, "registry.example.com", "the/repository", "", "sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888")
 }

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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.2.3", got, "expected exact match")
-	}
-	if got != "1.2.3" {
-		t.Fatalf("expected exact match '1.2.3', got %q", got)
-	}
 }
 
 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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
-	}
 	// Should pick the first valid semver tag in order, which is 0.9.0
-	if got != "0.9.0" {
+	require.Equal(t, "0.9.0", got)
-		t.Fatalf("expected '0.9.0', got %q", 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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.0.0", got, "first match in order")
-	}
-	if got != "1.0.0" { // first match in order
-		t.Fatalf("expected '1.0.0', got %q", got)
-	}
 
 	// Compound range
 	got, err = GetTagMatchingVersionOrConstraint(tags, ">=1.0.0 <2.0.0")
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.0.0", got)
-	}
-	if got != "1.0.0" {
-		t.Fatalf("expected '1.0.0', got %q", got)
-	}
 }
 
 func TestGetTagMatchingVersionOrConstraint_InvalidConstraint(t *testing.T) {
 	tags := []string{"1.0.0"}
 	_, err := GetTagMatchingVersionOrConstraint(tags, ">a1")
-	if err == nil {
+	require.Error(t, err, "expected error for invalid constraint")
-		t.Fatal("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 {
+	require.Error(t, err, "expected error when no tags match")
-		t.Fatal("expected error when no tags match")
+	require.Contains(t, err.Error(), ">=1.0.0", "expected error to contain version string")
-	}
-	if !strings.Contains(err.Error(), ">=1.0.0") {
-		t.Fatalf("expected error to contain version string, got: %v", err)
-	}
 }
 
 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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.0.0", got)
-	}
-	if got != "1.0.0" {
-		t.Fatalf("expected '1.0.0', got %q", 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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.3.0", got, "first satisfying tag")
-	}
-	if got != "1.3.0" {
-		t.Fatalf("expected '1.3.0' (first satisfying tag), got %q", got)
-	}
 }
 
 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 {
+	require.NoError(t, err)
-		t.Fatalf("unexpected error: %v", err)
+	require.Equal(t, "1.2.3", got, "expected exact match")
-	}
-	if got != "1.2.3" {
-		t.Fatalf("expected exact match '1.2.3', got %q", got)
-	}
 }

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 {
+			assert.Equal(t, tt.want, isPrintableContentType(tt.contentType))
-				t.Errorf("isPrintableContentType() = %v, want %v", got, tt.want)
-			}
 		})
 	}
 }
@@ -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 {
+			assert.Equal(t, tt.want, logResponseBody(tt.resp))
-				t.Errorf("logResponseBody() = %v, want %v", got, tt.want)
-			}
 			// validate the response body
 			if tt.resp.Body != nil {
 				readBytes, err := io.ReadAll(tt.resp.Body)
-				if err != nil {
+				require.NoError(t, err, "failed to read body after logResponseBody()")
-					t.Errorf("failed to read body after logResponseBody(), err= %v", err)
+				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()")
-				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)
-				}
 			}
 		})
 	}
@@ -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 {
+			assert.Equal(t, tt.want, logResponseBody(tt.resp))
-				t.Errorf("logResponseBody() = %v, want %v", got, tt.want)
+			assert.NoError(t, tt.resp.Body.Close(), "failed to close body after logResponseBody()")
-			}
-			if closeErr := tt.resp.Body.Close(); closeErr != nil {
-				t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr)
-			}
 		})
 	}
 }
@@ -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 {
+			assert.Equal(t, tt.want, containsCredentials(tt.body))
-				t.Errorf("containsCredentials() = %v, want %v", got, tt.want)
-			}
 		})
 	}
 }