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

Updates Turtle tests to use triple term syntax and reifications #135

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

gkellogg
Copy link
Member

… instead of quoted triples.

@gkellogg gkellogg marked this pull request as draft April 4, 2024 16:25
@gkellogg gkellogg marked this pull request as ready for review August 5, 2024 23:59
@gkellogg gkellogg requested review from niklasl and afs and removed request for niklasl August 5, 2024 23:59
Copy link

@niklasl niklasl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are the required updates for the new reifier naming syntax.

rdf/rdf12/rdf-turtle/eval/turtle-star-eval-03.ttl Outdated Show resolved Hide resolved
rdf/rdf12/rdf-turtle/eval/turtle-star-eval-04.ttl Outdated Show resolved Hide resolved
@gkellogg gkellogg requested a review from niklasl August 17, 2024 19:20
@pchampin
Copy link
Contributor

This was discussed during the rdf-star meeting on 2024-08-29:
https://www.w3.org/2024/08/29-rdf-star-minutes.html#t04

@pchampin
Copy link
Contributor

pchampin commented Sep 12, 2024

This was discussed during the rdf-star meeting on 12 September 2024.

View the transcript

w3c/rdf-tests#135

gkellogg: first one has been open for a bit. new tests for updated turtle.
… need even more on that.
… waiting for someone else to have an implementation.
… AndyS is working on updates to his parser. Hopefully we can get an approval after that.

AndyS: about at the stage where I can run those tests.

ora: so not to merge yet?

gkellogg: yeah, let's wait until someone else is passing the tests.


@afs
Copy link
Contributor

afs commented Sep 15, 2024

N-triples tests:

ntriples-base* - "base" is a confusing name, especially when copied to nt-ttl-base, because they are not about BASE.
Suggestions: ntriples-langdir named after the terminal or ntriples-textdir.

ntriples-star-bad-reifier-1 tests reifier in the subject position.
ntriples-star-bad-reifier-2 tests reifier in the object position.
ntriples-star-bad-reifier-3 tests both subject and object - was a test of the predicate position intended?

ntriples-star-bad-reifier-4 is unclear. Maybe its testing declaration but it's hard to tell. Simplify to:
<< <http://example/s> <http://example/p> <http://example/o> >> .

ntriples-star-syntax-03 and ntriples-star-syntax-04 are the same.

ntriples-star-nested-bad-annotated-syntax-*.nt don't add much over ntriples-star-bnode-bad-annotated-syntax*.nt because the failure expected is at the first {|.

@gkellogg
Copy link
Member Author

N-triples tests:

ntriples-base* - "base" is a confusing name, especially when copied to nt-ttl-base, because they are not about BASE. Suggestions: ntriples-langdir named after the terminal or ntriples-textdir.

I renamed these all as you suggested.

ntriples-star-bad-reifier-1 tests reifier in the subject position. ntriples-star-bad-reifier-2 tests reifier in the object position. ntriples-star-bad-reifier-3 tests both subject and object - was a test of the predicate position intended?

``ntriples-star-bad-reifier-4 is unclear. Maybe it's testing declaration but it's hard to tell. Simplify to: `<< http://example/s http://example/p http://example/o >> .`

I wasn't able to find these tests in the repository; there are likely some additional tests that would be required. Can you point me to them?

ntriples-star-syntax-03 and ntriples-star-syntax-04 are the same.

ntriples-star-syntax-04 was an old file, and unreferenced; I removed it.

ntriples-star-nested-bad-annotated-syntax-*.nt don't add much over ntriples-star-bnode-bad-annotated-syntax*.nt because the failure expected is at the first {|.

True, if we were to consolidate, which set should remain?

@afs
Copy link
Contributor

afs commented Sep 18, 2024

I wasn't able to find these tests in the repository; there are likely some additional tests that would be required. Can you point me to them?

rdf/rdf12/rdf-n-triples/syntax/ntriples-star-bad-reifier-syntax-*.nt

ntriples-star-nested-bad-annotated-syntax-*.nt don't add much over ntriples-star-bnode-bad-annotated-syntax*.nt because the failure expected is at the first {|.

True, if we were to consolidate, which set should remain?

The simpler ntriples-star-bnode-bad-annotated-syntax*.nt. Annotations aren't in N-triples.

@gkellogg
Copy link
Member Author

rdf/rdf12/rdf-n-triples/syntax/ntriples-star-bad-reifier-syntax-*.nt

I didn't see it because the name is slightly different: rdf/rdf12/rdf-n-triples/syntax/ntriples-star-bad-reified-syntax-*.nt. I'll take care of your suggestion.

@gkellogg
Copy link
Member Author

This will still need updates for further grammar branches and to reconcile w3c/rdf-turtle#70.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separators should be space-emdash-space (or space-double hyphen-space), not space-hyphen-space.

</a>
<span about='../syntax#ntriples-star-nested-bad-annotated-syntax-1' property='mf:name'>N-Triples-star - Bad - annotated triple, nested subject term</span>
<span about='../syntax#ntriples-langdir-bad-1' property='mf:name'>N-Triples literal- Bad - undefined base direction</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<span about='../syntax#ntriples-langdir-bad-1' property='mf:name'>N-Triples literal- Bad - undefined base direction</span>
<span about='../syntax#ntriples-langdir-bad-1' property='mf:name'>N-Triples literalBad undefined base direction</span>

@@ -482,7 +482,7 @@ <h2>
<a class='testlink' href='#ntriples-star-bad-reified-4'>
ntriples-star-bad-reified-4:
</a>
<span about='../syntax#ntriples-star-bad-reified-4' property='mf:name'>N-Triples-star - Bad - reified triples whitespace and terms</span>
<span about='../syntax#ntriples-star-bad-reified-4' property='mf:name'>N-Triples-star - Bad - predicate reified triple</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<span about='../syntax#ntriples-star-bad-reified-4' property='mf:name'>N-Triples-star - Bad - predicate reified triple</span>
<span about='../syntax#ntriples-star-bad-reified-4' property='mf:name'>N-Triples-star Bad predicate reified triple</span>

</a>
<span about='../syntax#ntriples-star-nested-bad-annotated-syntax-2' property='mf:name'>N-Triples-star - Bad - annotated triple, nested object term</span>
<span about='../syntax#ntriples-langdir-bad-2' property='mf:name'>N-Triples literal- Bad - upper case LTR</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<span about='../syntax#ntriples-langdir-bad-2' property='mf:name'>N-Triples literal- Bad - upper case LTR</span>
<span about='../syntax#ntriples-langdir-bad-2' property='mf:name'>N-Triples literalBad upper case LTR</span>

@@ -168,7 +166,7 @@ trs:ntriples-star-bad-reified-3 rdf:type rdft:TestNTriplesNegativeSyntax ;
.

trs:ntriples-star-bad-reified-4 rdf:type rdft:TestNTriplesNegativeSyntax ;
mf:name "N-Triples-star - Bad - reified triples whitespace and terms" ;
mf:name "N-Triples-star - Bad - predicate reified triple" ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mf:name "N-Triples-star - Bad - predicate reified triple" ;
mf:name "N-Triples-star Bad predicate reified triple" ;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way the tests (all three generations) are. Developers use hyphen.
Keep it simple in the manifest because of viewing with the wrong character set.

The rest is automatically produced.

(FYI: Chicago style guide prefers no spaces around em-dashes, which then looks bad in this context.)

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

Successfully merging this pull request may close these issues.

5 participants