Skip to content

Commit 1623509

Browse files
authored
refactor(firestore): modularize and improve test suite structure (#5505)
* refactor(firestore): modularize and improve test suite structure * fix test failures * gofmt
1 parent 5d70c1c commit 1623509

23 files changed

Lines changed: 534 additions & 384 deletions

firestore/aggregate_query_count_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,14 @@ package firestore
1616

1717
import (
1818
"bytes"
19-
"os"
2019
"strings"
2120
"testing"
2221
)
2322

2423
func TestCreateCountQuery(t *testing.T) {
2524
// Note: This test assumes a pre-populated Firestore collection in the
2625
// below-referenced project.
27-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
26+
projectID := getProjectID(t)
2827
var bytes bytes.Buffer
2928
err := createCountQuery(&bytes, projectID)
3029
if err != nil {

firestore/aggregate_query_explain_analyze_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,12 @@ package firestore
1616

1717
import (
1818
"bytes"
19-
"os"
2019
"strings"
2120
"testing"
2221
)
2322

2423
func TestAggregationQueryExplainAnalyze(t *testing.T) {
25-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
26-
if projectID == "" {
27-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
28-
}
24+
projectID := getProjectID(t)
2925

3026
_, cleanup := setupClientAndCities(t, projectID)
3127
t.Cleanup(cleanup)

firestore/aggregate_query_explain_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,12 @@ package firestore
1616

1717
import (
1818
"bytes"
19-
"os"
2019
"strings"
2120
"testing"
2221
)
2322

2423
func TestAggregationQueryExplain(t *testing.T) {
25-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
26-
if projectID == "" {
27-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
28-
}
24+
projectID := getProjectID(t)
2925

3026
_, cleanup := setupClientAndCities(t, projectID)
3127
t.Cleanup(cleanup)

firestore/collection_group_test.go

Lines changed: 146 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,22 @@ package firestore
1717
import (
1818
"bytes"
1919
"context"
20-
"os"
20+
"fmt"
2121
"strings"
2222
"testing"
2323

2424
"cloud.google.com/go/firestore"
25+
apiv1 "cloud.google.com/go/firestore/apiv1/admin"
26+
"cloud.google.com/go/firestore/apiv1/admin/adminpb"
2527
"github.com/GoogleCloudPlatform/golang-samples/internal/testutil"
28+
"google.golang.org/genproto/protobuf/field_mask"
2629
)
2730

2831
func TestCollectionGroup(t *testing.T) {
2932
tc := testutil.SystemTest(t)
3033
// TODO(#559): revert this to testutil.SystemTest(t).ProjectID
3134
// when datastore and firestore can co-exist in a project.
32-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
33-
if projectID == "" {
34-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
35-
}
35+
projectID := getProjectID(t)
3636

3737
ctx := context.Background()
3838

@@ -45,18 +45,25 @@ func TestCollectionGroup(t *testing.T) {
4545
collection := tc.ProjectID + "-collection-group-cities"
4646

4747
// Delete all docs first to make sure collectionGroupSetup works.
48-
docs, err := client.Collection(collection).DocumentRefs(ctx).GetAll()
49-
// Ignore errors; this isn't essential.
50-
if err == nil {
51-
for _, d := range docs {
52-
d.Delete(ctx)
53-
}
54-
}
48+
cleanupCollection(ctx, t, client, collection)
5549

5650
if err := collectionGroupSetup(projectID, collection); err != nil {
5751
t.Fatalf("collectionGroupSetup: %v", err)
5852
}
5953

54+
if indexCleanup, err := createIndexExemption(projectID); err != nil {
55+
t.Fatalf("createCollectionGroupIndex: %v", err)
56+
} else {
57+
t.Cleanup(func() {
58+
if indexCleanup != nil {
59+
err := indexCleanup()
60+
if err != nil {
61+
t.Errorf("cleanup failed: %v", err)
62+
}
63+
}
64+
})
65+
}
66+
6067
buf := &bytes.Buffer{}
6168
if err := collectionGroupQuery(buf, projectID); err != nil {
6269
t.Fatalf("collectionGroupQuery: %v", err)
@@ -67,14 +74,122 @@ func TestCollectionGroup(t *testing.T) {
6774
}
6875
}
6976

77+
func createIndexExemption(projectID string) (func() error, error) {
78+
ctx := context.Background()
79+
80+
// Create admin client
81+
adminClient, err := apiv1.NewFirestoreAdminClient(ctx)
82+
if err != nil {
83+
return nil, fmt.Errorf("NewFirestoreAdminClient: %v", err)
84+
}
85+
defer adminClient.Close()
86+
87+
fieldResourceName := fmt.Sprintf("projects/%s/databases/%s/collectionGroups/%s/fields/%s",
88+
projectID, "(default)", "landmarks", "type")
89+
90+
getFieldRequest := &adminpb.GetFieldRequest{
91+
Name: fieldResourceName,
92+
}
93+
94+
origField, err := adminClient.GetField(ctx, getFieldRequest)
95+
if err != nil {
96+
return nil, fmt.Errorf("GetField: %v", err)
97+
}
98+
99+
updateReq := &adminpb.UpdateFieldRequest{
100+
Field: &adminpb.Field{
101+
Name: fieldResourceName,
102+
IndexConfig: &adminpb.Field_IndexConfig{
103+
// Providing an empty list of indexes disables all automatic indexes
104+
Indexes: []*adminpb.Index{
105+
{
106+
QueryScope: adminpb.Index_COLLECTION,
107+
Fields: []*adminpb.Index_IndexField{
108+
{
109+
FieldPath: "type",
110+
ValueMode: &adminpb.Index_IndexField_Order_{
111+
Order: adminpb.Index_IndexField_ASCENDING,
112+
},
113+
},
114+
},
115+
},
116+
{
117+
QueryScope: adminpb.Index_COLLECTION,
118+
Fields: []*adminpb.Index_IndexField{
119+
{
120+
FieldPath: "type",
121+
ValueMode: &adminpb.Index_IndexField_Order_{
122+
Order: adminpb.Index_IndexField_DESCENDING,
123+
},
124+
},
125+
},
126+
},
127+
{
128+
QueryScope: adminpb.Index_COLLECTION,
129+
Fields: []*adminpb.Index_IndexField{
130+
{
131+
FieldPath: "type",
132+
ValueMode: &adminpb.Index_IndexField_ArrayConfig_{
133+
ArrayConfig: adminpb.Index_IndexField_CONTAINS,
134+
},
135+
},
136+
},
137+
},
138+
{
139+
QueryScope: adminpb.Index_COLLECTION_GROUP,
140+
Fields: []*adminpb.Index_IndexField{
141+
{
142+
FieldPath: "type",
143+
ValueMode: &adminpb.Index_IndexField_Order_{
144+
Order: adminpb.Index_IndexField_ASCENDING,
145+
},
146+
},
147+
},
148+
},
149+
},
150+
},
151+
},
152+
UpdateMask: &field_mask.FieldMask{
153+
Paths: []string{"index_config"},
154+
},
155+
}
156+
157+
op, updateErr := adminClient.UpdateField(ctx, updateReq)
158+
if updateErr != nil {
159+
if strings.Contains(updateErr.Error(), "already exists") {
160+
return nil, nil
161+
}
162+
return nil, fmt.Errorf("UpdateField: %v", updateErr)
163+
}
164+
// Wait until the operation completes.
165+
_, waitErr := op.Wait(ctx)
166+
if waitErr != nil {
167+
return nil, fmt.Errorf("UpdateField.Wait: %v", waitErr)
168+
}
169+
return func() error {
170+
adminClient, err := apiv1.NewFirestoreAdminClient(ctx)
171+
if err != nil {
172+
return fmt.Errorf("NewFirestoreAdminClient: %v", err)
173+
}
174+
defer adminClient.Close()
175+
_, updateErr := adminClient.UpdateField(ctx, &adminpb.UpdateFieldRequest{
176+
Field: origField,
177+
UpdateMask: &field_mask.FieldMask{
178+
Paths: []string{"index_config"},
179+
},
180+
})
181+
if updateErr != nil {
182+
return fmt.Errorf("UpdateField: %v", updateErr)
183+
}
184+
return nil
185+
}, nil
186+
}
187+
70188
func TestCollectionGroupPartitionQueries(t *testing.T) {
71189
tc := testutil.SystemTest(t)
72190
// TODO(#559): revert this to testutil.SystemTest(t).ProjectID
73191
// when datastore and firestore can co-exist in a project.
74-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
75-
if projectID == "" {
76-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
77-
}
192+
projectID := getProjectID(t)
78193

79194
ctx := context.Background()
80195

@@ -87,13 +202,7 @@ func TestCollectionGroupPartitionQueries(t *testing.T) {
87202
collection := tc.ProjectID + "-collection-group-cities"
88203

89204
// Delete all docs first to make sure collectionGroupSetup works.
90-
docs, err := client.Collection(collection).DocumentRefs(ctx).GetAll()
91-
// Ignore errors; this isn't essential.
92-
if err == nil {
93-
for _, d := range docs {
94-
d.Delete(ctx)
95-
}
96-
}
205+
cleanupCollection(ctx, t, client, collection)
97206

98207
if err := collectionGroupSetup(projectID, collection); err != nil {
99208
t.Fatalf("collectionGroupSetup: %v", err)
@@ -109,3 +218,17 @@ func TestCollectionGroupPartitionQueries(t *testing.T) {
109218
t.Errorf("serializePartitionQuery unexpected result: %s", err)
110219
}
111220
}
221+
222+
func cleanupCollection(ctx context.Context, t *testing.T, client *firestore.Client, collection string) {
223+
t.Helper()
224+
docs, err := client.Collection(collection).DocumentRefs(ctx).GetAll()
225+
if err != nil {
226+
t.Logf("Warning: failed to get document refs for cleanup: %v", err)
227+
return
228+
}
229+
for _, d := range docs {
230+
if _, err := d.Delete(ctx); err != nil {
231+
t.Logf("Warning: failed to delete doc %s: %v", d.ID, err)
232+
}
233+
}
234+
}

firestore/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require (
66
cloud.google.com/go/firestore v1.18.0
77
github.com/GoogleCloudPlatform/golang-samples v0.0.0-20240724083556-7f760db013b7
88
google.golang.org/api v0.217.0
9+
google.golang.org/genproto v0.0.0-20250115164207-1a7da9e5054f
910
google.golang.org/grpc v1.69.4
1011
)
1112

@@ -51,7 +52,6 @@ require (
5152
golang.org/x/sys v0.29.0 // indirect
5253
golang.org/x/text v0.21.0 // indirect
5354
golang.org/x/time v0.9.0 // indirect
54-
google.golang.org/genproto v0.0.0-20250115164207-1a7da9e5054f // indirect
5555
google.golang.org/genproto/googleapis/api v0.0.0-20250115164207-1a7da9e5054f // indirect
5656
google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f // indirect
5757
google.golang.org/protobuf v1.36.3 // indirect

firestore/increment_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package firestore
1616

1717
import (
1818
"context"
19-
"os"
2019
"testing"
2120

2221
"cloud.google.com/go/firestore"
@@ -27,10 +26,7 @@ func TestUpdateDocumentIncrement(t *testing.T) {
2726
tc := testutil.SystemTest(t)
2827
// TODO(#559): revert this to testutil.SystemTest(t).ProjectID
2928
// when datastore and firestore can co-exist in a project.
30-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
31-
if projectID == "" {
32-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
33-
}
29+
projectID := getProjectID(t)
3430

3531
ctx := context.Background()
3632

firestore/listen_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"context"
2020
"io/ioutil"
2121
"log"
22-
"os"
2322
"strings"
2423
"testing"
2524
"time"
@@ -32,10 +31,7 @@ var duration = 20 * time.Second
3231

3332
func setup(ctx context.Context, t *testing.T) (*firestore.Client, string, string) {
3433
tc := testutil.SystemTest(t)
35-
projectID := os.Getenv("GOLANG_SAMPLES_FIRESTORE_PROJECT")
36-
if projectID == "" {
37-
t.Skip("Skipping firestore test. Set GOLANG_SAMPLES_FIRESTORE_PROJECT.")
38-
}
34+
projectID := getProjectID(t)
3935
collection := tc.ProjectID + "-collection-cities"
4036

4137
client, err := firestore.NewClient(ctx, projectID)

0 commit comments

Comments
 (0)