search

comunica / sparqlee

⚙️ SPARQL expression evaluator library - Moved to @comunica/expression-evaluator
https://www.npmjs.com/package/sparqlee
14 stars 7 forks source link

Copy the spec tests for aggregates #41

Closed wschella closed 2 years ago

wschella commented 5 years ago

As we did for the spec tests for functions, we should copy the spec tests for aggregates from the test-repo.

jitsedesmet commented 3 years ago

@wschella or @rubensworks, do you have a reference on where to find these test?

rubensworks commented 3 years ago

Probably these ones: https://github.com/w3c/rdf-tests/tree/main/sparql11/data-sparql11/aggregates

jitsedesmet commented 3 years ago

Since these are also a lot of files, I will split the spec directory.

jitsedesmet commented 3 years ago

I've been trying for quite some time now. I have no idea on how to test this within sparqlee. Take for example the agg2 test. If I'm not mistaken we have a whole actor in comunica to handle these kind of combinations? How would we go about testing this here?

rubensworks commented 3 years ago

I don't think the intention was to copy the tests as-is. But instead, to select the elements of these tests that are applicable to sparqlee, i.e., those that interact directly with the aggregate evaluator.

jitsedesmet commented 3 years ago

So we would just exclude the agg2 test? because I don't think we can test this.

rubensworks commented 3 years ago

You'll know better than me, but can't we test the COUNT part? If I remember correctly, grouping is done purely on the Comunica side, so that can't be tested in Sparqlee indeed.

jitsedesmet commented 3 years ago

Yes, I guess we could. I'll write the tests. Doesn't really seem all that valuable to me but I guess it's nice to have.

jitsedesmet commented 2 years ago

I think that this issue can be closed because of #131? Like I said here before (https://github.com/comunica/sparqlee/issues/41#issuecomment-900339813) the test is already included in the Comunica repo.

rubensworks commented 2 years ago

Sure.