Don't use reflect.DeepEqual to compare errors.

FB-1771

In api_test.go, it was being used to make sure that incorrect input
produced the right errors; this is now handled by making sure the
error isn't nil and then checking its string against the expected
error string.

In executor_test.go and internal_client_test.go, it was being used
to compare QueryResponse structures, which contain an error.
handler.go now has a function specifically for comparing them,
which can provide additional detail if necessary.

Added a test for SameAs to handler_test.go.

(cherry picked from commit 775fd0b08c)

api_test.go

@@ -1265,8 +1265,8 @@ func TestVariousApiTranslateCalls(t *testing.T) {
 		t.Run("translateIndexDbOnNilIndex",
 			func(t *testing.T) {
 				err := api.TranslateIndexDB(context.Background(), "nonExistentIndex", 0, r)
-				expected := fmt.Errorf("index %q not found", "nonExistentIndex")
-				if !reflect.DeepEqual(err, expected) {
+				expected := fmt.Sprintf("index %q not found", "nonExistentIndex")
+				if err == nil || err.Error() != expected {
 					t.Fatalf("expected '%#v', got '%#v'", expected, err)
 				}
 			})
@@ -1274,8 +1274,8 @@ func TestVariousApiTranslateCalls(t *testing.T) {
 		t.Run("translateIndexDbOnNilTranslateStore",
 			func(t *testing.T) {
 				err := api.TranslateIndexDB(context.Background(), c.Idx(), 0, r)
-				expected := fmt.Errorf("index %q has no translate store", c.Idx())
-				if !reflect.DeepEqual(err, expected) {
+				expected := fmt.Sprintf("index %q has no translate store", c.Idx())
+				if err == nil || err.Error() != expected {
 					t.Fatalf("expected '%#v', got '%#v'", expected, err)
 				}
 			})
@@ -1283,8 +1283,8 @@ func TestVariousApiTranslateCalls(t *testing.T) {
 		t.Run("translateFieldDbOnNilIndex",
 			func(t *testing.T) {
 				err := api.TranslateFieldDB(context.Background(), "nonExistentIndex", "field", r)
-				expected := fmt.Errorf("index %q not found", "nonExistentIndex")
-				if !reflect.DeepEqual(err, expected) {
+				expected := fmt.Sprintf("index %q not found", "nonExistentIndex")
+				if err == nil || err.Error() != expected {
 					t.Fatalf("expected '%#v', got '%#v'", expected, err)
 				}
 			})
@@ -1292,8 +1292,8 @@ func TestVariousApiTranslateCalls(t *testing.T) {
 		t.Run("translateFieldDbOnNilField",
 			func(t *testing.T) {
 				err := api.TranslateFieldDB(context.Background(), c.Idx(), "nonExistentField", r)
-				expected := fmt.Errorf("field %q/%q not found", c.Idx(), "nonExistentField")
-				if !reflect.DeepEqual(err, expected) {
+				expected := fmt.Sprintf("field %q/%q not found", c.Idx(), "nonExistentField")
+				if err == nil || err.Error() != expected {
 					t.Fatalf("expected '%#v', got '%#v'", expected, err)
 				}
 			})

executor_test.go

@@ -9335,7 +9335,7 @@ func TestExternalLookup(t *testing.T) {
 				t.Parallel()
 
 				result := c.Query(t, c.Idx(), tc.query)
-				if !reflect.DeepEqual(result, tc.expect) {
+				if tc.expect.SameAs(&result) != nil {
 					t.Errorf("expected %v but got %v", tc.expect, result)
 				}
 			})

handler.go

@@ -3,7 +3,9 @@ package pilosa
 
 import (
 	"encoding/json"
+	"fmt"
 	"math/bits"
+	"reflect"
 	"time"
 
 	"github.com/featurebasedb/featurebase/v3/shardwidth"
@@ -77,6 +79,28 @@ func (resp *QueryResponse) MarshalJSON() ([]byte, error) {
 	})
 }
 
+// SameAs compares one QueryResponse to another and returns nil if the Results
+// and Err are both identical, or a descriptive error if they differ.
+// This function replaces using reflect.DeepEqual directly on the
+// QueryResponse, since QueryResponse contains an error field and reflect.DeepEqual
+// should not be used on errors.
+func (qr *QueryResponse) SameAs(other *QueryResponse) error {
+	switch {
+	case !reflect.DeepEqual(qr.Results, other.Results):
+		return fmt.Errorf("responses contained different results")
+	case qr.Err == nil && other.Err == nil:
+		return nil
+	case qr.Err == nil && other.Err != nil:
+		return fmt.Errorf("unexpected error: %w", other.Err)
+	case qr.Err != nil && other.Err == nil:
+		return fmt.Errorf("missing error: expected %v, got no error", qr.Err)
+	case qr.Err.Error() != other.Err.Error():
+		return fmt.Errorf("wrong error: expected %v, got %w", qr.Err, other.Err)
+	default:
+		return nil
+	}
+}
+
 // HandlerI is the interface for the data handler, a wrapper around
 // Pilosa's data store.
 type HandlerI interface {

handler_test.go

@@ -2,7 +2,9 @@
 package pilosa_test
 
 import (
+	"fmt"
 	"math/rand"
+	"strings"
 	"testing"
 
 	pilosa "github.com/featurebasedb/featurebase/v3"
@@ -26,3 +28,38 @@ func TestSortToShards(t *testing.T) {
 		}
 	}
 }
+
+func TestQueryResponseSameAs(t *testing.T) {
+	var qr1, qr2 pilosa.QueryResponse
+	qr1.Results = []any{"testing different results"}
+	qr2.Results = []any{"with SCIENCE!"}
+	qr1.Err = nil
+	qr2.Err = nil
+	if err := qr1.SameAs(&qr2); err.Error() != "responses contained different results" {
+		t.Fatalf("expected responses to contain different results")
+	}
+	qr1.Results = []any{"this time, for sure!"}
+	qr2.Results = []any{"this time, for sure!"}
+	if err := qr1.SameAs(&qr2); err != nil {
+		t.Fatalf("expected responses to be the same")
+	}
+	err1 := fmt.Errorf("out of cheese")
+	err2 := fmt.Errorf("out of dryd frorg pills")
+	qr1.Err = err1
+	if err := qr1.SameAs(&qr2); !strings.Contains(err.Error(), "missing error") {
+		t.Fatalf("should have had a missing error")
+	}
+	qr1.Err = nil
+	qr2.Err = err2
+	if err := qr1.SameAs(&qr2); !strings.Contains(err.Error(), "unexpected error") {
+		t.Fatalf("should have had an unexpected error")
+	}
+	qr1.Err = err1
+	if err := qr1.SameAs(&qr2); !strings.Contains(err.Error(), "wrong error") {
+		t.Fatalf("should have had a wrong error")
+	}
+	qr2.Err = err1
+	if err := qr1.SameAs(&qr2); err != nil {
+		t.Fatalf("expected responses top be the same")
+	}
+}

internal_client_test.go

@@ -161,11 +161,11 @@ func TestClient_MultiNode(t *testing.T) {
 	}
 
 	// Compare TopN results across all nodes in the cluster.
-	if !reflect.DeepEqual(result, result1) {
+	if result.SameAs(result1) != nil {
 		t.Fatalf("TopN result should be the same on node0 and node1: %s", spew.Sdump(result1))
 	}
 
-	if !reflect.DeepEqual(result, result2) {
+	if result.SameAs(result2) != nil {
 		t.Fatalf("TopN result should be the same on node0 and node2: %s", spew.Sdump(result2))
 	}
 }