chore(firestore): fix the query round trip test (#10585)

The QueryFromProtoRoundTrip test was failing intermittently,
because it relied on the iteration order of maps.

Looking at it more closely, the test's more significant problem
was that it was comparing byte slices, so diffs were unhelpful.

Change the test to compare protos before and after round-tripping.
All cases now pass reliably.

Use subtests to make it easier to isolate individual cases for debugging.

Also, fix a potential bug when converting a proto to a Query:
the transformation from a structured filter to a list of filters is
only valid for AND filters, not OR filters.

firestore/query.go

@@ -355,7 +355,7 @@ func (q Query) Serialize() ([]byte, error) {
 
 // Deserialize takes a slice of bytes holding the wire-format message of RunQueryRequest,
 // the underlying proto message used by Queries. It then populates and returns a
-// Query object that can be used to execut that Query.
+// Query object that can be used to execute that Query.
 func (q Query) Deserialize(bytes []byte) (Query, error) {
 	runQueryRequest := pb.RunQueryRequest{}
 	err := proto.Unmarshal(bytes, &runQueryRequest)
@@ -558,7 +558,7 @@ func (q Query) fromProto(pbQuery *pb.RunQueryRequest) (Query, error) {
 
 	// 	filters                []*pb.StructuredQuery_Filter
 	if w := pbq.GetWhere(); w != nil {
-		if cf := w.GetCompositeFilter(); cf != nil {
+		if cf := w.GetCompositeFilter(); cf != nil && cf.Op == pb.StructuredQuery_CompositeFilter_AND {
 			q.filters = cf.GetFilters()
 		} else {
 			q.filters = []*pb.StructuredQuery_Filter{w}

firestore/query_test.go

@@ -16,7 +16,6 @@ package firestore
 
 import (
 	"context"
-	"fmt"
 	"math"
 	"sort"
 	"testing"
@@ -732,33 +731,29 @@ func TestQueryToProto(t *testing.T) {
 	}
 }
 
-// Convert a Query to a Proto and back again verifying roundtripping
+// Convert a Query to a Proto and back again verifying roundtripping.
+// We cannot in general verify the round trip from Query back to Query,
+// because information is lost. But we can check that the deserialized query's proto
+// matches the original proto.
 func TestQueryFromProtoRoundTrip(t *testing.T) {
-	t.Skip("flaky due to random map order iteration")
 	c := &Client{projectID: "P", databaseID: "DB"}
-
 	for _, test := range createTestScenarios(t) {
-		fmt.Println(test.desc)
-		proto, err := test.in.Serialize()
-		if err != nil {
-			t.Fatalf("%s: %v", test.desc, err)
-		}
-		got, err := Query{c: c}.Deserialize(proto)
-		if err != nil {
-			t.Fatalf("%s: %v", test.desc, err)
-		}
-
-		want := test.in
-		gotProto, err := got.Serialize()
-		fmt.Println(gotProto)
-		if err != nil {
-			t.Fatalf("%s: %v", test.desc, err)
-		}
-
-		// Compare protos before and after taking to a query. proto -> query -> proto.
-		if diff := cmp.Diff(gotProto, proto, protocmp.Transform()); diff != "" {
-			t.Errorf("%s:\ngot\n%v\nwant\n%v\ndiff\n%v", test.desc, pretty.Value(got), pretty.Value(want), diff)
-		}
+		t.Run(test.desc, func(t *testing.T) {
+			protoBytes, err := test.in.Serialize()
+			if err != nil {
+				t.Fatal(err)
+			}
+			gotq, err := Query{c: c}.Deserialize(protoBytes)
+			if err != nil {
+				t.Fatal(err)
+			}
+			got, err := gotq.toProto()
+			want := test.want
+			want.From = []*pb.StructuredQuery_CollectionSelector{{CollectionId: "C"}}
+			if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
+				t.Errorf("mismatch (-want, +got)\n: %s", diff)
+			}
+		})
 	}
 }