Inconsistent handling of cypher parameters in cypher tests

[johnymontana]

Discussion from PR #255:

test/helpers/cypherTestHelpers.js const checkCypherMutation = (object, params, ctx, resolveInfo) => { const [query, queryParams] = cypherMutation(params, ctx, resolveInfo); t.is(query, expectedCypherQuery); t.deepEqual(queryParams, expectedCypherParams); @roschaefer roschaefer on May 24 Author Contributor @johnymontana @michaeldgraham can you tell me where expectedCypherParams comes from? The function augmentedSchemaCypherTestRunner does not expect a positional parameter on line 135, unlike cypherTestRunner. When I add the parameter, I see a lot of failing tests.

Is it possible that we have a lot of false negatives here? Some of the tests pass in a parameter, which apparently does not even get checked?

@roschaefer roschaefer on May 24 Author Contributor Ok, I was able to fix one test case. In all test cases where augmentedSchemaCypherTestRunner is used, we should pass in expectedCypherParams.

Before

test('simple Cypher query', t => { const graphQLQuery = { Movie(title: "River Runs Through It, A") { title } }, expectedCypherQuery = MATCH (\movie`:`Movie` {title:$title}) RETURN `movie` { .title } AS `movie``;

t.plan(3); return Promise.all([ cypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery, { title: 'River Runs Through It, A', first: -1, cypherParams: CYPHER_PARAMS, offset: 0 }), augmentedSchemaCypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery) ]); }); After

test('Simple Cypher query', t => { const graphQLQuery = { Movie(title: "River Runs Through It, A") { title } }, expectedCypherQuery = MATCH (\movie`:`Movie` {title:$title}) RETURN `movie` { .title } AS `movie``, expectedCypherParams = { title: 'River Runs Through It, A', first: -1, cypherParams: CYPHER_PARAMS, offset: 0 }

t.plan(4); return Promise.all([ cypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery, expectedCypherParams), augmentedSchemaCypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery, expectedCypherParams) ]); }); This is some really tedious typing work, so I would like to get some feedback before I continue. It seems that right now cypherTestRunner is always used before augmentedSchemaCypherTestRunner. I believe that's the reason why expectedCypherParams is not undefined.

When I add expectedCypherParams explicity, I get 90 tests failed

Ensure the expected cypher parameters are validated properly in Cypher tests.

[michaeldgraham]

https://github.com/neo4j-graphql/neo4j-graphql-js/issues/608