Add test for having clause with expression (Var != Iri)

[white-gecko]

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.

[nicholascar]

Open/close Travis

[nicholascar]

@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?

[white-gecko]

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 ;-)

[nicholascar]

I'm adding wontfix as this PR can't be merged as it contains a test which will fail.

[nicholascar]

Open Close Travis

[white-gecko]

I'm adding wontfix as 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)

[nicholascar]

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"?

[white-gecko]

"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.

[nicholascar]

A draft PR sounds good! I’ve not done one of those before.

[aucampia]

Maybe it makes sense to merge this with @unittest.expectedFailure

[ghost]

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) == 2


def 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 0

else:

Schedule for closing as “fixed in PR #1093”