install external packages for tests #901

orbeckst · 2016-07-11T22:35:57Z

Proof of concept for #898

Changes made in this Pull Request:

Install additional packages for the full testing environment on Travis CI:

install HOLE
install clustalw2

This enables additional tests that are currently skipped because executables are not available.

PR Checklist

n/a Tests?
n/a Docs?
CHANGELOG updated?
Issue raised/referenced?

coveralls · 2016-07-11T23:47:20Z

Coverage increased (+1.4%) to 82.402% when pulling 74373ac on issue-898-external-code into 98d5c10 on develop.

orbeckst · 2016-07-12T00:33:32Z

Works for hole in 74373ac (see e.g. Linux travis build 2104.2).

This works for Linux and OSX (see 2104.6).

The HOLE package is downloaded and unpacked into ${HOME}/opt/packages and the bin directory added to PATH. Coverage of the analysis.hole module is now 59% (if we just run the plotting code it would be most of it...) and overall coverage +1.4% :-).

Download and installation of hole is less than 2 s of the build time.

orbeckst · 2016-07-12T00:36:06Z

conda packages for clustalw2 were built by the nice folks at https://biobuilds.org/ — they have both Linux and OSX conda packages: https://anaconda.org/biobuilds/clustalw. It installs clustalw2, which is the name of the executable as present in the tests for aanalysis.align.

This was added in 642a1d9 and is currently running on Travis.

orbeckst · 2016-07-12T01:07:47Z

Needs opinions/reviews.

coveralls · 2016-07-12T01:24:55Z

Coverage increased (+1.5%) to 82.51% when pulling 642a1d9 on issue-898-external-code into 98d5c10 on develop.

coveralls · 2016-07-12T02:33:40Z

Coverage decreased (-1.3%) to 79.747% when pulling 642a1d9 on issue-898-external-code into 98d5c10 on develop.

coveralls · 2016-07-12T02:34:55Z

Coverage decreased (-0.07%) to 80.951% when pulling 642a1d9 on issue-898-external-code into 98d5c10 on develop.

orbeckst · 2016-07-12T05:03:24Z

On OSX full test_hole_module_fd_closure() failed in 2106.6 with

ERROR: MDAnalysis.analysis.hole: Issue 129: ensure low level file descriptors to PDB files used by Hole program are properly closed
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/travis/build/MDAnalysis/mdanalysis/miniconda/envs/pyenv/lib/python2.7/site-packages/numpy/testing/decorators.py", line 147, in skipper_func
  File "/Users/travis/build/MDAnalysis/mdanalysis/miniconda/envs/pyenv/lib/python2.7/site-packages/numpy/testing/decorators.py", line 147, in skipper_func
  File "/Users/travis/build/MDAnalysis/mdanalysis/miniconda/envs/pyenv/lib/python2.7/site-packages/MDAnalysisTests/analysis/test_hole.py", line 150, in test_hole_module_fd_closure
  File "/Users/travis/build/MDAnalysis/mdanalysis/miniconda/envs/pyenv/lib/python2.7/site-packages/MDAnalysisTests/tempdir.py", line 72, in __enter__

OSError: '[Errno 24] Too many open files\n-------------------- >> begin captured logging << --------------------\nMDAnalysis.core.AtomGroup: DEBUG: Universe.load_new(): loading /Users/travis/build/MDAnalysis/mdanalysis/miniconda/envs/pyenv/lib/python2.7/site-packages/MDAnalysisTests/data/1grm_elNemo_mode7.pdb.bz2...\n--------------------- >> end captured logging << ---------------------'
...

The problems seems to be that there are already too many files open. Once we temporarily reduce the open file descriptors to 64 we cannot even create the tempdir anymore so the tests errors.

If #363 solves the open files problem (as @jbarnoud said) then I am inclined not to do anything about this yet and merge as it is if we decide to install the additional external packages #898.

jbarnoud · 2016-07-13T07:55:00Z

.travis.yml

+         export PATH=${PATH}:${HOLE_BINDIR}; \
+     fi
+  # we should still be here but make doubly sure:
+  - cd ${TRAVIS_BUILD_DIR}


Is there a reason to make doubly sure here specifically?

I tried to make it a bit future-proof: You can put other installation scripts into this if statement. But if you are jumping directories then thr rest of the travis script will fail. The "making doubly sure" serves as a reminder that one of the assumptions of the travis script is that everything happens from within the checked-out git repo.

orbeckst · 2016-07-19T19:45:22Z

@jbarnoud or @richardjgowers – do you want to review the PR?

coveralls · 2016-07-19T20:07:17Z

Coverage increased (+1.5%) to 82.548% when pulling 7e334c3 on issue-898-external-code into 98d5c10 on develop.

coveralls · 2016-07-19T22:16:52Z

Coverage increased (+1.5%) to 82.532% when pulling f02dc88 on issue-898-external-code into 98d5c10 on develop.

orbeckst · 2016-07-19T23:00:46Z

I removed f02dc88 from the PR (and force-pushed). I thought I would switch coverage reporting to the second Travis job (SETUP == full) but that was not the case. (I only switched doc building to the full job but it's not clear that this is of any advantage.)

orbeckst · 2016-07-19T23:03:22Z

I do have a question, though: Which coverage badge is shown? When you look at https://coveralls.io/builds/7076831 you see

79.77% 2118.1 (SETUP=minimal PYTHON_VERSION=2.7)
82.47% 2118.2 (SETUP=full PYTHON_VERSION=2.7)

I would like to show/see the one for job 2. How does one do that?

jdetle · 2016-07-19T23:20:15Z

In my experience it shows the greater of the two. I remember this anecdotally because of the last PR I made increasing RMSD test coverage. In fact I am fairly confident it always shows the second.

coveralls · 2016-07-20T00:04:20Z

Coverage decreased (-1.3%) to 79.77% when pulling 389d866 on issue-898-external-code into 054a9de on develop.

coveralls · 2016-07-20T00:48:34Z

Coverage decreased (-1.2%) to 79.786% when pulling a7a2b50 on issue-898-external-code into 054a9de on develop.

coveralls · 2016-07-20T01:21:16Z

Coverage increased (+2.08%) to 83.111% when pulling 3378db2 on issue-898-external-code into 054a9de on develop.

coveralls · 2016-07-20T01:48:23Z

Coverage increased (+2.05%) to 83.075% when pulling 994e80e on issue-898-external-code into 054a9de on develop.

- just run the plot() and plot3D() methods and check that we get appropriate matplotlib objects - removed unnecessary checks for hole executable from the code: we already use the executable_not_found() decorators on the tests so we are guaranteed to have hole available - similarly, removed unnecessary checks for resources (rlimits)

…D surface creation - tests that the programs sph_process and sos_triangle can be run - these two programs litter the terminal: captured stdout/stderr in HOLE.create_vmd_surface() (at the moment, the output is never shown, which might make debugging harder) - added new utility context manager in_dir() to the MDAnalysisTests.__init__ to make directory jumping easier

@jbarnoud

In order to provoke the failure described by issue #129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under #901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by #363 so once that corresponding PR #815 is merged we should be able to revert this commit.

coveralls · 2016-07-20T06:21:15Z

Coverage increased (+2.05%) to 83.075% when pulling 0b1829b on issue-898-external-code into 054a9de on develop.

richardjgowers · 2016-07-20T09:40:32Z

It's only failing on macos, I think the highest number of open files I saw was 250!

orbeckst · 2016-07-20T16:45:02Z

I added a hack to the test for OS X just to get it to pass. Currently the max number of open fd for Darwin is 128 but we can adjust it to say 256. It's a horrible fudge but we can revisit after the #363 merge.

Or just skip the test on OS X?

Oliver Beckstein
email: orbeckst@gmail.com

Am Jul 20, 2016 um 2:40 schrieb Richard Gowers notifications@github.com:

It's only failing on macos, I think the highest number of open files I saw was 250!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

richardjgowers · 2016-07-20T16:48:47Z

I'd rather fudge the fd limit for now.

On Wed, 20 Jul 2016, 17:45 Oliver Beckstein, notifications@github.com
wrote:

I added a hack to the test for OS X just to get it to pass. Currently the
max number of open fd for Darwin is 128 but we can adjust it to say 256.
It's a horrible fudge but we can revisit after the #363 merge.

Or just skip the test on OS X?

Oliver Beckstein
email: orbeckst@gmail.com

Am Jul 20, 2016 um 2:40 schrieb Richard Gowers <notifications@github.com
:

It's only failing on macos, I think the highest number of open files I
saw was 250!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#901 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jB_5WJ9xSi5kyIps3E6Ds390VjbA6ks5qXlCPgaJpZM4JJ3ky
.

orbeckst · 2016-07-20T16:56:03Z

Ok, will do that during the day (unless anyone else beats me to it… could do this with edit on GH…)

On 20 Jul, 2016, at 09:48, Richard Gowers notifications@github.com wrote:

I'd rather fudge the fd limit for now.

Oliver Beckstein * orbeckst@gmx.net
skype: orbeckst * orbeckst@gmail.com

@jbarnoud

In order to provoke the failure described by issue #129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under #901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by #363 so once that corresponding PR #815 is merged we should be able to revert this commit.

orbeckst · 2016-07-20T18:20:38Z

Set the OS X fd limits to 256 and force-pushed.

coveralls · 2016-07-20T19:05:36Z

Coverage increased (+2.02%) to 83.125% when pulling b8bcac4 on issue-898-external-code into 6c6510c on develop.

orbeckst · 2016-07-20T19:19:50Z

Well, os x still fails. I'll try again with 512 open fd...

@jbarnoud

In order to provoke the failure described by issue #129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under #901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by #363 so once that corresponding PR #815 is merged we should be able to revert this commit.

orbeckst · 2016-07-20T20:50:58Z

Tadaa! Take that, OS X!

@richardjgowers , this seems to be done. I would prefer if the commits were not squashed because they all do different little things and in the future it might come in handy to re-evaluate them individually.

richardjgowers · 2016-07-20T21:57:22Z

@orbeckst awesome, creeping back towards 90% coverage!

orbeckst · 2016-07-20T21:59:29Z

On 20 Jul, 2016, at 14:57, Richard Gowers notifications@github.com wrote:

@orbeckst awesome, creeping back towards 90% coverage!

Partly by running Something.plot() and just testing that we can get a proper matplotlib axes instance… but yeah, I am pleased with +2% :-).

Once we throw stuff into legacy and remove it from coverage (how does one do that?) it will look even better ;-).

Oliver Beckstein * orbeckst@gmx.net
skype: orbeckst * orbeckst@gmail.com

jdetle · 2016-07-20T22:00:55Z

We can get another big boost if we add coverage for PSA.

orbeckst · 2016-07-20T22:02:52Z

On 20 Jul, 2016, at 15:00, John Detlefs notifications@github.com wrote:

We can get another big boost if we add coverage for PSA.

Ping @sseyler ;-) – maybe you can get @tcolburn to write some tests?

@jbarnoud

In order to provoke the failure described by issue MDAnalysis#129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under MDAnalysis#901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by MDAnalysis#363 so once that corresponding PR MDAnalysis#815 is merged we should be able to revert this commit.

@jbarnoud

NIMUstA8lOQd9w5kL+PcYC3o+J7vh1/gjIyJsZ5iE9y9zQ3aYtzFQShPnCfrKYyl Bco/xr+U3bN9piXTZqhYKIRzVUs6SSd2uy7q63de8QDNsNWkKouKZ1+PhvZPkMNN i+PhBW/gp+PXeta+4Y5REnBrUpX4bW3DCHuKTJ+nM80PXmsMG0i64ShRX/umXnoG qpBfOGfnr40SD/ZFmmYc8qqZvllzPjew8GMGXXdjidetOZHaAkFRDMzOr3FNYkWD 2zSKN95CJGDynSwdsDTo9rrUN5lcJr58JG+JeDBLoK7XxN5VVhPge+cV0gHF2SLk TQcgRXXY/AJY0ZpME0PRk7YKpUPqW/UjKPHENRFqNPlskF+8dzvFu2KNk928A+Ws 4otsRWCzn76tzXsGAK66kXsX9l2mc3IxhDy9TH69GXhog7ASHlglJZ1kZTcPPR8T h7SfjW1Pfi7C/HL7RJ+shmmIQd4qvfn828JK/SoHud6dG7/wmfqYIiva7RB0usdt CK8y7an2XUGXjnvIv2C2CDUHKy0KgtdRw6wpwChkeVl9ci59cT0vHkOZqXgGSaBS L168OTkm0djbpjrMGj8vWe3lna1/Fxf+ELYOC/rUTPOYt7T0SuRoUHhcHhURCD/w vViD4Ic6nvs7OR+ODkyZ =eUcw -----END PGP SIGNATURE----- adjusted HOLE open file descriptor test (MDAnalysis#129) for Darwin In order to provoke the failure described by issue MDAnalysis#129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under MDAnalysis#901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by MDAnalysis#363 so once that corresponding PR MDAnalysis#815 is merged we should be able to revert this commit.

…code install external packages for tests

orbeckst added testing Work in progress labels Jul 11, 2016

orbeckst force-pushed the issue-898-external-code branch 3 times, most recently from e0b7c2d to 74373ac Compare July 11, 2016 23:21

orbeckst force-pushed the issue-898-external-code branch from bb17de1 to 642a1d9 Compare July 12, 2016 00:12

orbeckst changed the title ~~WIP: install external packages for tests~~ install external packages for tests Jul 12, 2016

orbeckst removed the Work in progress label Jul 12, 2016

orbeckst added the needs review label Jul 12, 2016

jbarnoud reviewed Jul 13, 2016
View reviewed changes

orbeckst force-pushed the issue-898-external-code branch from f02dc88 to 7e334c3 Compare July 19, 2016 22:59

orbeckst added 2 commits July 19, 2016 22:34

orbeckst force-pushed the issue-898-external-code branch from 994e80e to 0b1829b Compare July 20, 2016 05:55

richardjgowers self-assigned this Jul 20, 2016

richardjgowers added this to the 0.16.0 milestone Jul 20, 2016

orbeckst force-pushed the issue-898-external-code branch from 0b1829b to b8bcac4 Compare July 20, 2016 18:20

orbeckst mentioned this pull request Jul 20, 2016

install external code on travis for complete analysis testing? #898

Closed

3 tasks

orbeckst force-pushed the issue-898-external-code branch from b8bcac4 to 1028307 Compare July 20, 2016 19:20

richardjgowers merged commit 9a632a9 into develop Jul 20, 2016

orbeckst mentioned this pull request Jul 21, 2016

too many open files makes AtomGroup.sequence tests fail on MacOS? #853

Closed

kain88-de deleted the issue-898-external-code branch August 23, 2016 14:01

abiedermann pushed a commit to abiedermann/mdanalysis that referenced this pull request Oct 26, 2016

Merge pull request MDAnalysis#901 from MDAnalysis/issue-898-external-…

bea242f

…code install external packages for tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

install external packages for tests #901

install external packages for tests #901

orbeckst commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

orbeckst commented Jul 12, 2016

orbeckst commented Jul 12, 2016

orbeckst commented Jul 12, 2016

coveralls commented Jul 12, 2016 •

edited

Loading

coveralls commented Jul 12, 2016

coveralls commented Jul 12, 2016

orbeckst commented Jul 12, 2016 •

edited

Loading

jbarnoud Jul 13, 2016

orbeckst Jul 19, 2016

orbeckst commented Jul 19, 2016

coveralls commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

orbeckst commented Jul 19, 2016

orbeckst commented Jul 19, 2016

jdetle commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

orbeckst commented Jul 20, 2016

coveralls commented Jul 20, 2016 •

edited

Loading

orbeckst commented Jul 20, 2016

orbeckst commented Jul 20, 2016

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

jdetle commented Jul 20, 2016

orbeckst commented Jul 20, 2016

install external packages for tests #901

install external packages for tests #901

Conversation

orbeckst commented Jul 11, 2016 • edited Loading

PR Checklist

coveralls commented Jul 11, 2016 • edited Loading

orbeckst commented Jul 12, 2016

orbeckst commented Jul 12, 2016

orbeckst commented Jul 12, 2016

coveralls commented Jul 12, 2016 • edited Loading

coveralls commented Jul 12, 2016

coveralls commented Jul 12, 2016

orbeckst commented Jul 12, 2016 • edited Loading

jbarnoud Jul 13, 2016

Choose a reason for hiding this comment

orbeckst Jul 19, 2016

Choose a reason for hiding this comment

orbeckst commented Jul 19, 2016

coveralls commented Jul 19, 2016 • edited Loading

coveralls commented Jul 19, 2016 • edited Loading

orbeckst commented Jul 19, 2016

orbeckst commented Jul 19, 2016

jdetle commented Jul 19, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

orbeckst commented Jul 20, 2016

coveralls commented Jul 20, 2016 • edited Loading

orbeckst commented Jul 20, 2016

orbeckst commented Jul 20, 2016

richardjgowers commented Jul 20, 2016

orbeckst commented Jul 20, 2016

jdetle commented Jul 20, 2016

orbeckst commented Jul 20, 2016

orbeckst commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 12, 2016 •

edited

Loading

orbeckst commented Jul 12, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

jdetle commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading