Skip to content

Commit

Permalink
test(firestore): Resolve flaky tests (#11281)
Browse files Browse the repository at this point in the history 
* test(firestore): Resolve flaky tests

* update cloud.google.com/go version to fix kokoro build failure
  • Loading branch information
@bhshkh
bhshkh authored Dec 20, 2024
1 parent f91871c commit 156f02c
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 53 deletions.
2 changes: 1 addition & 1 deletion firestore/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module cloud.google.com/go/firestore
go 1.21

require (
cloud.google.com/go v0.116.0
cloud.google.com/go v0.117.0
cloud.google.com/go/longrunning v0.6.2
github.com/google/go-cmp v0.6.0
github.com/googleapis/gax-go/v2 v2.14.0
Expand Down
4 changes: 2 additions & 2 deletions firestore/go.sum
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
cloud.google.com/go v0.116.0 h1:B3fRrSDkLRt5qSHWe40ERJvhvnQwdZiHu0bJOpldweE=
cloud.google.com/go v0.116.0/go.mod h1:cEPSRWPzZEswwdr9BxE6ChEn01dWlTaF05LiC2Xs70U=
cloud.google.com/go v0.117.0 h1:Z5TNFfQxj7WG2FgOGX1ekC5RiXrYgms6QscOm32M/4s=
cloud.google.com/go v0.117.0/go.mod h1:ZbwhVTb1DBGt2Iwb3tNO6SEK4q+cplHZmLWH+DelYYc=
cloud.google.com/go/auth v0.12.1 h1:n2Bj25BUMM0nvE9D2XLTiImanwZhO3DkfWSYS/SAJP4=
cloud.google.com/go/auth v0.12.1/go.mod h1:BFMu+TNpF3DmvfBO9ClqTR/SiqVIm7LukKF9mbendF4=
cloud.google.com/go/auth/oauth2adapt v0.2.6 h1:V6a6XDu2lTwPZWOawrAa9HUK+DB2zfJyTuciBG5hFkU=
Expand Down
132 changes: 82 additions & 50 deletions firestore/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const (
envPrivateKey = "GCLOUD_TESTS_GOLANG_FIRESTORE_KEY"
envDatabases = "GCLOUD_TESTS_GOLANG_FIRESTORE_DATABASES"
envEmulator = "FIRESTORE_EMULATOR_HOST"
indexBuilding = "index is currently building"
)

var (
Expand Down Expand Up @@ -260,12 +261,14 @@ func handleCreateIndexResp(ctx context.Context, indexNames []string, wg *sync.Wa
// deleteIndexes deletes composite indexes created in createIndexes function
func deleteIndexes(ctx context.Context, indexNames []string) {
for _, indexName := range indexNames {
err := iAdminClient.DeleteIndex(ctx, &adminpb.DeleteIndexRequest{
Name: indexName,
testutil.RetryWithoutTest(5, 5*time.Second, func(r *testutil.R) {
err := iAdminClient.DeleteIndex(ctx, &adminpb.DeleteIndexRequest{
Name: indexName,
})
if err != nil {
r.Errorf("Failed to delete index \"%s\": %+v\n", indexName, err)
}
})
if err != nil {
log.Printf("Failed to delete index \"%s\": %+v\n", indexName, err)
}
}
}

Expand Down Expand Up @@ -2437,7 +2440,7 @@ func TestDetectProjectID(t *testing.T) {
ts := testutil.ErroringTokenSource{}
// Try to use creds without project ID.
_, err := NewClient(ctx, DetectProjectID, option.WithTokenSource(ts))
if err == nil || err.Error() != "firestore: see the docs on DetectProjectID" {
if err == nil || err.Error() != "unable to detect projectID, please refer to docs for DetectProjectID" {
t.Errorf("expected an error while using TokenSource that does not have a project ID")
}
}
Expand Down Expand Up @@ -2931,28 +2934,39 @@ func TestIntegration_AggregationQueries(t *testing.T) {
}

for _, tc := range testcases {
var aggResult AggregationResult
var err error
if tc.runInTransaction {
client.RunTransaction(ctx, func(ctx context.Context, tx *Transaction) error {
aggResult, err = tc.aggregationQuery.Transaction(tx).Get(ctx)
return err
t.Run(tc.desc, func(t *testing.T) {
testutil.Retry(t, 5, 5*time.Second, func(r *testutil.R) {
var aggResult AggregationResult
var err error
if tc.runInTransaction {
client.RunTransaction(ctx, func(ctx context.Context, tx *Transaction) error {
aggResult, err = tc.aggregationQuery.Transaction(tx).Get(ctx)
return err
})
} else {
aggResult, err = tc.aggregationQuery.Get(ctx)
}

// Retry only if index building is in progress
s, ok := status.FromError(err)
if err != nil && ok && s != nil && s.Code() != codes.FailedPrecondition &&
strings.Contains(s.Message(), indexBuilding) {
r.Errorf("Get: %v", err)
return
}

// Compare expected and actual results
if err != nil && !tc.wantErr {
r.Fatalf("got: %v, want: nil", err)
}
if err == nil && tc.wantErr {
r.Fatalf("got: %v, wanted error", err)
}
if !reflect.DeepEqual(aggResult, tc.result) {
r.Fatalf("got: %v, want: %v", aggResult, tc.result)
}
})
} else {
aggResult, err = tc.aggregationQuery.Get(ctx)
}
if err != nil && !tc.wantErr {
t.Errorf("%s: got: %v, want: nil", tc.desc, err)
continue
}
if err == nil && tc.wantErr {
t.Errorf("%s: got: %v, wanted error", tc.desc, err)
continue
}
if !reflect.DeepEqual(aggResult, tc.result) {
t.Errorf("%s: got: %v, want: %v", tc.desc, aggResult, tc.result)
continue
}
})
}
}

Expand Down Expand Up @@ -3313,33 +3327,51 @@ func TestIntegration_FindNearest(t *testing.T) {
},
} {
t.Run(tc.desc, func(t *testing.T) {
iter := tc.vq.Documents(ctx)
gotDocs, err := iter.GetAll()
if err != nil {
t.Fatalf("GetAll: %+v", err)
}

if len(gotDocs) != len(tc.wantBeans) {
t.Fatalf("Expected %v results, got %d", len(tc.wantBeans), len(gotDocs))
}

for i, doc := range gotDocs {
var gotBean coffeeBean
if len(tc.wantResField) != 0 {
_, ok := doc.Data()[tc.wantResField]
if !ok {
t.Errorf("Expected %v field to exist in %v", tc.wantResField, doc.Data())
}
testutil.Retry(t, 5, 5*time.Second, func(r *testutil.R) {
// Get all documents
iter := tc.vq.Documents(ctx)
gotDocs, err := iter.GetAll()

// Retry only if index building is in progress
s, ok := status.FromError(err)
if err != nil && ok && s != nil && s.Code() != codes.FailedPrecondition &&
strings.Contains(s.Message(), indexBuilding) {
r.Errorf("GetAll: %v", err)
return
}
err := doc.DataTo(&gotBean)

if err != nil {
t.Errorf("#%v: DataTo: %+v", doc.Ref.ID, err)
continue
t.Fatalf("GetAll: %+v", err)
}

// Compare expected and actual results length
if len(gotDocs) != len(tc.wantBeans) {
t.Fatalf("Expected %v results, got %d", len(tc.wantBeans), len(gotDocs))
}
if tc.wantBeans[i].ID != gotBean.ID {
t.Errorf("#%v: want: %v, got: %v", i, beans[i].ID, gotBean.ID)

// Compare results
for i, doc := range gotDocs {
var gotBean coffeeBean

// Compare expected and actual result field
if len(tc.wantResField) != 0 {
_, ok := doc.Data()[tc.wantResField]
if !ok {
t.Errorf("Expected %v field to exist in %v", tc.wantResField, doc.Data())
}
}

// Compare expected and actual document ID
err := doc.DataTo(&gotBean)
if err != nil {
t.Errorf("#%v: DataTo: %+v", doc.Ref.ID, err)
continue
}
if tc.wantBeans[i].ID != gotBean.ID {
t.Errorf("#%v: want: %v, got: %v", i, beans[i].ID, gotBean.ID)
}
}
}
})
})
}
}

0 comments on commit 156f02c

Please sign in to comment.