Skip to content

Commit ebef191

Browse files
committed
fix(acl): Improve error handling -- apply suggestions
Signed-off-by: Viacheslav Vasilyev <avoidik@gmail.com>
1 parent 8715866 commit ebef191

1 file changed

Lines changed: 17 additions & 27 deletions

File tree

internal/clients/kafka/acl/acl_test.go

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"time"
99

1010
"github.com/google/go-cmp/cmp"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1113
"github.com/twmb/franz-go/pkg/kadm"
1214
"github.com/twmb/franz-go/pkg/kerr"
1315
"k8s.io/apimachinery/pkg/util/json"
@@ -504,76 +506,64 @@ func TestList(t *testing.T) {
504506
// --- Unit tests for broker-level error propagation (no real Kafka needed) ---
505507

506508
func TestCreateBrokerError(t *testing.T) {
509+
t.Parallel()
507510
cl := &fakeACLAdmin{
508511
createResults: kadm.CreateACLsResults{
509512
{Principal: "User:alice", Err: kerr.ClusterAuthorizationFailed},
510513
},
511514
}
512515
err := Create(context.Background(), cl, &baseACL)
513-
if err == nil {
514-
t.Fatal("Create() expected error for broker-level Err, got nil")
515-
}
516+
require.Error(t, err)
516517
}
517518

518519
func TestCreateEmptyResponse(t *testing.T) {
520+
t.Parallel()
519521
cl := &fakeACLAdmin{createResults: kadm.CreateACLsResults{}}
520522
err := Create(context.Background(), cl, &baseACL)
521-
if err == nil {
522-
t.Fatal("Create() expected error for empty response, got nil")
523-
}
523+
require.Error(t, err)
524524
}
525525

526526
func TestListBrokerError(t *testing.T) {
527+
t.Parallel()
527528
cl := &fakeACLAdmin{
528529
describeResults: kadm.DescribeACLsResults{
529530
{Err: kerr.ClusterAuthorizationFailed},
530531
},
531532
}
532533
got, err := List(context.Background(), cl, &baseACL)
533-
if err == nil {
534-
t.Fatal("List() expected error for broker-level Err, got nil")
535-
}
536-
if got != nil {
537-
t.Errorf("List() expected nil result on error, got %v", got)
538-
}
534+
require.Error(t, err)
535+
assert.Nil(t, got)
539536
}
540537

541538
func TestListNotFound(t *testing.T) {
539+
t.Parallel()
542540
cl := &fakeACLAdmin{
543541
describeResults: kadm.DescribeACLsResults{
544542
{Described: kadm.DescribedACLs{}},
545543
},
546544
}
547545
got, err := List(context.Background(), cl, &baseACL)
548-
if err != nil {
549-
t.Fatalf("List() unexpected error: %v", err)
550-
}
551-
if got != nil {
552-
t.Errorf("List() expected nil for no matching ACL, got %v", got)
553-
}
546+
require.NoError(t, err)
547+
assert.Nil(t, got)
554548
}
555549

556550
func TestListEmptyResponse(t *testing.T) {
551+
t.Parallel()
557552
cl := &fakeACLAdmin{describeResults: kadm.DescribeACLsResults{}}
558553
got, err := List(context.Background(), cl, &baseACL)
559-
if err != nil {
560-
t.Fatalf("List() unexpected error: %v", err)
561-
}
562-
if got != nil {
563-
t.Errorf("List() expected nil for empty response, got %v", got)
564-
}
554+
require.NoError(t, err)
555+
assert.Nil(t, got)
565556
}
566557

567558
func TestDeleteBrokerError(t *testing.T) {
559+
t.Parallel()
568560
cl := &fakeACLAdmin{
569561
deleteResults: kadm.DeleteACLsResults{
570562
{Err: kerr.ClusterAuthorizationFailed},
571563
},
572564
}
573565
err := Delete(context.Background(), cl, &baseACL)
574-
if err == nil {
575-
t.Fatal("Delete() expected error for broker-level Err, got nil")
576-
}
566+
require.Error(t, err)
577567
}
578568

579569
// TestListAtProviderNotFound verifies that List returns nil when the ACL does not exist.

0 commit comments

Comments
 (0)