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

Enable full test suite with Python 3. #774

Merged
merged 13 commits into from Feb 27, 2018

Conversation

Projects
None yet
3 participants
@jmchilton
Copy link
Member

jmchilton commented Feb 23, 2018

Tweaking testing in a way that needs to be stripped out before merge but hoping to get some fresh Python 3 tracebacks on Travis to work from.

Problems:

  • Something in dependency_script
    Error processing /home/travis/build/galaxyproject/planemo/tests/data/repos/package_1/tool_dependencies.xml - Checksum failure for /tmp/tmph9zwo1rp/download_cache/samtools-0.1.16-linux-i386.tgz, got b'7090bd62142df0ed3a2b4000292fc0edc917e773d0269c2f3f5b0d36b7888c22' but wanted '7090bd62142df0ed3a2b4000292fc0edc917e773d0269c2f3f5b0d36b7888c22'
  • Galaxy getting loaded without target tool in the tool panel - other people have observed this in the wild - seems now somehow related to Python 3 Planemo?
 FAIL: test_serve_workflow (tests.test_cmd_serve.ServeTestCase)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/home/travis/build/galaxyproject/planemo/tests/test_cmd_serve.py", line 54, in 
 test_serve_workflow
     assert len(user_gi.workflows.get_workflows()) == 1
 nose.proxy.AssertionError: 
     assert len(GalaxyInstance object for Galaxy at 
 http://localhost:51267.histories.get_histories(name='Cool History 42')) == 0
     GalaxyInstance object for Galaxy at http://localhost:51267.histories.create_history('Cool History 
 42')
     assert GalaxyInstance object for Galaxy at 
 http://localhost:51267.tools.get_tools(tool_id="random_lines1")
 >>  assert len(GalaxyInstance object for Galaxy at http://localhost:51267.workflows.get_workflows()) == 1

Probably same problem as above?

@jmchilton jmchilton force-pushed the jmchilton:py3_full branch from 72003e8 to a93c842 Feb 23, 2018

@nsoranzo

This comment has been minimized.

Copy link
Member

nsoranzo commented Feb 26, 2018

Commit 19c5f32 fixed the last point on the PR description, which I have now removed. Also tests are able to download the latest SQLite database and therefore run faster.

@nsoranzo nsoranzo force-pushed the jmchilton:py3_full branch from 416a332 to 19c5f32 Feb 26, 2018

@nsoranzo nsoranzo referenced this pull request Feb 26, 2018

Merged

Python3 fixes #92

@nsoranzo

This comment has been minimized.

Copy link
Member

nsoranzo commented Feb 26, 2018

Down to 4 failures, I may need some help with those.

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 26, 2018

Wow - thanks so much @nsoranzo - this is awesome.

Trying to pick it up from here - I can replicate broadly the behavior here #765 with Python 3 now - the toolbox loads fine with Python 3 and planemo serve but there are no tests with planemo test. This is at least one of the 4 remaining errors - this one about expecting a test failure and not finding one.

@nsoranzo

This comment has been minimized.

Copy link
Member

nsoranzo commented Feb 27, 2018

Thanks! I tried to restart the test build a few times, but it keeps stalling at "Test testing a simple GA workflow with Galaxy" test.

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 27, 2018

Working on it - locks up pretty quickly here. Looks like the output is fetched from Galaxy but nothing ends.

galaxy.jobs DEBUG 2018-02-26 21:07:34,550 job 3 ended (finish() executed in (1415.952 ms))
127.0.0.1 - - [26/Feb/2018:21:07:34 -0400] "GET /api/workflows/2891970512fa2d5a/invocations/2891970512fa2d5a?key=a293420304dd548281d311bc6ff1f0ba HTTP/1.1" 200 - "-" "python-requests/2.18.4"
127.0.0.1 - - [26/Feb/2018:21:07:34 -0400] "GET /api/jobs/54f2a3a23292eb07?full=True&key=a293420304dd548281d311bc6ff1f0ba HTTP/1.1" 200 - "-" "python-requests/2.18.4"
127.0.0.1 - - [26/Feb/2018:21:07:35 -0400] "GET /api/histories/2891970512fa2d5a/contents/54f2a3a23292eb07?key=a293420304dd548281d311bc6ff1f0ba HTTP/1.1" 200 - "-" "python-requests/2.18.4"
127.0.0.1 - - [26/Feb/2018:21:07:35 -0400] "GET /api/histories/2891970512fa2d5a/contents/54f2a3a23292eb07/display HTTP/1.1" 200 - "-" "python-requests/2.18.4"

Maybe killing the server and re-joining is broken in Python 3... hmm...

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 27, 2018

This piece of code in galaxy-lib I suspect is the problem:

    if path is not None:
        properties["path"] = path
        f = open(path, "rb")
    else:
        f = StringIO(content)

    try:
        contents = f.read(1024 * 1024)
        filesize = 0
        while contents != "":
            checksum.update(contents)
            filesize += len(contents)
            contents = f.read(1024 * 1024)
    finally:
        f.close()
@peterjc

This comment has been minimized.

Copy link
Contributor

peterjc commented Feb 27, 2018

@jmchilton Given the file is opened in binary mode, and you want the raw length, then from io import BytesIO seems a better bet than StringIO here.

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 27, 2018

@peterjc Yup - that was exactly the fix - galaxyproject/galaxy-lib#93. I should have linked it against this PR.

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 27, 2018

I think the only broken test here is a test that has been broken in master as we worked on this. Local conda packages maybe broke during the big galaxy-lib update or maybe broke due to Conda changes elsewhere.

shell("ls '%s'" % (template_dir))
shell("mv '%s'/* '%s'" % (template_dir, path))
shell(['ls', template_dir])
shell(['mv', "%s/*" % template_dir, path])

This comment has been minimized.

@jmchilton

jmchilton Feb 27, 2018

Author Member

Thanks for the fix - the rest of this looks good but this line required the shell expansion to happen. Can I revert just this line or are you working on an approach that avoids shell to do this?

This comment has been minimized.

@nsoranzo

nsoranzo Feb 27, 2018

Member

Yep, this broke project_init tests. Rebased with a fixed (and shorter) version.

@nsoranzo nsoranzo force-pushed the jmchilton:py3_full branch from 88aafc9 to 207bf01 Feb 27, 2018

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 27, 2018

The jmchilton branch Travis finishes faster:

___________________________________ summary ____________________________________
  py34: commands succeeded
  congratulations :)

🎆 🎆 🎆 🎆 🎆

Thanks so much @nsoranzo - this is really awesome. I really appreciate all the work you put into this.

nsoranzo and others added some commits Feb 26, 2018

Python3: unicodify filehash
Fix the following test:
```
$ nosetests tests/test_dependency_script.py
..F.....
======================================================================
FAIL: test_good_examples (tests.test_dependency_script.DependencyScriptTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/users/ga002/soranzon/software/nsoranzo_planemo/tests/test_dependency_script.py", line 79, in test_good_examples
    self._check_exit_code(ds_cmd, 0)
  File "/usr/users/ga002/soranzon/software/nsoranzo_planemo/tests/test_utils.py", line 100, in _check_exit_code
    raise AssertionError(message)
...
-------------------- >> begin captured stdout << ---------------------
Command list output is [Downloading http://depot.galaxyproject.org/package/linux/i386/samtools/samtools-0.1.16-linux-i386.tgz to '/tmp/tmp8mq79eb0/download_cache/samtools-0.1.16-linux-i386.tgz'
Verifying checksum for samtools-0.1.16-linux-i386.tgz
Error processing /usr/users/ga002/soranzon/software/nsoranzo_planemo/tests/data/repos/package_1/tool_dependencies.xml - Checksum failure for /tmp/tmp8mq79eb0/download_cache/samtools-0.1.16-linux-i386.tgz, got b'7090bd62142df0ed3a2b4000292fc0edc917e773d0269c2f3f5b0d36b7888c22' but wanted '7090bd62142df0ed3a2b4000292fc0edc917e773d0269c2f3f5b0d36b7888c22'
...
```
Python 3 fix.
Fixes #765 I think.
Pre-emptively just stick with a list for mulled target stuff.
In the spirit of 3e46d71 but I'm not actually sure this fixes anything - at very least it shouldn't break anything.

@jmchilton jmchilton force-pushed the jmchilton:py3_full branch from 207bf01 to 75da607 Feb 27, 2018

@jmchilton jmchilton changed the title [WIP] Enable full test suite with Python 3. Enable full test suite with Python 3. Feb 27, 2018

@jmchilton jmchilton merged commit da6b48d into galaxyproject:master Feb 27, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment