From ebad3eb5466f7d177e2f9282af29eeda5eeaeb97 Mon Sep 17 00:00:00 2001 From: AndrewSisley Date: Thu, 7 Sep 2023 11:46:30 -0400 Subject: [PATCH] test(i): Skip unsupported mutation types at test level (#1850) ## Relevant issue(s) Resolves #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. --- .../update/simple/with_deleted_test.go | 8 +++---- .../mutation/create/simple_test.go | 12 +++++----- .../update/field_kinds/array_bool_test.go | 22 +++++++++---------- .../update/field_kinds/array_float_test.go | 22 +++++++++---------- .../update/field_kinds/array_int_test.go | 22 +++++++++---------- .../update/field_kinds/array_string_test.go | 22 +++++++++---------- tests/integration/test_case.go | 21 ++++++------------ tests/integration/utils2.go | 6 ++--- 8 files changed, 63 insertions(+), 72 deletions(-) diff --git a/tests/integration/collection/update/simple/with_deleted_test.go b/tests/integration/collection/update/simple/with_deleted_test.go index 7becd49bef..444d16f87c 100644 --- 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 index dd2f9d509e..6961895426 100644 --- 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 index bbc6938c28..831c47c6cb 100644 --- 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 index 0889c12be9..255d5f8538 100644 --- 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 index 0ce79d44dc..7278b15e23 100644 --- 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 index 74c9055ff2..b15003072a 100644 --- 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 index f59f3bfc3c..e17adfdeaa 100644 --- 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 index d8da1e54dd..5c42f7da12 100644 --- 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 {