Add positive and negative SPARQL 1.2 triple terms syntax tests #144

rubensworks · 2024-09-25T13:17:22Z

As part of #140, this PR adds positive and negative syntax tests related to the new triple terms in SPARQL 1.2.
Evaluation tests are not included yet in this PR.

Some notes on this:

Tests are derived from the SPARQL star tests, and repeated as reifier, anonreifier, and tripleterm in the test name where applicable.
The SPARQL 1.1 syntax tests (positive and negative) are all captured in a single manifest. Here, I opted to split positive and negative, and have dedicated directories for triple terms. This attempts to separate tests by feature, similar to how SPARQL 1.1 already did this for evaluation tests.
This PR makes use of the existing SPARQL 1.1-specific test classes. SPARQL syntax test classes in test manifest vocabulary include version in IRI (11) #143 needs to be resolved first if we don't want this.
All of these tests in my fork of SPARQL.js, but it would be good if someone can double-check correctness in another implementation.

rubensworks · 2024-10-03T12:20:17Z

@afs Your comment regarding compound-all.rq has been resolved. I still need to look into the other issues next.

rubensworks · 2024-10-03T15:05:03Z

Reified triple declaration tests (positive and negative) have also been added.
I still need to add tests for multiple reifier ids in annotation syntax next.

sparql/sparql12/syntax-triple-terms-negative/manifest.ttl

sparql/sparql12/syntax-triple-terms-positive/manifest.ttl

sparql/sparql12/syntax-triple-terms-positive/expr-tripleterm-02.rq

rubensworks · 2024-10-16T12:06:02Z

Thanks for the reviews, all comments should be resolved now.

@afs I've also added some tests for multiple reifiers in annotation syntax (which is the reason this took me a while, as I didn't have this properly supported in SPARQL.js yet...)

rubensworks · 2024-10-16T12:14:55Z

Regarding the comment on #140 by @afs:

The SPARQL 1.1 are organised separated by query vs update as syntax-query/ and syntax-update-1/ syntax-update-2/. Positive and negative tests are in the same directory.

The grouping of all update-related tests in a single (or two) directory seemed a bit weird to me, since for non-update tests a feature-based directory grouping was chosen. So I opted for a pure feature-based directory grouping here.

I've split the positive and negative tests in separate directories to make management of the test manifest easier.

But I don't have a strong opinion on this, so I'm happy to move things around.

sparql/sparql12/syntax-triple-terms-positive/basic-tripleterm-05.rq

sparql/sparql12/syntax-triple-terms-positive/compound-tripleterm.rq

sparql/sparql12/syntax-triple-terms-positive/nested-tripleterm-01.rq

kasei · 2024-10-20T17:38:08Z

sparql/sparql12/syntax-triple-terms-negative/list-anonreifier-01.rq

+    # TODO: See if this should be throwing an error
+    << _:b ex:x () >> ex:broken true .


Is the suggestion here that () shouldn't parse as rdf:nil and be disallowed because you shouldn't be able to reify a triple with a list as an object?

This test was derived from the RDF-star tests.
I suspect you are right.
But I wonder if we really want/need to disallow this.
A message on this exact topic was also just raised on the mailinglist.

@afs perhaps you have some more insights on this?

See also w3c/rdf-ucr#26 ("Example 3: RDF Lists").

Is the suggestion here that () shouldn't parse as rdf:nil and be disallowed because you shouldn't be able to reify a triple with a list as an object?

Yes. The only meaning would be to assert the triples into the graph. RDF Collections are messy. In N3, collections are first-class citizens but not in RDF 1.x.

The idea is to keep triple terms as terms, without additional triples asserted in to a graph.
<<:s :p :o >> becomes _:r rdf:reifies <<( :s :p :o )>> -- <<...>> usage is connected to triple terms.

In SPARQL, a query expression can create terms outside a graph. No lists one or greater, no predicate object lists.

In annotation syntax, it is more reasonable to put the asserted triples in the graph although it only puts the head blank node into the triple term.
-- w3c/rdf-ucr#26 ("Example 3: RDF Lists") -- but << >> is not asserting.

() is an odd case because it is a term whereas any other list has triples. It can't occur in expressions as rdf:nil.

See also:

Turtle Grammar: Collections and blank node property lists in triple terms rdf-star-wg#132

More widely about (): Consider changing use of NIL to LPAREN, RPAREN sparql-query#160

kasei · 2024-10-20T17:39:24Z

sparql/sparql12/syntax-triple-terms-negative/list-tripleterm-01.rq

+    # TODO: See if this should be throwing an error
+    <<( _:b ex:x () )>> ex:broken true .


Same question here as with list-anonreifier-01 – is this about ()/rdf:nil?

niklasl

Would it be beneficial to move positive tests involving triple terms as subjects (here commented) to a "generalized RDF" feature set?

sparql/sparql12/syntax-triple-terms-positive/basic-tripleterm-06.rq

sparql/sparql12/syntax-triple-terms-positive/basic-tripleterm-07.rq

sparql/sparql12/syntax-triple-terms-positive/update-tripleterm-01.rq

sparql/sparql12/syntax-triple-terms-positive/update-tripleterm-03.rq

sparql/sparql12/syntax-triple-terms-positive/update-tripleterm-04.rq

rubensworks · 2024-10-23T09:01:19Z

Would it be beneficial to move positive tests involving triple terms as subjects (here commented) to a "generalized RDF" feature set?

@niklasl I've raised a new issue to discuss this topic: #149

This makes the SPARQL syntax test classes more flexible towards future spec versions. Closes w3c#143

gkellogg

My implementation passes all these tests, although the test entry types should be changed as I suggested in #143 (comment).

afs · 2024-10-24T09:13:37Z

@kasei pointed out that reverse paths means that subject and object positions should be the same. This isn't a generalized RDF issue.

afs · 2024-10-24T09:25:18Z

My implementation passes the current PR when adding #UpdateSyntaxTest and #NegativeUpdateSyntaxTest.

afs · 2024-10-24T09:34:36Z

@gkellogg The report generation is failing. Is this because the PR branch is not in the repository?

From https://github.com/w3c/rdf-tests
 * [new branch]      feature-iri-resolution -> origin/feature-iri-resolution
 * [new branch]      fix-protocol-manifest  -> origin/fix-protocol-manifest
 * [new branch]      fix-results-tests      -> origin/fix-results-tests
 * [new branch]      index-redirects        -> origin/index-redirects
 * [new branch]      main                   -> origin/main
 * [new branch]      original-tests         -> origin/original-tests
 * [new branch]      pfps-sparql-exists     -> origin/pfps-sparql-exists
 * [new branch]      resolution             -> origin/resolution
fatal: invalid reference: feature/sparql-triple-terms

gkellogg · 2024-10-24T16:59:36Z

@gkellogg The report generation is failing. Is this because the PR branch is not in the repository?

From https://github.com/w3c/rdf-tests
 * [new branch]      feature-iri-resolution -> origin/feature-iri-resolution
 * [new branch]      fix-protocol-manifest  -> origin/fix-protocol-manifest
 * [new branch]      fix-results-tests      -> origin/fix-results-tests
 * [new branch]      index-redirects        -> origin/index-redirects
 * [new branch]      main                   -> origin/main
 * [new branch]      original-tests         -> origin/original-tests
 * [new branch]      pfps-sparql-exists     -> origin/pfps-sparql-exists
 * [new branch]      resolution             -> origin/resolution
fatal: invalid reference: feature/sparql-triple-terms

I will look into it; could be transitory.

kasei · 2024-10-24T16:59:47Z

sparql/sparql12/syntax-triple-terms-negative/manifest.ttl

+:alternate-path-anonreifier rdf:type   mf:NegativeSyntaxTest ;
+   dawgt:approval dawgt:Proposed ;
+   mf:name        "alternate-path-anonreifier.rq" ;
+   mf:specVersion "sparql-1.2";


We have IRIs for the different SPARQL language versions (via the service description spec). I'd personally prefer if we used those instead of literals here. Thoughts?

We have IRIs for the different SPARQL language versions (via the service description spec). I'd personally prefer if we used those instead of literals here. Thoughts?

@kasei Would you be OK with raising that as separate issue, either one it's own or one to cover all manifest vocabulary changes? This is to let the test material go in - it's big PR.

Yes, totally fine if that's handled later.

afs · 2024-10-24T17:20:52Z

We discussed this PR in the WG meeting 2024-10-24 (brief minutes)

It was noted that there are sub-parts on the tests themselves and on the test manifest vocabulary.

The WG would be happy if the tests were merged and issues subsequently raised. The decision is left to this group.

kasei · 2024-10-24T18:13:46Z

I also have an implementation passing all of these tests. Happy for this to get merged now.

rubensworks · 2024-10-24T18:36:14Z

I agree with merging this PR in its current form, and discussing remaining issues through separate issues.

TallTed

Merge it. Big improvement over status quo. Nothing prevents further fine tuning of issues, whether remaining after or caused by this PR.

niklasl · 2024-10-24T19:38:41Z

I also agree.

gkellogg · 2024-10-24T20:49:05Z

@gkellogg The report generation is failing. Is this because the PR branch is not in the repository?

From https://github.com/w3c/rdf-tests
 * [new branch]      feature-iri-resolution -> origin/feature-iri-resolution
 * [new branch]      fix-protocol-manifest  -> origin/fix-protocol-manifest
 * [new branch]      fix-results-tests      -> origin/fix-results-tests
 * [new branch]      index-redirects        -> origin/index-redirects
 * [new branch]      main                   -> origin/main
 * [new branch]      original-tests         -> origin/original-tests
 * [new branch]      pfps-sparql-exists     -> origin/pfps-sparql-exists
 * [new branch]      resolution             -> origin/resolution
fatal: invalid reference: feature/sparql-triple-terms

I will look into it; could be transitory.

So, I believe the problem is that the PR is from a forked repository, and stefanzweifel/git-auto-commit-action@v4 seems to expect that it is a local branch? Anyway, I would just merge it and some future PR will end up rebuilding the HTML manifests.

Generally, I prefer that PRs come from local branches, but that requires write permission from the authors, which shouldn't be a problem.

Add positive and negative SPARQL 1.2 triple terms syntax tests

f6c7fe5

rubensworks added enhancement SPARQL labels Sep 25, 2024

rubensworks requested review from kasei, afs, Tpt and hartig September 25, 2024 13:17

afs mentioned this pull request Sep 26, 2024

The SPARQL grammar allows expression syntax triple terms to have triple terms as subjects. w3c/sparql-query#158

Closed

rubensworks added 2 commits October 3, 2024 17:03

Make compound-all a negative test and add positive variant

bcd1108

Add syntax tests for triple terms and reifiers not in triple patterns

fef7c5d

rubensworks force-pushed the feature/sparql-triple-terms branch from f116dfa to fef7c5d Compare October 3, 2024 15:04

kasei reviewed Oct 3, 2024

View reviewed changes

sparql/sparql12/syntax-triple-terms-negative/manifest.ttl Outdated Show resolved Hide resolved

kasei reviewed Oct 3, 2024

View reviewed changes

sparql/sparql12/syntax-triple-terms-positive/manifest.ttl Outdated Show resolved Hide resolved

kasei reviewed Oct 3, 2024

View reviewed changes

sparql/sparql12/syntax-triple-terms-positive/expr-tripleterm-02.rq Outdated Show resolved Hide resolved

rubensworks added 4 commits October 16, 2024 13:42

Add tests for multiple reifiers in annotation syntax

bfc6364

Remove mfx prefix

a62f4f0

Fix incorrect : prefixes

109b173

Remove expr-tripleterm-02

0e7088f

kasei reviewed Oct 20, 2024

View reviewed changes

niklasl reviewed Oct 23, 2024

View reviewed changes

Disallow triple terms in subject position

690d699

rubensworks mentioned this pull request Oct 23, 2024

Tests for generalized RDF #149

Open

Introduce version-agnostic SPARQL test classes and mf:specVersion

eafb886

This makes the SPARQL syntax test classes more flexible towards future spec versions. Closes w3c#143

rubensworks force-pushed the feature/sparql-triple-terms branch from 52735fd to eafb886 Compare October 23, 2024 09:24

rubensworks mentioned this pull request Oct 23, 2024

SPARQL syntax test classes in test manifest vocabulary include version in IRI (11) #143

Closed

rubensworks self-assigned this Oct 23, 2024

gkellogg reviewed Oct 23, 2024

View reviewed changes

Remove redundancy in SPARQL test classes

3b4894a

afs approved these changes Oct 24, 2024

View reviewed changes

kasei reviewed Oct 24, 2024

View reviewed changes

TallTed approved these changes Oct 24, 2024

View reviewed changes

kasei approved these changes Oct 24, 2024

View reviewed changes

rubensworks merged commit 831feb1 into w3c:main Oct 25, 2024
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add positive and negative SPARQL 1.2 triple terms syntax tests #144

Add positive and negative SPARQL 1.2 triple terms syntax tests #144

rubensworks commented Sep 25, 2024 •

edited

Loading

rubensworks commented Oct 3, 2024

rubensworks commented Oct 3, 2024

rubensworks commented Oct 16, 2024

rubensworks commented Oct 16, 2024

kasei Oct 20, 2024

rubensworks Oct 23, 2024

niklasl Oct 23, 2024

afs Oct 23, 2024 •

edited

Loading

kasei Oct 20, 2024

niklasl left a comment

rubensworks commented Oct 23, 2024

gkellogg left a comment

afs commented Oct 24, 2024 •

edited

Loading

afs commented Oct 24, 2024

afs commented Oct 24, 2024 •

edited

Loading

gkellogg commented Oct 24, 2024

kasei Oct 24, 2024

afs Oct 24, 2024

kasei Oct 24, 2024

afs commented Oct 24, 2024

kasei commented Oct 24, 2024

rubensworks commented Oct 24, 2024

TallTed left a comment

niklasl commented Oct 24, 2024

gkellogg commented Oct 24, 2024

		# TODO: See if this should be throwing an error
		<< _:b ex:x () >> ex:broken true .

		# TODO: See if this should be throwing an error
		<<( _:b ex:x () )>> ex:broken true .

Add positive and negative SPARQL 1.2 triple terms syntax tests #144

Add positive and negative SPARQL 1.2 triple terms syntax tests #144

Conversation

rubensworks commented Sep 25, 2024 • edited Loading

rubensworks commented Oct 3, 2024

rubensworks commented Oct 3, 2024

rubensworks commented Oct 16, 2024

rubensworks commented Oct 16, 2024

kasei Oct 20, 2024

Choose a reason for hiding this comment

rubensworks Oct 23, 2024

Choose a reason for hiding this comment

niklasl Oct 23, 2024

Choose a reason for hiding this comment

afs Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

kasei Oct 20, 2024

Choose a reason for hiding this comment

niklasl left a comment

Choose a reason for hiding this comment

rubensworks commented Oct 23, 2024

gkellogg left a comment

Choose a reason for hiding this comment

afs commented Oct 24, 2024 • edited Loading

afs commented Oct 24, 2024

afs commented Oct 24, 2024 • edited Loading

gkellogg commented Oct 24, 2024

kasei Oct 24, 2024

Choose a reason for hiding this comment

afs Oct 24, 2024

Choose a reason for hiding this comment

kasei Oct 24, 2024

Choose a reason for hiding this comment

afs commented Oct 24, 2024

kasei commented Oct 24, 2024

rubensworks commented Oct 24, 2024

TallTed left a comment

Choose a reason for hiding this comment

niklasl commented Oct 24, 2024

gkellogg commented Oct 24, 2024

rubensworks commented Sep 25, 2024 •

edited

Loading

afs Oct 23, 2024 •

edited

Loading

afs commented Oct 24, 2024 •

edited

Loading

afs commented Oct 24, 2024 •

edited

Loading