Add test for having clause with expression (Var != Iri) by white-gecko · Pull Request #935 · RDFLib/rdflib (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits4 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This pull request adds three test cases for a group by (testGroupBy) and two group by with having clauses (testHavingAggregateEqLiteral and testHavingPrimaryExpressionVarNeqIri).
The having clause that compares an aggregate with a literal works correctly (testHavingAggregateEqLiteral), while the comparison of a variable with an iri does not work correctly (testHavingPrimaryExpressionVarNeqIri).
I expect the having clause in testHavingPrimaryExpressionVarNeqIri to be true in all cases and thus to produce two results, just as the testGroupBy does.
Test for issue #936.
@white-gecko are you expecting testHavingPrimaryExpressionVarNeqIri to fail (as it does) and then, you're going to add code to this PR to make it pass? What's the plan?
I want to highlight the bug as described in #936 with this test. So it would be good to pass but it is expected to fail at the moment unless #936 is fixed. So far I have no clear idea how to fix it but I knew how to reproduce it. So this is a preliminary contribution for a test-driven development ;-)
I'm adding wontfix as this PR can't be merged as it contains a test which will fail.
I'm adding
wontfixas this PR can't be merged as it contains a test which will fail.
Somehow this is ok in such cases, but on the other hand my intention is, writing an issue tells us, that a problem exists, but writing a test allows us to automatically check when an issue is fixed. (Towards Test-driven Development: https://en.wikipedia.org/wiki/Test-driven_development)
Towards Test-driven Development
If we merge it, it will break every other PR due to a failing test so we will have to check Travis for each PR to see if only thif failed or more tests.
If the PR sits here un-merged, we might see work added to fix it in the future. Perhaps we just need a label other than "wontfix", perhaps we just leave it un-merged with "help wanted"?
"testing" should fit, but is quite broad.
I'm not very sure in general about the tag "help wanted", as this is an open source project, where help is always welcome for every issue :-D
I think for now this is fine to tag it with both of these tags and maybe also make it a draft pull-request, which prevents it from being merged. For sure it should not be merged until there is a fix.
If others will take up this pattern we can introduce a new tag for it.
A draft PR sounds good! I’ve not done one of those before.
Nicholas Car added 2 commits
If PR #1093 is rejected and closed, minimally, change to:
import pytest from rdflib import Graph
g = Graph()
data = """ urn:a urn:p 1 . urn:b urn:p 3 . urn:c urn:q 1 . """ g.parse(data=data, format="turtle")
def test_group_by(): query = "SELECT ?p" "WHERE { ?s ?p ?o } " "GROUP BY ?p" qres = g.query(query)
assert len(qres) == 2def test_having_aggregate_eq_literal(): query = ( "SELECT ?p (avg(?o) as ?a) " "WHERE { ?s ?p ?o } " "GROUP BY ?p HAVING (avg(?o) = 2 )" ) qres = g.query(query)
assert len(qres) == 1@pytest.mark.xfail() def test_having_primary_expression_var_neq_iri(): query = ( "SELECT ?p " "WHERE { ?s ?p ?o } " "GROUP BY ?p HAVING (?p != urn:foo )" ) qres = g.query(query)
assert len(qres) == 2 # is 0else:
Schedule for closing as “fixed in PR #1093”
