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

clarify status of run_tests_py3.sh #664

Closed
chrysn opened this issue Nov 8, 2016 · 7 comments · Fixed by #668
Closed

clarify status of run_tests_py3.sh #664

chrysn opened this issue Nov 8, 2016 · 7 comments · Fixed by #668
Labels
cleanup enhancement New feature or request testing
Milestone

Comments

@chrysn
Copy link
Contributor

chrysn commented Nov 8, 2016

the run_tests_py3.sh script seems to be in bad shape; on a debian testing with rdflib 4.2.1 and sparqlwrapper 1.7.6, i got 3 errors and 3 failures.

one of the failures i could trace down to test/test_issue375.py executing python in a subprocess; with all the 2to3'd stuff around, that would only ever work if python is a (symlink to a) python3 implementation, which happens but is rare enough to lead me to the conclusion that run_tests_py3.sh is not regularly used. in comparison, .travis.yml has explicit precautions against phenomenon, but invokes nosetests explicitly.

do the scripts still reflect the test suite that rdflib is supposed to pass, or are they an artifact and plain nosetests should be used instead?

@dbs
Copy link
Contributor

dbs commented Nov 8, 2016

It's a bit of an aside, but as the person originally responsible for test/test_issue375, that test should probably just be removed. It was useful to test rdfpipe for py2/py3 issues such as af87abe but there must be a better way to properly test it.

@joernhees
Copy link
Member

could you test with the 5.0.0-dev branch? IIRC this is somehow related to #375 and the RDFa and microcode duplication issues that are fixed there...

@chrysn
Copy link
Contributor Author

chrysn commented Nov 10, 2016

On Wed, Nov 09, 2016 at 07:51:30AM -0800, Jörn Hees wrote:

could you test with the 5.0.0-dev branch? IIRC this is somehow related
to #375 and the RDFa and microcode duplication issues that are fixed
there...

i still get failures. there is still an from urllib.parse import urlparse, urljoin, urldefrag /ImportError: No module named parse error
that hints at python being assumed to be python3. (having a
python->python3 in the PATH fixes 2 out of 3 errors).

the remaining test fails for lack of the graph_tool dependency (also
present in master). for that, i didn't find a note in any of the
requirements files -- is it a purely optional dependency, and shouldn't
the tests be skipped if it's absent? (the failing test is
rdflib.extras.external_graph_libs.rdflib_to_graphtool)

@joernhees
Copy link
Member

hmm, just to be sure... you're talking about these python?

The whole tests assume that they're run in a virtualenv which is either py2 or py3 and TBH in light of some distros now calling python3 python and python2 python2, that's probably the sanest thing to do. In consequence we should probably just rename all occurrences of python3 in run_tests_py3.sh to python.

yet another option would be to just wait for this mess to be fixed by our six'erization: #519 ... as soon as that's done run_tests_py3.sh should be removed anyhow, as there won't be any 2to3 anymore and no need to run the py3 tests in another dir than the py2 tests.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 13, 2016

On Sun, Nov 13, 2016 at 07:41:32AM -0800, Jörn Hees wrote:

yes; at least they have the very arguments i see the python program invoked with.

The whole tests assume that they're run in a virtualenv which is either py2 or py3 and TBH in light of some distros now calling python3 python and python2 python2, that's probably the sanest thing to do. In consequence we should probably just rename all occurrences of python3 in run_tests_py3.sh to python.

i disagree: the test suite should be runnable in the native environment. crafting a virtualenv to make the test suite runnable seems to limit the applicability of the results to the production environment.

i'd suggest that the occurrences of 'python' be replaced with sys.executable.

@joernhees
Copy link
Member

👍 thanks for pointing that out, that's obviously a much more portable solution... (sorry for the late reply, my email notifications somehow don't seem to work reliably anymore :-/)

joernhees added a commit to joernhees/rdflib that referenced this issue Nov 18, 2016
joernhees added a commit that referenced this issue Nov 18, 2016
* master:
  make test 375 more portable (use sys.executable rather than python), thanks to @chrysn, fixes #664
  re-add logger version output: INFO for interactive session, DEBUG for others
  Remove logger output
@joernhees
Copy link
Member

@chrysn so that part should be fixed in both 4.2.2-dev (current master) and 5.0.0-dev, please reopen / create a new issue if there's still a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup enhancement New feature or request testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants