Respond to PR feedback.

Author: nicktobey

enginetest/enginetests.go

@@ -263,7 +263,7 @@ func TestBrokenQueries(t *testing.T, harness Harness) {
 // queries during debugging.
 func RunQueryTests(t *testing.T, harness Harness, queries []queries.QueryTest) {
 	for _, tt := range queries {
-		testQuery(t, harness, tt.Query, tt.Expected, tt.ExpectedColumns, nil, !tt.DontUnwrap)
+		testQuery(t, harness, tt.Query, tt.Expected, tt.ExpectedColumns, nil, tt.WrapBehavior)
 	}
 }
 

enginetest/evaluation.go

@@ -326,10 +326,10 @@ func TestTransactionScriptWithEngine(t *testing.T, e QueryEngine, harness Harnes
 // TestQuery runs a query on the engine given and asserts that results are as expected.
 // TODO: this should take en engine
 func TestQuery(t *testing.T, harness Harness, q string, expected []sql.Row, expectedCols []*sql.Column, bindings map[string]sqlparser.Expr) {
-	testQuery(t, harness, q, expected, expectedCols, bindings, true)
+	testQuery(t, harness, q, expected, expectedCols, bindings, queries.WrapBehavior_Unwrap)
 }
 
-func testQuery(t *testing.T, harness Harness, q string, expected []sql.Row, expectedCols []*sql.Column, bindings map[string]sqlparser.Expr, unwrapValues bool) {
+func testQuery(t *testing.T, harness Harness, q string, expected []sql.Row, expectedCols []*sql.Column, bindings map[string]sqlparser.Expr, wrapBehavior queries.WrapBehavior) {
 	t.Run(q, func(t *testing.T) {
 		if sh, ok := harness.(SkippingHarness); ok {
 			if sh.SkipQueryTest(q) {
@@ -340,7 +340,7 @@ func testQuery(t *testing.T, harness Harness, q string, expected []sql.Row, expe
 		e := mustNewEngine(t, harness)
 		defer e.Close()
 		ctx := NewContext(harness)
-		testQueryWithContext(t, ctx, e, harness, q, expected, expectedCols, bindings, nil, unwrapValues)
+		testQueryWithContext(t, ctx, e, harness, q, expected, expectedCols, bindings, nil, wrapBehavior)
 	})
 }
 
@@ -383,7 +383,7 @@ func TestQueryWithContext(
 	bindings map[string]sqlparser.Expr,
 	qFlags *sql.QueryFlags,
 ) {
-	testQueryWithContext(t, ctx, e, harness, q, expected, expectedCols, bindings, qFlags, true)
+	testQueryWithContext(t, ctx, e, harness, q, expected, expectedCols, bindings, qFlags, queries.WrapBehavior_Unwrap)
 }
 
 func testQueryWithContext(
@@ -396,7 +396,7 @@ func testQueryWithContext(
 	expectedCols []*sql.Column,
 	bindings map[string]sqlparser.Expr,
 	qFlags *sql.QueryFlags,
-	unwrapValues bool,
+	wrapBehavior queries.WrapBehavior,
 ) {
 	ctx = ctx.WithQuery(q)
 	require := require.New(t)
@@ -412,7 +412,7 @@ func testQueryWithContext(
 	require.NoError(err, "Unexpected error for query %s: %s", q, err)
 
 	if expected != nil {
-		checkResults(t, harness, expected, expectedCols, sch, rows, q, e, unwrapValues)
+		checkResults(t, harness, expected, expectedCols, sch, rows, q, e, wrapBehavior)
 	}
 
 	require.Equal(
@@ -531,7 +531,7 @@ func CheckResults(
 	q string,
 	e QueryEngine,
 ) {
-	checkResults(t, h, expected, expectedCols, sch, rows, q, e, true)
+	checkResults(t, h, expected, expectedCols, sch, rows, q, e, queries.WrapBehavior_Unwrap)
 }
 
 func checkResults(
@@ -543,12 +543,12 @@ func checkResults(
 	rows []sql.Row,
 	q string,
 	e QueryEngine,
-	unwrapValues bool,
+	wrapBehavior queries.WrapBehavior,
 ) {
 	if reh, ok := h.(ResultEvaluationHarness); ok {
-		reh.EvaluateQueryResults(t, expected, expectedCols, sch, rows, q, unwrapValues)
+		reh.EvaluateQueryResults(t, expected, expectedCols, sch, rows, q, wrapBehavior)
 	} else {
-		checkResultsDefault(t, expected, expectedCols, sch, rows, q, e, unwrapValues)
+		checkResultsDefault(t, expected, expectedCols, sch, rows, q, e, wrapBehavior)
 	}
 }
 
@@ -713,7 +713,7 @@ func checkResultsDefault(
 	rows []sql.Row,
 	q string,
 	e QueryEngine,
-	unwrapValues bool,
+	wrapBehavior queries.WrapBehavior,
 ) {
 	widenedRows := WidenRows(t, sch, rows)
 	widenedExpected := WidenRows(t, sch, expected)
@@ -751,11 +751,12 @@ func checkResultsDefault(
 					}
 				}
 			case sql.AnyWrapper:
-				if unwrapValues {
+				switch wrapBehavior {
+				case queries.WrapBehavior_Unwrap:
 					var err error
 					widenedRow[i], err = sql.UnwrapAny(context.Background(), v)
 					require.NoError(t, err)
-				} else {
+				case queries.WrapBehavior_Hash:
 					widenedRow[i] = v.Hash()
 				}
 			}
@@ -1198,12 +1199,12 @@ func RunWriteQueryTestWithEngine(t *testing.T, harness Harness, e QueryEngine, t
 	}
 
 	ctx := NewContext(harness)
-	testQueryWithContext(t, ctx, e, harness, tt.WriteQuery, tt.ExpectedWriteResult, nil, nil, nil, !tt.DontUnwrap)
+	testQueryWithContext(t, ctx, e, harness, tt.WriteQuery, tt.ExpectedWriteResult, nil, nil, nil, tt.WrapBehavior)
 	expectedSelect := tt.ExpectedSelect
 	if IsServerEngine(e) && tt.SkipServerEngine {
 		expectedSelect = nil
 	}
-	testQueryWithContext(t, ctx, e, harness, tt.SelectQuery, expectedSelect, nil, nil, nil, !tt.DontUnwrap)
+	testQueryWithContext(t, ctx, e, harness, tt.SelectQuery, expectedSelect, nil, nil, nil, tt.WrapBehavior)
 }
 
 func supportedDialect(harness Harness, dialect string) bool {

enginetest/harness.go

@@ -15,6 +15,7 @@
 package enginetest
 
 import (
+	"github.com/dolthub/go-mysql-server/enginetest/queries"
 	"testing"
 
 	"gopkg.in/src-d/go-errors.v1"
@@ -162,7 +163,7 @@ type ResultEvaluationHarness interface {
 		expectdSch sql.Schema,
 		actualRows []sql.Row,
 		query string,
-		unwrapValues bool,
+		wrapBehavior queries.WrapBehavior,
 	)
 
 	// EvaluateExpectedError compares expected error strings to actual errors and emits failed test assertions in the

enginetest/queries/queries.go

@@ -29,6 +29,16 @@ import (
 	"github.com/dolthub/go-mysql-server/sql/types"
 )
 
+// WrapBehavior determines how the test engine will process sql.AnyWrapper values in query results before comparing them to expected results.
+type WrapBehavior int
+
+const (
+	// WrapBehavior_Unwrap causes the engine to return the unwrapped value. Use this for tests that verify the semantic meaning of the result. (Most tests)
+	WrapBehavior_Unwrap = iota
+	// WrapBehavior_Hash causes the engine to return the result of the wrapper's Hash() function. Use this for tests that verify the specific representation of the result.
+	WrapBehavior_Hash
+)
+
 type QueryTest struct {
 	// Query is the query string to execute
 	Query string
@@ -47,10 +57,10 @@ type QueryTest struct {
 	// Dialect is the supported dialect for this query, which must match the dialect of the harness if specified.
 	// The query is skipped if the dialect doesn't match.
 	Dialect string
-	// DontUnwrap indicates whether to skip normalizing the select results via unwrapping wrapped values.
-	// Instead, the test engine will replace wrapped values with their hash (as determined by sql.AnyWrapped.Hash).
-	// Set this to test the exact encodings being returned by the query.
-	DontUnwrap bool
+	// WrapBehavior indicates whether to normalize the select results via unwrapping wrapped values (the default),
+	// or replace wrapped values with their hash as determined by sql.AnyWrapped.Hash.
+	// Set this to WrapBehvior_Hash to test the exact encodings being returned by the query.
+	WrapBehavior WrapBehavior
 }
 
 type QueryPlanTest struct {
@@ -11391,10 +11401,10 @@ type WriteQueryTest struct {
 	// Dialect is the supported dialect for this test, which must match the dialect of the harness if specified.
 	// The script is skipped if the dialect doesn't match.
 	Dialect string
-	// DontUnwrap indicates whether to skip normalizing the select results via unwrapping wrapped values.
-	// Instead, the test engine will replace wrapped values with their hash (as determined by sql.AnyWrapped.Hash).
-	// Set this to test the exact encodings being returned by the query.
-	DontUnwrap bool
+	// WrapBehavior indicates whether to normalize the select results via unwrapping wrapped values (the default),
+	// or replace wrapped values with their hash as determined by sql.AnyWrapped.Hash.
+	// Set this to WrapBehvior_Hash to test the exact encodings being returned by the query.
+	WrapBehavior WrapBehavior
 }
 
 // GenericErrorQueryTest is a query test that is used to assert an error occurs for some query, without specifying what

enginetest/wrapper_test.go

@@ -67,7 +67,7 @@ func (w ErrorWrapper[T]) IsExactLength() bool {
 }
 
 func (w ErrorWrapper[T]) Hash() interface{} {
-	return nil
+	panic("not implemented")
 }
 
 func setupWrapperTests(t *testing.T) (*sql.Context, *memory.Database, *MemoryHarness, *sqle.Engine) {

memory/wrapper_test.go

@@ -136,7 +136,7 @@ func (w ErrorWrapper[T]) IsExactLength() bool {
 }
 
 func (w ErrorWrapper[T]) Hash() interface{} {
-	return nil
+	panic("not implemented")
 }
 
 // TestWrapperCompare tests that a wrapped value can be used in comparisons.