From fee0d7ee92b6bf978322fe18180a9caf4331f370 Mon Sep 17 00:00:00 2001 From: AndrewSisley Date: Mon, 28 Oct 2024 10:20:46 -0400 Subject: [PATCH] test: Allow soft-referencing of Cids in tests (#3176) ## Relevant issue(s) Resolves #3172 ## Description Allows soft-referencing of Cids in tests. When encountering an unpleasant change-detector problem in https://github.com/sourcenetwork/defradb/pull/3173 I realised we had no need to reference Cids based on index, and we really only ever cared that they were valid, and either not the same as another cid, or the same. This change allows us to soft-reference them without ever really caring about where they came from. This allows us to give them descriptive ids, and avoids having to have the test framework from having to fetch them independently from the DB - something that raises questions over whether we are testing the test framework or the production code, especially in the change detector. The system can be extended to other types fairly easily. ## How has this been tested? I had a fairly long play around giving it bad ids in tests to make sure it would actually fail when appropriate. --- .../integration/query/commits/simple_test.go | 26 +++---- .../query/commits/with_delete_test.go | 7 ++ tests/integration/results.go | 77 +++++++++++++++++-- tests/integration/state.go | 4 + tests/integration/utils.go | 17 ++-- 5 files changed, 106 insertions(+), 25 deletions(-) diff --git a/tests/integration/query/commits/simple_test.go b/tests/integration/query/commits/simple_test.go index ca8c1e51b5..3660efdb87 100644 --- a/tests/integration/query/commits/simple_test.go +++ b/tests/integration/query/commits/simple_test.go @@ -37,13 +37,13 @@ func TestQueryCommits(t *testing.T) { Results: map[string]any{ "commits": []map[string]any{ { - "cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e", + "cid": testUtils.NewUniqueCid("name"), }, { - "cid": "bafyreigtnj6ntulcilkmin4pgukjwv3nwglqpiiyddz3dyfexdbltze7sy", + "cid": testUtils.NewUniqueCid("age"), }, { - "cid": "bafyreia2vlbfkcbyogdjzmbqcjneabwwwtw7ti2xbd7yor5mbu2sk4pcoy", + "cid": testUtils.NewUniqueCid("head"), }, }, }, @@ -364,7 +364,7 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { Results: map[string]any{ "commits": []map[string]any{ { - "cid": "bafyreih5h6i6ohfsgrcjtg76iarebqcurpaft73gpobl2z2cfsvihsgdqu", + "cid": testUtils.NewUniqueCid("age update"), "collectionID": int64(1), "delta": testUtils.CBORValue(22), "docID": "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3", @@ -373,13 +373,13 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { "height": int64(2), "links": []map[string]any{ { - "cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e", + "cid": testUtils.NewUniqueCid("age create"), "name": "_head", }, }, }, { - "cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e", + "cid": testUtils.NewUniqueCid("age create"), "collectionID": int64(1), "delta": testUtils.CBORValue(21), "docID": "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3", @@ -389,7 +389,7 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { "links": []map[string]any{}, }, { - "cid": "bafyreigtnj6ntulcilkmin4pgukjwv3nwglqpiiyddz3dyfexdbltze7sy", + "cid": testUtils.NewUniqueCid("name create"), "collectionID": int64(1), "delta": testUtils.CBORValue("John"), "docID": "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3", @@ -399,7 +399,7 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { "links": []map[string]any{}, }, { - "cid": "bafyreiale6qsjc7qewod3c6h2odwamfwcf7vt4zlqtw7ldcm57xdkgxja4", + "cid": testUtils.NewUniqueCid("update composite"), "collectionID": int64(1), "delta": nil, "docID": "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3", @@ -408,17 +408,17 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { "height": int64(2), "links": []map[string]any{ { - "cid": "bafyreia2vlbfkcbyogdjzmbqcjneabwwwtw7ti2xbd7yor5mbu2sk4pcoy", + "cid": testUtils.NewUniqueCid("create composite"), "name": "_head", }, { - "cid": "bafyreih5h6i6ohfsgrcjtg76iarebqcurpaft73gpobl2z2cfsvihsgdqu", + "cid": testUtils.NewUniqueCid("age update"), "name": "age", }, }, }, { - "cid": "bafyreia2vlbfkcbyogdjzmbqcjneabwwwtw7ti2xbd7yor5mbu2sk4pcoy", + "cid": testUtils.NewUniqueCid("create composite"), "collectionID": int64(1), "delta": nil, "docID": "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3", @@ -427,11 +427,11 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) { "height": int64(1), "links": []map[string]any{ { - "cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e", + "cid": testUtils.NewUniqueCid("age create"), "name": "age", }, { - "cid": "bafyreigtnj6ntulcilkmin4pgukjwv3nwglqpiiyddz3dyfexdbltze7sy", + "cid": testUtils.NewUniqueCid("name create"), "name": "name", }, }, diff --git a/tests/integration/query/commits/with_delete_test.go b/tests/integration/query/commits/with_delete_test.go index cd52f5d861..91b1125d6c 100644 --- a/tests/integration/query/commits/with_delete_test.go +++ b/tests/integration/query/commits/with_delete_test.go @@ -40,8 +40,10 @@ func TestQueryCommits_AfterDocDeletion_ShouldStillFetch(t *testing.T) { Request: ` query { commits(fieldId: "C") { + cid fieldName links { + cid name } } @@ -50,20 +52,25 @@ func TestQueryCommits_AfterDocDeletion_ShouldStillFetch(t *testing.T) { Results: map[string]any{ "commits": []map[string]any{ { + "cid": testUtils.NewUniqueCid("delete"), "fieldName": nil, "links": []map[string]any{ { + "cid": testUtils.NewUniqueCid("create composite"), "name": "_head", }, }, }, { + "cid": testUtils.NewUniqueCid("create composite"), "fieldName": nil, "links": []map[string]any{ { + "cid": testUtils.NewUniqueCid("create age"), "name": "age", }, { + "cid": testUtils.NewUniqueCid("create name"), "name": "name", }, }, diff --git a/tests/integration/results.go b/tests/integration/results.go index cc54565fe8..23435a3807 100644 --- a/tests/integration/results.go +++ b/tests/integration/results.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "github.com/ipfs/go-cid" "github.com/sourcenetwork/immutable" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,22 +24,37 @@ import ( "github.com/sourcenetwork/defradb/client" ) +// Validator instances can be substituted in place of concrete values +// and will be asserted on using their [Validate] function instead of +// asserting direct equality. +// +// They may mutate test state. +// +// Todo: This does not currently support chaining/nesting of Validators, +// although we would like that long term: +// https://github.com/sourcenetwork/defradb/issues/3189 +type Validator interface { + Validate(s *state, actualValue any, msgAndArgs ...any) +} + // AnyOf may be used as `Results` field where the value may // be one of several values, yet the value of that field must be the same // across all nodes due to strong eventual consistency. type AnyOf []any -// assertResultsAnyOf asserts that actual result is equal to at least one of the expected results. +var _ Validator = (AnyOf)(nil) + +// Validate asserts that actual result is equal to at least one of the expected results. // // The comparison is relaxed when using client types other than goClientType. -func assertResultsAnyOf(t testing.TB, client ClientType, expected AnyOf, actual any, msgAndArgs ...any) { - switch client { +func (a AnyOf) Validate(s *state, actualValue any, msgAndArgs ...any) { + switch s.clientType { case HTTPClientType, CLIClientType: - if !areResultsAnyOf(expected, actual) { - assert.Contains(t, expected, actual, msgAndArgs...) + if !areResultsAnyOf(a, actualValue) { + assert.Contains(s.t, a, actualValue, msgAndArgs...) } default: - assert.Contains(t, expected, actual, msgAndArgs...) + assert.Contains(s.t, a, actualValue, msgAndArgs...) } } @@ -68,6 +84,55 @@ func areResultsAnyOf(expected AnyOf, actual any) bool { return false } +// UniqueCid allows the referencing of Cids by an arbitrary test-defined ID. +// +// Instead of asserting on a specific Cid value, this type will assert that +// no other [UniqueCid]s with different [ID]s has the first Cid value that this instance +// describes. +// +// It will also ensure that all Cids described by this [UniqueCid] have the same +// valid, Cid value. +type UniqueCid struct { + // ID is the arbitrary, but hopefully descriptive, id of this [UniqueCid]. + ID any +} + +var _ Validator = (*UniqueCid)(nil) + +// NewUniqueCid creates a new [UniqueCid] of the given arbitrary, but hopefully descriptive, +// id. +// +// All results described by [UniqueCid]s with the given id must have the same valid Cid value. +// No other [UniqueCid] ids may describe the same Cid value. +func NewUniqueCid(id any) *UniqueCid { + return &UniqueCid{ + ID: id, + } +} + +func (ucid *UniqueCid) Validate(s *state, actualValue any, msgAndArgs ...any) { + isNew := true + for id, value := range s.cids { + if id == ucid.ID { + require.Equal(s.t, value, actualValue) + isNew = false + } else { + require.NotEqual(s.t, value, actualValue, "UniqueCid must be unique!", msgAndArgs) + } + } + + if isNew { + require.IsType(s.t, "", actualValue) + + cid, err := cid.Decode(actualValue.(string)) + if err != nil { + require.NoError(s.t, err) + } + + s.cids[ucid.ID] = cid.String() + } +} + // areResultsEqual returns true if the expected and actual results are of equal value. // // Values of type json.Number and immutable.Option will be reduced to their underlying types. diff --git a/tests/integration/state.go b/tests/integration/state.go index 77fe2e52cd..b4a3777d03 100644 --- a/tests/integration/state.go +++ b/tests/integration/state.go @@ -181,6 +181,9 @@ type state struct { // nodes. docIDs [][]client.DocID + // Valid Cid string values by [UniqueCid] ID. + cids map[any]string + // Indexes, by index, by collection index, by node index. indexes [][][]client.IndexDescription @@ -225,6 +228,7 @@ func newState( collectionNames: collectionNames, collectionIndexesByRoot: map[uint32]int{}, docIDs: [][]client.DocID{}, + cids: map[any]string{}, indexes: [][][]client.IndexDescription{}, isBench: false, } diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 3bf34d1138..aff1ebecb7 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -2020,8 +2020,10 @@ func assertRequestResults( actualDocs, stack, ) - case AnyOf: - assertResultsAnyOf(s.t, s.clientType, exp, actual) + + case Validator: + exp.Validate(s, actual) + default: assertResultsEqual( s.t, @@ -2065,9 +2067,12 @@ func assertRequestResultDocs( for field, actualValue := range actualDoc { stack.pushMap(field) + pathInfo := fmt.Sprintf("node: %v, path: %s", nodeID, stack) + switch expectedValue := expectedDoc[field].(type) { - case AnyOf: - assertResultsAnyOf(s.t, s.clientType, expectedValue, actualValue) + case Validator: + expectedValue.Validate(s, actualValue, pathInfo) + case DocIndex: expectedDocID := s.docIDs[expectedValue.CollectionIndex][expectedValue.Index].String() assertResultsEqual( @@ -2075,7 +2080,7 @@ func assertRequestResultDocs( s.clientType, expectedDocID, actualValue, - fmt.Sprintf("node: %v, path: %s", nodeID, stack), + pathInfo, ) case []map[string]any: actualValueMap := ConvertToArrayOfMaps(s.t, actualValue) @@ -2094,7 +2099,7 @@ func assertRequestResultDocs( s.clientType, expectedValue, actualValue, - fmt.Sprintf("node: %v, path: %s", nodeID, stack), + pathInfo, ) } stack.pop()