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

TestPlannerQuery in planner_test.go should test deeper #130

Open
rogerlucena opened this issue Aug 10, 2020 · 0 comments
Open

TestPlannerQuery in planner_test.go should test deeper #130

rogerlucena opened this issue Aug 10, 2020 · 0 comments

Comments

@rogerlucena
Copy link
Contributor

rogerlucena commented Aug 10, 2020

The test function TestPlannerQuery in planner_test.go verifies only if the resolved query has the expected number of bindings and if the result obtained after querying the graph has the expected number of rows. This can open space for mistakes in the future.

To illustrate, take the query below:

SELECT ?time
FROM ?test
WHERE {
  ?s ?p ?o .
  OPTIONAL { ?s ?p AT ?time ?o }
};

Imagine that we expect its result to have 3 rows, on which the bindings for ?time should have the <NULL> value (like in the case the ?p bindings were all immutable predicates in our graph ?test). If a change is made in the code later on and a side effect is that the result of this query still has 3 rows but now with the bindings for ?time having non <NULL> values, the test would not detect the problem and would still pass as the number of returned rows is the same expected 3 as before, which is dangerous as the code is not having the right behavior anymore.

This is just one scenario to illustrate this potential problem. Another one could involve changes made in the ?test graph as well.

Then, a deeper test should be done here in TestPlannerQuery to assure that the values of the rows are indeed the ones we expect, and not only the number of them.

In addition to that, it would be nice to also be able to personalize, for each query tested there, which graph it is querying, instead of using the same graph for all of them. This way, if we develop something in the future that requires new triples in the ?test graph for us to be able to properly test it there in planner_test.go, we could just create another graph and use it to write those tests instead of adding new triples to the shared ?test graph and having to change all other tests impacted by these additions (which is error-prone).

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

No branches or pull requests

1 participant