test(i): Skip unsupported mutation types at test level (sourcenetwork…

…#1850) ## Relevant issue(s) Resolves sourcenetwork#1849 ## Description Skips unsupported mutation types at test level. Earlier version caused the change detector to fail, as the skip was only executed in the setup stage, not the main/assert stage resulting in test failures. I also think this location makes much more sense, as the skip is a test-level thing, not something that acts at the action-level.
AndrewSisley · Sep 7, 2023 · ebad3eb · ebad3eb
1 parent 8b10d0b
commit ebad3eb
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 72 deletions.
diff --git a/tests/integration/collection/update/simple/with_deleted_test.go b/tests/integration/collection/update/simple/with_deleted_test.go
@@ -21,6 +21,10 @@ import (
 func TestUpdateSave_DeletedDoc_DoesNothing(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Save existing, deleted document",
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			// We only wish to test collection.Save in this test.
+			testUtils.CollectionSaveMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -43,10 +47,6 @@ func TestUpdateSave_DeletedDoc_DoesNothing(t *testing.T) {
 					"name": "Fred"
 				}`,
 				ExpectedError: "a document with the given dockey has been deleted",
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					// We only wish to test collection.Save in this test.
-					testUtils.CollectionSaveMutationType,
-				}),
 			},
 		},
 	}

diff --git a/tests/integration/mutation/create/simple_test.go b/tests/integration/mutation/create/simple_test.go
@@ -90,6 +90,12 @@ func TestMutationCreate(t *testing.T) {
 func TestMutationCreate_GivenDuplicate_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple create mutation where document already exists.",
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			// Collection.Save would treat the second create as an update, and so
+			// is excluded from this test.
+			testUtils.CollectionNamedMutationType,
+			testUtils.GQLRequestMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -110,12 +116,6 @@ func TestMutationCreate_GivenDuplicate_Errors(t *testing.T) {
 					"name": "John",
 					"age": 27
 				}`,
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					// Collection.Save would treat the second create as an update, and so
-					// is excluded from this test.
-					testUtils.CollectionNamedMutationType,
-					testUtils.GQLRequestMutationType,
-				}),
 				ExpectedError: "a document with the given dockey already exists.",
 			},
 		},

diff --git a/tests/integration/mutation/update/field_kinds/array_bool_test.go b/tests/integration/mutation/update/field_kinds/array_bool_test.go
@@ -21,6 +21,11 @@ import (
 func TestMutationUpdate_WithArrayOfBooleansToNil(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with boolean array, replace with nil",
+		// This restriction should be removed when we can, it is here because of
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.GQLRequestMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -40,11 +45,6 @@ func TestMutationUpdate_WithArrayOfBooleansToNil(t *testing.T) {
 				Doc: `{
 					"likedIndexes": null
 				}`,
-				// This restriction should be removed when we can, it is here because of
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.GQLRequestMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `
@@ -69,6 +69,12 @@ func TestMutationUpdate_WithArrayOfBooleansToNil(t *testing.T) {
 func TestMutationUpdate_WithArrayOfBooleansToNil_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with boolean array, replace with nil",
+		// This is a bug, this test should be removed in
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.CollectionNamedMutationType,
+			testUtils.CollectionSaveMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -88,12 +94,6 @@ func TestMutationUpdate_WithArrayOfBooleansToNil_Errors(t *testing.T) {
 				Doc: `{
 					"likedIndexes": null
 				}`,
-				// This is a bug, this test should be removed in
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.CollectionNamedMutationType,
-					testUtils.CollectionSaveMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `

diff --git a/tests/integration/mutation/update/field_kinds/array_float_test.go b/tests/integration/mutation/update/field_kinds/array_float_test.go
@@ -21,6 +21,11 @@ import (
 func TestMutationUpdate_WithArrayOfFloatsToNil(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with float array, replace with nil",
+		// This restriction should be removed when we can, it is here because of
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.GQLRequestMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -40,11 +45,6 @@ func TestMutationUpdate_WithArrayOfFloatsToNil(t *testing.T) {
 				Doc: `{
 					"favouriteFloats": null
 				}`,
-				// This restriction should be removed when we can, it is here because of
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.GQLRequestMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `
@@ -69,6 +69,12 @@ func TestMutationUpdate_WithArrayOfFloatsToNil(t *testing.T) {
 func TestMutationUpdate_WithArrayOfFloatsToNil_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with float array, replace with nil",
+		// This is a bug, this test should be removed in
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.CollectionNamedMutationType,
+			testUtils.CollectionSaveMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -88,12 +94,6 @@ func TestMutationUpdate_WithArrayOfFloatsToNil_Errors(t *testing.T) {
 				Doc: `{
 					"favouriteFloats": null
 				}`,
-				// This is a bug, this test should be removed in
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.CollectionNamedMutationType,
-					testUtils.CollectionSaveMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `

diff --git a/tests/integration/mutation/update/field_kinds/array_int_test.go b/tests/integration/mutation/update/field_kinds/array_int_test.go
@@ -21,6 +21,11 @@ import (
 func TestMutationUpdate_WithArrayOfIntsToNil(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with integer array, replace with nil",
+		// This restriction should be removed when we can, it is here because of
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.GQLRequestMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -40,11 +45,6 @@ func TestMutationUpdate_WithArrayOfIntsToNil(t *testing.T) {
 				Doc: `{
 					"favouriteIntegers": null
 				}`,
-				// This restriction should be removed when we can, it is here because of
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.GQLRequestMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `
@@ -69,6 +69,12 @@ func TestMutationUpdate_WithArrayOfIntsToNil(t *testing.T) {
 func TestMutationUpdate_WithArrayOfIntsToNil_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with integer array, replace with nil",
+		// This is a bug, this test should be removed in
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.CollectionNamedMutationType,
+			testUtils.CollectionSaveMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -88,12 +94,6 @@ func TestMutationUpdate_WithArrayOfIntsToNil_Errors(t *testing.T) {
 				Doc: `{
 					"favouriteIntegers": null
 				}`,
-				// This is a bug, this test should be removed in
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.CollectionNamedMutationType,
-					testUtils.CollectionSaveMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `

diff --git a/tests/integration/mutation/update/field_kinds/array_string_test.go b/tests/integration/mutation/update/field_kinds/array_string_test.go
@@ -21,6 +21,11 @@ import (
 func TestMutationUpdate_WithArrayOfStringsToNil(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with string array, replace with nil",
+		// This restriction should be removed when we can, it is here because of
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.GQLRequestMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -40,11 +45,6 @@ func TestMutationUpdate_WithArrayOfStringsToNil(t *testing.T) {
 				Doc: `{
 					"preferredStrings": null
 				}`,
-				// This restriction should be removed when we can, it is here because of
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.GQLRequestMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `
@@ -69,6 +69,12 @@ func TestMutationUpdate_WithArrayOfStringsToNil(t *testing.T) {
 func TestMutationUpdate_WithArrayOfStringsToNil_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Simple update mutation with string array, replace with nil",
+		// This is a bug, this test should be removed in
+		// https://github.com/sourcenetwork/defradb/issues/1842
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.CollectionNamedMutationType,
+			testUtils.CollectionSaveMutationType,
+		}),
 		Actions: []any{
 			testUtils.SchemaUpdate{
 				Schema: `
@@ -88,12 +94,6 @@ func TestMutationUpdate_WithArrayOfStringsToNil_Errors(t *testing.T) {
 				Doc: `{
 					"preferredStrings": null
 				}`,
-				// This is a bug, this test should be removed in
-				// https://github.com/sourcenetwork/defradb/issues/1842
-				SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
-					testUtils.CollectionNamedMutationType,
-					testUtils.CollectionSaveMutationType,
-				}),
 			},
 			testUtils.Request{
 				Request: `

diff --git a/tests/integration/test_case.go b/tests/integration/test_case.go
@@ -26,6 +26,13 @@ type TestCase struct {
 	// this test should execute.  They will execute in the order that they
 	// are provided.
 	Actions []any
+
+	// If provided a value, SupportedMutationTypes will cause this test to be skipped
+	// if the active mutation type is not within the given set.
+	//
+	// This is to only be used in the very rare cases where we really do want behavioural
+	// differences between mutation types, or we need to temporarily document a bug.
+	SupportedMutationTypes immutable.Option[[]MutationType]
 }
 
 // SetupComplete is a flag to explicitly notify the change detector at which point
@@ -97,13 +104,6 @@ type CreateDoc struct {
 	// String can be a partial, and the test will pass if an error is returned that
 	// contains this string.
 	ExpectedError string
-
-	// If provided a value, SupportedMutationTypes will cause this test to be skipped
-	// if the active mutation type is not within the given set.
-	//
-	// This is to only be used in the very rare cases where we really do want behavioural
-	// differences between mutation types, or we need to temporarily document a bug.
-	SupportedMutationTypes immutable.Option[[]MutationType]
 }
 
 // DeleteDoc will attempt to delete the given document in the given collection
@@ -159,13 +159,6 @@ type UpdateDoc struct {
 
 	// Setting DontSync to true will prevent waiting for that update.
 	DontSync bool
-
-	// If provided a value, SupportedMutationTypes will cause this test to be skipped
-	// if the active mutation type is not within the given set.
-	//
-	// This is to only be used in the very rare cases where we really do want behavioural
-	// differences between mutation types, or we need to temporarily document a bug.
-	SupportedMutationTypes immutable.Option[[]MutationType]
 }
 
 // CreateIndex will attempt to create the given secondary index for the given collection

diff --git a/tests/integration/utils2.go b/tests/integration/utils2.go
@@ -317,6 +317,8 @@ func ExecuteTestCase(
 		return
 	}
 
+	skipIfMutationTypeUnsupported(t, testCase.SupportedMutationTypes)
+
 	ctx := context.Background()
 	dbts := GetDatabaseTypes()
 	// Assert that this is not empty to protect against accidental mis-configurations,
@@ -1041,8 +1043,6 @@ func createDoc(
 	s *state,
 	action CreateDoc,
 ) {
-	skipIfMutationTypeUnsupported(s.t, action.SupportedMutationTypes)
-
 	var mutation func(*state, CreateDoc, *net.Node, []client.Collection) (*client.Document, error)
 
 	switch mutationType {
@@ -1184,8 +1184,6 @@ func updateDoc(
 	s *state,
 	action UpdateDoc,
 ) {
-	skipIfMutationTypeUnsupported(s.t, action.SupportedMutationTypes)
-
 	var mutation func(*state, UpdateDoc, *net.Node, []client.Collection) error
 
 	switch mutationType {