PR FIXUP - Assert expected error raised

Also converts one of the transaction tests, as the old system does not specify which action should expect an error, just that one action should raise it.  That is not particularly great in my opinion and this commit changes the new system so that as well as tying the error to a particular action (as it did before), actions declared after an action with an expected action will still be executed - the old system would instead exit early, the request in the old version of the converted test was never actually executed.

tests/integration/mutation/simple/mix/with_txn_test.go

@@ -214,18 +214,17 @@ func TestMutationWithTxnDoesNotUpdateUserGivenDifferentTransactions(t *testing.T
 }
 
 func TestMutationWithTxnDoesNotAllowUpdateInSecondTransactionUser(t *testing.T) {
-	test := testUtils.RequestTestCase{
+	test := testUtils.TestCase{
 		Description: "Update by two different transactions",
-		Docs: map[int][]string{
-			0: {
-				`{
+		Actions: []any{
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `{
 					"name": "John",
 					"age": 27
 				}`,
 			},
-		},
-		TransactionalRequests: []testUtils.TransactionRequest{
-			{
+			testUtils.TransactionRequest2{
 				TransactionId: 0,
 				Request: `mutation {
 					update_user(data: "{\"age\": 28}") {
@@ -242,7 +241,7 @@ func TestMutationWithTxnDoesNotAllowUpdateInSecondTransactionUser(t *testing.T)
 					},
 				},
 			},
-			{
+			testUtils.TransactionRequest2{
 				TransactionId: 1,
 				Request: `mutation {
 					update_user(data: "{\"age\": 29}") {
@@ -258,25 +257,33 @@ func TestMutationWithTxnDoesNotAllowUpdateInSecondTransactionUser(t *testing.T)
 						"age":  uint64(29),
 					},
 				},
+			},
+			testUtils.TransactionCommit{
+				TransactionId: 0,
+			},
+			testUtils.TransactionCommit{
+				TransactionId: 1,
 				ExpectedError: "Transaction Conflict. Please retry",
 			},
-		},
-		// Query after transactions have been commited:
-		Request: `query {
-			user {
-				_key
-				name
-				age
-			}
-		}`,
-		Results: []map[string]any{
-			{
-				"_key": "bae-88b63198-7d38-5714-a9ff-21ba46374fd1",
-				"name": "John",
-				"age":  uint64(28),
+			testUtils.Request{
+				// Query after transactions have been commited:
+				Request: `query {
+					user {
+						_key
+						name
+						age
+					}
+				}`,
+				Results: []map[string]any{
+					{
+						"_key": "bae-88b63198-7d38-5714-a9ff-21ba46374fd1",
+						"name": "John",
+						"age":  uint64(28),
+					},
+				},
 			},
 		},
 	}
 
-	simpleTests.ExecuteTestCase(t, test)
+	simpleTests.Execute(t, test)
 }

tests/integration/mutation/simple/utils.go

@@ -28,3 +28,21 @@ var userSchema = (`
 func ExecuteTestCase(t *testing.T, test testUtils.RequestTestCase) {
 	testUtils.ExecuteRequestTestCase(t, userSchema, []string{"user"}, test)
 }
+
+func Execute(t *testing.T, test testUtils.TestCase) {
+	testUtils.ExecuteTestCase(
+		t,
+		[]string{"user"},
+		testUtils.TestCase{
+			Description: test.Description,
+			Actions: append(
+				[]any{
+					testUtils.SchemaUpdate{
+						Schema: userSchema,
+					},
+				},
+				test.Actions...,
+			),
+		},
+	)
+}

tests/integration/utils2.go

@@ -322,31 +322,21 @@ func executeTestCase(
 	for i := startActionIndex; i <= endActionIndex; i++ {
 		switch action := testCase.Actions[i].(type) {
 		case SchemaUpdate:
-			if updateSchema(ctx, t, dbi.db, testCase, action) {
-				return
-			}
+			updateSchema(ctx, t, dbi.db, testCase, action)
 			// If the schema was updated we need to refresh the collection definitions.
 			collections = getCollections(ctx, t, dbi.db, collectionNames)
 
 		case CreateDoc:
-			if documents, done = createDoc(ctx, t, testCase, collections, documents, action); done {
-				return
-			}
+			documents = createDoc(ctx, t, testCase, collections, documents, action)
 
 		case UpdateDoc:
-			if updateDoc(ctx, t, testCase, collections, documents, action) {
-				return
-			}
+			updateDoc(ctx, t, testCase, collections, documents, action)
 
 		case TransactionRequest2:
-			if txns, done = executeTransactionRequest(ctx, t, dbi.db, txns, testCase, action); done {
-				return
-			}
+			txns = executeTransactionRequest(ctx, t, dbi.db, txns, testCase, action)
 
 		case TransactionCommit:
-			if commitTransaction(ctx, t, txns, testCase, action) {
-				return
-			}
+			commitTransaction(ctx, t, txns, testCase, action)
 
 		case SubscriptionRequest:
 			var resultsChan subscriptionResult
@@ -357,9 +347,7 @@ func executeTestCase(
 			resultsChans = append(resultsChans, resultsChan)
 
 		case Request:
-			if executeRequest(ctx, t, dbi.db, testCase, action) {
-				return
-			}
+			executeRequest(ctx, t, dbi.db, testCase, action)
 
 		case SetupComplete:
 			// no-op, just continue.
@@ -482,9 +470,11 @@ func updateSchema(
 	db client.DB,
 	testCase TestCase,
 	action SchemaUpdate,
-) bool {
+) {
 	err := db.AddSchema(ctx, action.Schema)
-	return AssertError(t, testCase.Description, err, action.ExpectedError)
+	expectedErrorRaised := AssertError(t, testCase.Description, err, action.ExpectedError)
+
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
 }
 
 // createDoc creates a document using the collection api and caches it in the
@@ -496,24 +486,27 @@ func createDoc(
 	collections []client.Collection,
 	documents [][]*client.Document,
 	action CreateDoc,
-) ([][]*client.Document, bool) {
+) [][]*client.Document {
 	doc, err := client.NewDocFromJSON([]byte(action.Doc))
 	if AssertError(t, testCase.Description, err, action.ExpectedError) {
-		return nil, true
+		return nil
 	}
 
 	err = collections[action.CollectionID].Save(ctx, doc)
-	if AssertError(t, testCase.Description, err, action.ExpectedError) {
-		return nil, true
+	expectedErrorRaised := AssertError(t, testCase.Description, err, action.ExpectedError)
+	if expectedErrorRaised {
+		return nil
 	}
 
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
+
 	if action.CollectionID >= len(documents) {
 		// Expand the slice if required, so that the document can be accessed by collection index
 		documents = append(documents, make([][]*client.Document, action.CollectionID-len(documents)+1)...)
 	}
 	documents[action.CollectionID] = append(documents[action.CollectionID], doc)
 
-	return documents, false
+	return documents
 }
 
 // updateDoc updates a document using the collection api.
@@ -524,16 +517,18 @@ func updateDoc(
 	collections []client.Collection,
 	documents [][]*client.Document,
 	action UpdateDoc,
-) bool {
+) {
 	doc := documents[action.CollectionID][action.DocID]
 
 	err := doc.SetWithJSON([]byte(action.Doc))
 	if AssertError(t, testCase.Description, err, action.ExpectedError) {
-		return true
+		return
 	}
 
 	err = collections[action.CollectionID].Save(ctx, doc)
-	return AssertError(t, testCase.Description, err, action.ExpectedError)
+	expectedErrorRaised := AssertError(t, testCase.Description, err, action.ExpectedError)
+
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
 }
 
 // executeTransactionRequest executes the given transactional request.
@@ -548,7 +543,7 @@ func executeTransactionRequest(
 	txns []datastore.Txn,
 	testCase TestCase,
 	action TransactionRequest2,
-) ([]datastore.Txn, bool) {
+) []datastore.Txn {
 	if action.TransactionId >= len(txns) {
 		// Extend the txn slice so this txn can fit and be accessed by TransactionId
 		txns = append(txns, make([]datastore.Txn, action.TransactionId-len(txns)+1)...)
@@ -559,14 +554,14 @@ func executeTransactionRequest(
 		txn, err := db.NewTxn(ctx, false)
 		if AssertError(t, testCase.Description, err, action.ExpectedError) {
 			txn.Discard(ctx)
-			return nil, true
+			return nil
 		}
 
 		txns[action.TransactionId] = txn
 	}
 
 	result := db.ExecTransactionalRequest(ctx, action.Request, txns[action.TransactionId])
-	done := assertRequestResults(
+	expectedErrorRaised := assertRequestResults(
 		ctx,
 		t,
 		testCase.Description,
@@ -575,14 +570,16 @@ func executeTransactionRequest(
 		action.ExpectedError,
 	)
 
-	if done {
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
+
+	if expectedErrorRaised {
 		// Make sure to discard the transaction before exit, else an unwanted error
 		// may surface later (e.g. on database close).
 		txns[action.TransactionId].Discard(ctx)
-		return nil, true
+		return nil
 	}
 
-	return txns, false
+	return txns
 }
 
 // commitTransaction commits the given transaction.
@@ -595,13 +592,15 @@ func commitTransaction(
 	txns []datastore.Txn,
 	testCase TestCase,
 	action TransactionCommit,
-) bool {
+) {
 	err := txns[action.TransactionId].Commit(ctx)
 	if err != nil {
 		txns[action.TransactionId].Discard(ctx)
 	}
 
-	return AssertError(t, testCase.Description, err, action.ExpectedError)
+	expectedErrorRaised := AssertError(t, testCase.Description, err, action.ExpectedError)
+
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
 }
 
 // executeRequest executes the given request.
@@ -611,16 +610,18 @@ func executeRequest(
 	db client.DB,
 	testCase TestCase,
 	action Request,
-) bool {
+) {
 	result := db.ExecRequest(ctx, action.Request)
-	return assertRequestResults(
+	expectedErrorRaised := assertRequestResults(
 		ctx,
 		t,
 		testCase.Description,
 		&result.GQL,
 		action.Results,
 		action.ExpectedError,
 	)
+
+	assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
 }
 
 // subscriptionResult wraps details required to assert that the
@@ -679,16 +680,19 @@ func executeSubscriptionRequest(
 				resultChan.subscriptionAssert <- func() {
 					// This assert should be executed from the main test routine
 					// so that failures will be properly handled.
-					assertRequestResults(
+					expectedErrorRaised := assertRequestResults(
 						ctx,
 						t,
 						testCase.Description,
 						finalResult,
 						action.Results,
 						action.ExpectedError,
 					)
+
+					assertExpectedErrorRaised(t, testCase.Description, action.ExpectedError, expectedErrorRaised)
 				}
 
+				result.Pub.Unsubscribe()
 				return
 			}
 		}
@@ -775,3 +779,9 @@ func assertRequestResults(
 
 	return false
 }
+
+func assertExpectedErrorRaised(t *testing.T, description string, expectedError string, wasRaised bool) {
+	if expectedError != "" && !wasRaised {
+		assert.Fail(t, "Expected an error however none was raised.", description)
+	}
+}