pr: Adhere - Apply the code review suggestions.

For example:
- Fail if given an empty request for a non-transactional test.

One additional thing I crammed into this commit was to handle a list of
map to also be copied while copying a map.

tests/integration/explain/simple/utils.go

@@ -57,7 +57,7 @@ var bookAuthorGQLSchema = (`
 
 `)
 
-// TODO: Remove after draft has received feedback.
+// TODO: This should be resolved in ISSUE#953 (github.com/sourcenetwork/defradb).
 func executeTestCase(t *testing.T, test testUtils.QueryTestCase) {
 	testUtils.ExecuteQueryTestCase(
 		t,

tests/integration/explain/utils.go

@@ -15,11 +15,10 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/sourcenetwork/immutable"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/sourcenetwork/immutable"
-
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/logging"
 
@@ -132,7 +131,7 @@ func ExecuteExplainRequestTestCase(
 	if testUtils.AssertError(t, explainTest.Description, err, explainTest.ExpectedError) {
 		return
 	}
-	assert.NotEmpty(t, dbs)
+	require.NotEmpty(t, dbs)
 
 	for _, dbi := range dbs {
 		log.Info(ctx, explainTest.Description, logging.NewKV("Database", dbi.Name()))
@@ -152,9 +151,9 @@ func ExecuteExplainRequestTestCase(
 				)
 				dbi.DB().Close(ctx)
 				return
-			} else {
-				dbi = testUtils.SetupDatabaseUsingTargetBranch(ctx, t, dbi, collectionNames)
 			}
+
+			dbi = testUtils.SetupDatabaseUsingTargetBranch(ctx, t, dbi, collectionNames)
 		} else {
 			testUtils.SetupDatabase(
 				ctx,
@@ -170,7 +169,7 @@ func ExecuteExplainRequestTestCase(
 		}
 
 		result := dbi.DB().ExecQuery(ctx, explainTest.Request)
-		if assertExplainRequestTestCaseWithActualResult(
+		if assertExplainRequestResults(
 			ctx,
 			t,
 			&result.GQL,
@@ -187,13 +186,13 @@ func ExecuteExplainRequestTestCase(
 	}
 }
 
-func assertExplainRequestTestCaseWithActualResult(
+func assertExplainRequestResults(
 	ctx context.Context,
 	t *testing.T,
 	actualResult *client.GQLResult,
 	explainTest ExplainRequestTestCase,
 ) bool {
-	// 1) Check expected error matches actual error.
+	// Check expected error matches actual error.
 	if testUtils.AssertErrors(
 		t,
 		explainTest.Description,
@@ -207,8 +206,8 @@ func assertExplainRequestTestCaseWithActualResult(
 	resultantData := actualResult.Data.([]map[string]any)
 	log.Info(ctx, "", logging.NewKV("FullExplainGraphResult", actualResult.Data))
 
-	// 2) Check if the expected full explain graph (if provided) matches the actual full explain graph
-	//    that is returned, if doesn't match we would like to still see a diff comparison (handy while debugging).
+	// Check if the expected full explain graph (if provided) matches the actual full explain graph
+	// that is returned, if doesn't match we would like to still see a diff comparison (handy while debugging).
 	if lengthOfExpectedFullGraph := len(explainTest.ExpectedFullGraph); explainTest.ExpectedFullGraph != nil {
 		require.Equal(t, lengthOfExpectedFullGraph, len(resultantData), explainTest.Description)
 		for index, actualResult := range resultantData {
@@ -223,8 +222,8 @@ func assertExplainRequestTestCaseWithActualResult(
 		}
 	}
 
-	// 3) Ensure the complete high-level pattern matches, inother words check that all the
-	//    explain graph nodes are in the correct expected ordering.
+	// Ensure the complete high-level pattern matches, inother words check that all the
+	// explain graph nodes are in the correct expected ordering.
 	if explainTest.ExpectedPatterns != nil {
 		require.Equal(t, len(explainTest.ExpectedPatterns), len(resultantData), explainTest.Description)
 		for index, actualResult := range resultantData {
@@ -239,8 +238,8 @@ func assertExplainRequestTestCaseWithActualResult(
 		}
 	}
 
-	// 4) Match the targeted node's attributes (subset assertions), with the expected attributes.
-	//    Note: This does not check if the node is in correct location or not.
+	// Match the targeted node's attributes (subset assertions), with the expected attributes.
+	// Note: This does not check if the node is in correct location or not.
 	if explainTest.ExpectedTargets != nil {
 		for _, target := range explainTest.ExpectedTargets {
 			assertExplainTargetCase(t, explainTest.Description, target, resultantData)
@@ -372,12 +371,12 @@ func findTargetNode(
 // trimSubNodes returns a graph where all the immediate sub nodes are trimmed (i.e. no nested subnodes remain).
 func trimSubNodes(graph any) any {
 	checkGraph, ok := graph.(map[string]any)
-	trimGraph := copyMap(checkGraph)
-
 	if !ok {
 		return graph
 	}
 
+	// Copying is super important here so we don't trim the actual result (as we might want to continue using it),
+	trimGraph := copyMap(checkGraph)
 	for key := range trimGraph {
 		if isPlanNode(key) {
 			delete(trimGraph, key)
@@ -439,6 +438,14 @@ func copyMap(originalMap map[string]any) map[string]any {
 		switch v := oValue.(type) {
 		case map[string]any:
 			newMap[oKey] = copyMap(v)
+
+		case []map[string]any:
+			newList := make([]map[string]any, len(v))
+			for index, item := range v {
+				newList[index] = copyMap(item)
+			}
+			newMap[oKey] = newList
+
 		default:
 			newMap[oKey] = oValue
 		}

tests/integration/utils.go

@@ -25,9 +25,9 @@ import (
 
 	badger "github.com/dgraph-io/badger/v3"
 	ds "github.com/ipfs/go-datastore"
-	"github.com/stretchr/testify/assert"
-
 	"github.com/sourcenetwork/immutable"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/datastore"
@@ -339,21 +339,23 @@ func ExecuteQueryTestCase(
 	collectionNames []string,
 	test QueryTestCase,
 ) {
-	isTransactional := false
-	if len(test.TransactionalQueries) > 0 {
-		isTransactional = true
-	}
+	isTransactional := len(test.TransactionalQueries) > 0
 
 	if DetectDbChanges && DetectDbChangesPreTestChecks(t, collectionNames, isTransactional) {
 		return
 	}
 
+	// Must have a non-empty request.
+	if !isTransactional && test.Query == "" {
+		assert.Fail(t, "Test must have a non-empty request.", test.Description)
+	}
+
 	ctx := context.Background()
 	dbs, err := GetDatabases(ctx, t, test.DisableMapStore)
 	if AssertError(t, test.Description, err, test.ExpectedError) {
 		return
 	}
-	assert.NotEmpty(t, dbs)
+	require.NotEmpty(t, dbs)
 
 	for _, dbi := range dbs {
 		log.Info(ctx, test.Description, logging.NewKV("Database", dbi.name))
@@ -373,9 +375,9 @@ func ExecuteQueryTestCase(
 				)
 				dbi.db.Close(ctx)
 				return
-			} else {
-				dbi = SetupDatabaseUsingTargetBranch(ctx, t, dbi, collectionNames)
 			}
+
+			dbi = SetupDatabaseUsingTargetBranch(ctx, t, dbi, collectionNames)
 		} else {
 			SetupDatabase(
 				ctx,
@@ -445,7 +447,7 @@ func ExecuteQueryTestCase(
 
 		// We run the core query after the explicitly transactional ones to permit tests to query
 		//  the commited result of the transactional queries
-		if test.Query != "" {
+		if !isTransactional || (isTransactional && test.Query != "") {
 			result := dbi.db.ExecQuery(ctx, test.Query)
 			if result.Pub != nil {
 				for _, q := range test.PostSubscriptionQueries {