search

zazuko / rdf-validate-shacl

Validate RDF data purely in JavaScript. An implementation of the W3C SHACL specification on top of the RDFJS stack.
MIT License
98 stars 13 forks source link

fixes https://github.com/zazuko/rdf-validate-shacl/issues/136 #137

Closed s-tittel closed 2 months ago

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 6041552d9a2817367c61738a33413c4354d9fd5f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------ | ----- | | rdf-validate-shacl | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

tpluscode commented 2 months ago

Hey, thank you for your contribution!

Would you please add your failing test case to test/data/data-shapes/custom/ and to the manifest.ttl?

tpluscode commented 2 months ago

Also, I noticed that the CI wasn't set up correctly. You may want to rebase master in order to run the tests on GH

s-tittel commented 2 months ago

Hey, I have rebased master, added the test case and fixed a bug in my code that erroneously let all tests pass. Now I get:

132 passing (3s) 3 pending 6 failing

The failing tests are: 1) Test of validation report for shape xone-duplicate by property constraints 2) Test of sh:property at property shape 001 3) Test of sh:qualifiedMinCount with disjoint shapes at property shape 001 4) Test of sh:qualifiedValueShape at property shape 001 5) Test of sh:qualifiedValueShapesDisjoint at property shape 001 6) Test of validation report for shape shared by property constraints

I am not sure if my code really breaks something or if it just changes the order in the expected test reports.

tpluscode commented 2 months ago

Thank you for you update.

Unfortunately, there appear to be multiple types of issues here. It is most definitely not just ordering :)

These three produce less validation results, related to logical constraints, I think.

On the other hand these two produces duplicate results. Probably somehow related.

Finally, this one succeeds validation where it should have failed

s-tittel commented 2 months ago

Yes, unfortunately. The first 3 mentioned test cases could be fixed by reordering / deduplication of validation results (see latest commit). Regarding the qualified value shape tests I currently have no clue.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.77%. Comparing base (1d43b2f) to head (6041552).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #137 +/- ## ======================================= Coverage 98.76% 98.77% ======================================= Files 12 12 Lines 1866 1881 +15 ======================================= + Hits 1843 1858 +15 Misses 23 23 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

s-tittel commented 2 months ago

The latest commit is a bit quirky, but fixes validation for qualified value shapes by resetting the list of already checked focus nodes.

s-tittel commented 2 months ago

Ok, still 3 tests failing. I'll see what I can do.

s-tittel commented 2 months ago

the latest commit lets all tests pass. it's not a perfect solution, but I couldn't come up with something better. at least the qualified value shape tests obviously need some degree of recursion, so what I do is check for a max recursion depth. what do you think?