Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve test coverage of query pagination code for queries with type coercions #897

Open
obi1kenobi opened this issue Aug 11, 2020 · 2 comments

Comments

@obi1kenobi
Copy link
Contributor

While adding types to the query pagination code, I realized that a good chunk of the code appears to implicitly assume that queries do not contain type coercions: for example, there was a lot of use of ast.name.value, which is only defined for FieldNode and doesn't work for InlineFragmentNode AST types.

This situation emerged because our SQL backend doesn't currently support type coercions (because SQL in general doesn't do table inheritance), and because we prototyped the query pagination code on SQL.

I am doing my best to fix the code as best I can, but I'm not sure I can get everything. We need more test coverage to prove that query pagination works for all queries, including ones with type coercions.

@bojanserafimov
Copy link
Collaborator

Worth checking why test_query_pagination.py:test_with_compiler_tests doesn't catch those bugs. It runs pagination against all defined compiler test inputs, checking that no errors are raised.

@bojanserafimov
Copy link
Collaborator

bojanserafimov commented Aug 18, 2020

Ah, the current tests pass because we always paginate at the root right now. We have tests for the parameterizer that make it insert filters on a different path, but we don't have one with type coercions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants