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

SHACL testing #13

Merged
merged 18 commits into from
Nov 22, 2016
Merged

SHACL testing #13

merged 18 commits into from
Nov 22, 2016

Conversation

gaurav
Copy link
Member

@gaurav gaurav commented Oct 19, 2016

Provides a Java class that interfaces with TopBraid's SHACL library to validate an RDF/XML file against a file containing SHACL space. Also includes a Python script that can be called by py.test to run validations correctly and to report the result.

This is pretty complicated, so it might be easier to consider two other options:

    1. Use Docker to create a self-contained installation that contains Apache Jena and the SHACL library. We would still be dependent on Python 2.7 and Java 1.8 (neither Jena nor SHACL will work with Java 1.7) -- unless we could package those too?
    1. We don't actually do much with Python in here; all we use is rdflib. If we port our code to Java or Scala and use Apache Jena instead, we would end up with a single code base that doesn't depend on Python at all. This would probably make testing and deployment much simpler.

Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

Minor comment re: the SHACL runner, normally programs shouldn't output to stderr in case of success unless asked to (such as through --verbose or some such). But addressing that can be left to later - it doesn't change the test behavior at all.

@@ -1,4 +1,7 @@
language: python
sudo: required
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that's needed? Is that because of switching to language: java? I thought they had rid themselves of having to specify that. It used to be necessary for Docker-utilizing builds, which this one isn't.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, it looks like it is: without sudo: required, TravisCI uses the container-based virtualization environments (https://docs.travis-ci.com/user/ci-environment/), which for some reason won't allow pip install --users to work correctly: check out line 375 at https://travis-ci.org/gaurav/phylo2owl/builds/169361177#L375, where it appears to correctly install setuptools>=18.5, but then reports Successfully installed setuptools-12.0.5!

I might be able to download Jena to /tmp and use it from there, instead of sudo-ing a folder in /opt for it. Would that help? I don't think it makes a difference if we need to sudo for pip install anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. It used to work as recently as #22: https://travis-ci.org/phyloref/phylo2owl/jobs/147828629

I suppose what makes the difference is switching from language: python to language java. That switch causes pip install to fail without either --user using sudo? (I would not specify the version for setuptools. I vaguely recall that I ran into problems with that and then simply left it at whatever defaulted to the latest version.)

How about the alternative of leaving language: python and then installing Java on the fly? Does that not work because of the license restrictions on Java, which means you can't just download and install it (even if under a custom location and then setting JAVA_HOME)?

@gaurav
Copy link
Member Author

gaurav commented Oct 20, 2016

The idea with sending the output to stderr was to ensure that stdout was always well-formed Turtle: I didn't want any error messages getting in there!

@hlapp
Copy link
Member

hlapp commented Oct 20, 2016

The idea with sending the output to stderr was to ensure that stdout was always well-formed Turtle: I didn't want any error messages getting in there!

Right, but announcing success isn't an error message 😄

@gaurav
Copy link
Member Author

gaurav commented Nov 22, 2016

@hlapp: I got rid of the STDERR stuff and the sudo, and I managed to set up Maven so that it creates a JAR file containing all the prerequisites it needs. I still need to build testShacl.jar locally, rather than on Travis, but I've filed that issue for another day: #14

@hlapp
Copy link
Member

hlapp commented Nov 22, 2016

Great, nice work, @gaurav! There's a lot of commits here now of all the trials and errors. I can just squash the whole PR into a single commit ("squash and merge"), or if you want to retain specific commits, you can do an interactive rebase squashing what can be squashed and keeping commits as individual ones as you would like.

Let me know. By default I would squash into a single commit and merge.

@gaurav
Copy link
Member Author

gaurav commented Nov 22, 2016

Yup, let's squash and merge it. The largest changes are the source files needed by the SHACL library; the others are mostly small files, and each file should be pretty straightforward to understand.

@hlapp hlapp merged commit 5349dc7 into phyloref:master Nov 22, 2016
@gaurav gaurav deleted the shacl_testing branch November 22, 2016 23:08
@gaurav gaurav restored the shacl_testing branch November 22, 2016 23:21
@gaurav gaurav deleted the shacl_testing branch February 16, 2017 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants