Skip to content

Commit

Permalink
test(pubsublite): simplify comparisons in integration tests (#4763)
Browse files Browse the repository at this point in the history 
Replicates the approach taken in sample tests and converts project ids to project numbers. This allows us to just directly compare resource paths for equality.
  • Loading branch information
@tmdiep
tmdiep committed Sep 30, 2021
1 parent 1aef33e commit 320f497
Showing 1 changed file with 38 additions and 82 deletions.
120 changes: 38 additions & 82 deletions pubsublite/integration_test.go
View file
Expand Up @@ -15,15 +15,15 @@ package pubsublite

import (
"context"
"fmt"
"testing"
"time"

"cloud.google.com/go/internal/testutil"
"cloud.google.com/go/internal/uid"
"cloud.google.com/go/pubsublite/internal/test"
"cloud.google.com/go/pubsublite/internal/wire"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"google.golang.org/api/cloudresourcemanager/v1"
"google.golang.org/api/iterator"
"google.golang.org/api/option"

Expand All @@ -34,77 +34,33 @@ const gibi = 1 << 30

var (
resourceIDs = uid.NewSpace("go-admin-test", nil)

// The server returns topic and subscription configs with project numbers in
// resource paths. These will not match a project id specified for integration
// tests.
pathCmpOptions = []cmp.Option{
cmpopts.IgnoreFields(wire.TopicPath{}, "Project"),
cmpopts.IgnoreFields(wire.SubscriptionPath{}, "Project"),
cmpopts.IgnoreFields(wire.ReservationPath{}, "Project"),
}
configCmpOptions = []cmp.Option{
cmp.Comparer(func(t1, t2 *TopicConfig) bool {
return cmp.Equal(t1, t2, cmpopts.IgnoreFields(TopicConfig{}, "Name", "ThroughputReservation")) &&
TopicPathsEqual(t1.Name, t2.Name) && ReservationPathsEqual(t1.ThroughputReservation, t2.ThroughputReservation, true)
}),
cmp.Comparer(func(s1, s2 *SubscriptionConfig) bool {
return cmp.Equal(s1, s2, cmpopts.IgnoreFields(SubscriptionConfig{}, "Name", "Topic")) &&
TopicPathsEqual(s1.Topic, s2.Topic) && SubscriptionPathsEqual(s1.Name, s2.Name)
}),
cmp.Comparer(func(r1, r2 *ReservationConfig) bool {
return cmp.Equal(r1, r2, cmpopts.IgnoreFields(ReservationConfig{}, "Name")) &&
ReservationPathsEqual(r1.Name, r2.Name, false)
}),
}
)

func TopicPathsEqual(topic1, topic2 string) bool {
tp1, err := wire.ParseTopicPath(topic1)
if err != nil {
return false
}
tp2, err := wire.ParseTopicPath(topic2)
if err != nil {
return false
}
return cmp.Equal(tp1, tp2, pathCmpOptions...)
}

func SubscriptionPathsEqual(subscription1, subscription2 string) bool {
sp1, err := wire.ParseSubscriptionPath(subscription1)
if err != nil {
return false
func initIntegrationTest(t *testing.T) {
if testing.Short() {
t.Skip("Integration tests skipped in short mode")
}
sp2, err := wire.ParseSubscriptionPath(subscription2)
if err != nil {
return false
if testutil.ProjID() == "" {
t.Skip("Integration tests skipped. See CONTRIBUTING.md for details")
}
return cmp.Equal(sp1, sp2, pathCmpOptions...)
}

func ReservationPathsEqual(reservation1, reservation2 string, allowEmpty bool) bool {
if allowEmpty && len(reservation1)+len(reservation2) == 0 {
return true
func projectNumber(t *testing.T) string {
projID := testutil.ProjID()
if projID == "" {
return ""
}
rp1, err := wire.ParseReservationPath(reservation1)
// Pub/Sub Lite returns project numbers in resource paths, so we need to
// convert from project id to numbers for simpler comparisons in tests.
crm, err := cloudresourcemanager.NewService(context.Background())
if err != nil {
return false
t.Fatalf("Failed to create cloudresourcemanager: %v", err)
}
rp2, err := wire.ParseReservationPath(reservation2)
project, err := crm.Projects.Get(projID).Do()
if err != nil {
return false
}
return cmp.Equal(rp1, rp2, pathCmpOptions...)
}

func initIntegrationTest(t *testing.T) {
if testing.Short() {
t.Skip("Integration tests skipped in short mode")
}
if testutil.ProjID() == "" {
t.Skip("Integration tests skipped. See CONTRIBUTING.md for details")
t.Fatalf("Failed to retrieve project %q: %v", projID, err)
}
return fmt.Sprintf("%d", project.ProjectNumber)
}

func withGRPCHeadersAssertion(t *testing.T, opts ...option.ClientOption) []option.ClientOption {
Expand Down Expand Up @@ -163,7 +119,7 @@ func validateNewSeekOperation(t *testing.T, subscription string, seekOp *SeekSub
t.Errorf("Operation.Metadata() got err: %v", err)
return
}
if got, want := m.Target, subscription; !SubscriptionPathsEqual(got, want) {
if got, want := m.Target, subscription; got != want {
t.Errorf("Metadata.Target got: %v, want: %v", got, want)
}
if len(m.Verb) == 0 {
Expand All @@ -178,7 +134,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
initIntegrationTest(t)

ctx := context.Background()
proj := testutil.ProjID()
proj := projectNumber(t)
zone := test.RandomLiteZone()
region, _ := wire.LocationToRegion(zone)
resourceID := resourceIDs.New()
Expand All @@ -203,13 +159,13 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
t.Fatalf("Failed to create reservation: %v", err)
}
defer cleanUpReservation(ctx, t, admin, reservationPath)
if diff := testutil.Diff(gotResConfig, newResConfig, configCmpOptions...); diff != "" {
if diff := testutil.Diff(gotResConfig, newResConfig); diff != "" {
t.Errorf("CreateReservation() got: -, want: +\n%s", diff)
}

if gotResConfig, err := admin.Reservation(ctx, reservationPath); err != nil {
t.Errorf("Failed to get reservation: %v", err)
} else if diff := testutil.Diff(gotResConfig, newResConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotResConfig, newResConfig); diff != "" {
t.Errorf("Reservation() got: -, want: +\n%s", diff)
}

Expand All @@ -220,14 +176,14 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
if err == iterator.Done {
break
}
if ReservationPathsEqual(res.Name, reservationPath, false) {
if res.Name == reservationPath {
foundRes = res
break
}
}
if foundRes == nil {
t.Error("Reservations() did not return reservation config")
} else if diff := testutil.Diff(foundRes, newResConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(foundRes, newResConfig); diff != "" {
t.Errorf("Reservations() found config: -, want: +\n%s", diff)
}

Expand All @@ -241,7 +197,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
}
if gotResConfig, err := admin.UpdateReservation(ctx, resUpdate); err != nil {
t.Errorf("Failed to update reservation: %v", err)
} else if diff := testutil.Diff(gotResConfig, wantUpdatedResConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotResConfig, wantUpdatedResConfig); diff != "" {
t.Errorf("UpdateReservation() got: -, want: +\n%s", diff)
}

Expand All @@ -261,13 +217,13 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
t.Fatalf("Failed to create topic: %v", err)
}
defer cleanUpTopic(ctx, t, admin, topicPath)
if diff := testutil.Diff(gotTopicConfig, newTopicConfig, configCmpOptions...); diff != "" {
if diff := testutil.Diff(gotTopicConfig, newTopicConfig); diff != "" {
t.Errorf("CreateTopic() got: -, want: +\n%s", diff)
}

if gotTopicConfig, err := admin.Topic(ctx, topicPath); err != nil {
t.Errorf("Failed to get topic: %v", err)
} else if diff := testutil.Diff(gotTopicConfig, newTopicConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotTopicConfig, newTopicConfig); diff != "" {
t.Errorf("Topic() got: -, want: +\n%s", diff)
}

Expand All @@ -284,14 +240,14 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
if err == iterator.Done {
break
}
if TopicPathsEqual(topic.Name, topicPath) {
if topic.Name == topicPath {
foundTopic = topic
break
}
}
if foundTopic == nil {
t.Error("Topics() did not return topic config")
} else if diff := testutil.Diff(foundTopic, newTopicConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(foundTopic, newTopicConfig); diff != "" {
t.Errorf("Topics() found config: -, want: +\n%s", diff)
}

Expand All @@ -302,7 +258,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
if err == iterator.Done {
break
}
if TopicPathsEqual(topicPath, path) {
if topicPath == path {
foundTopicPath = true
break
}
Expand All @@ -328,7 +284,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
}
if gotTopicConfig, err := admin.UpdateTopic(ctx, topicUpdate1); err != nil {
t.Errorf("Failed to update topic: %v", err)
} else if diff := testutil.Diff(gotTopicConfig, wantUpdatedTopicConfig1, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotTopicConfig, wantUpdatedTopicConfig1); diff != "" {
t.Errorf("UpdateTopic() got: -, want: +\n%s", diff)
}

Expand All @@ -349,7 +305,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
}
if gotTopicConfig, err := admin.UpdateTopic(ctx, topicUpdate2); err != nil {
t.Errorf("Failed to update topic: %v", err)
} else if diff := testutil.Diff(gotTopicConfig, wantUpdatedTopicConfig2, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotTopicConfig, wantUpdatedTopicConfig2); diff != "" {
t.Errorf("UpdateTopic() got: -, want: +\n%s", diff)
}

Expand All @@ -365,13 +321,13 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
t.Fatalf("Failed to create subscription: %v", err)
}
defer cleanUpSubscription(ctx, t, admin, subscriptionPath)
if diff := testutil.Diff(gotSubsConfig, newSubsConfig, configCmpOptions...); diff != "" {
if diff := testutil.Diff(gotSubsConfig, newSubsConfig); diff != "" {
t.Errorf("CreateSubscription() got: -, want: +\n%s", diff)
}

if gotSubsConfig, err := admin.Subscription(ctx, subscriptionPath); err != nil {
t.Errorf("Failed to get subscription: %v", err)
} else if diff := testutil.Diff(gotSubsConfig, newSubsConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotSubsConfig, newSubsConfig); diff != "" {
t.Errorf("Subscription() got: -, want: +\n%s", diff)
}

Expand All @@ -382,14 +338,14 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
if err == iterator.Done {
break
}
if SubscriptionPathsEqual(subs.Name, subscriptionPath) {
if subs.Name == subscriptionPath {
foundSubs = subs
break
}
}
if foundSubs == nil {
t.Error("Subscriptions() did not return subscription config")
} else if diff := testutil.Diff(foundSubs, gotSubsConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(foundSubs, gotSubsConfig); diff != "" {
t.Errorf("Subscriptions() found config: -, want: +\n%s", diff)
}

Expand All @@ -400,7 +356,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
if err == iterator.Done {
break
}
if SubscriptionPathsEqual(subsPath, subscriptionPath) {
if subsPath == subscriptionPath {
foundSubsPath = true
break
}
Expand All @@ -420,7 +376,7 @@ func TestIntegration_ResourceAdminOperations(t *testing.T) {
}
if gotSubsConfig, err := admin.UpdateSubscription(ctx, subsUpdate); err != nil {
t.Errorf("Failed to update subscription: %v", err)
} else if diff := testutil.Diff(gotSubsConfig, wantUpdatedSubsConfig, configCmpOptions...); diff != "" {
} else if diff := testutil.Diff(gotSubsConfig, wantUpdatedSubsConfig); diff != "" {
t.Errorf("UpdateSubscription() got: -, want: +\n%s", diff)
}

Expand Down

0 comments on commit 320f497

Please sign in to comment.