Use a fixture for npz files in test_bat #2981

jbarnoud · 2020-10-13T09:51:37Z

The file test_bat_IO.npy was created in one test and used in
another one. This leads to the second test possibly failing
when the tests are run in parallel. This commit moves to create
the file into a fixture that the two tests depend.

The new fixture also uses the tmpdir fixture to avoid the file
created during the tests to remain in the test directory.

PR Checklist

Tests?
~~[ ] Docs?~~
CHANGELOG updated?
Issue raised/referenced?

Fixes #2979 The file test_bat_IO.npy was created in one test and used in another one. This leads to the second test possibly failing when the tests are run in parallel. This commit moves creating the file into a fixture that the two tests depends. The new fixture also uses the tmpdir fixture to avoid the file created during the tests to remain in the test directory.

codecov · 2020-10-13T12:48:49Z

Codecov Report

Merging #2981 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2981   +/-   ##
========================================
  Coverage    93.05%   93.05%           
========================================
  Files          186      186           
  Lines        24609    24609           
  Branches      3187     3187           
========================================
  Hits         22900    22900           
  Misses        1661     1661           
  Partials        48       48

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9543ba...cecfe06. Read the comment docs.

jbarnoud · 2020-10-13T13:00:03Z

There is one build failing in an odd way that seems absolutelly unrelated to the PR:

_________________ ERROR collecting MDAnalysisTests/test_api.py _________________

../../../miniconda/envs/test/lib/python3.6/site-packages/_pytest/python.py:571: in _importtestmodule

    mod = import_path(self.fspath, mode=importmode)

../../../miniconda/envs/test/lib/python3.6/site-packages/_pytest/pathlib.py:517: in import_path

    importlib.import_module(module_name)

../../../miniconda/envs/test/lib/python3.6/importlib/__init__.py:126: in import_module

    return _bootstrap._gcd_import(name[level:], package, level)

<frozen importlib._bootstrap>:994: in _gcd_import

    ???

<frozen importlib._bootstrap>:971: in _find_and_load

    ???

<frozen importlib._bootstrap>:955: in _find_and_load_unlocked

    ???

<frozen importlib._bootstrap>:665: in _load_unlocked

    ???

../../../miniconda/envs/test/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:171: in exec_module

    exec(co, module.__dict__)

testsuite/MDAnalysisTests/test_api.py:27: in <module>

    import MDAnalysis as mda

package/MDAnalysis/__init__.py:207: in <module>

    from .coordinates.core import writer as Writer

package/MDAnalysis/coordinates/__init__.py:735: in <module>

    from . import H5MD

../../../miniconda/envs/test/lib/python3.6/site-packages/duecredit/injections/injector.py:285: in __import

    mod = self._orig_import(name, *args, **kwargs)

package/MDAnalysis/coordinates/H5MD.py:193: in <module>

    import h5py

../../../miniconda/envs/test/lib/python3.6/site-packages/duecredit/injections/injector.py:285: in __import

    mod = self._orig_import(name, *args, **kwargs)

../../../miniconda/envs/test/lib/python3.6/site-packages/h5py/__init__.py:62: in <module>

    from .tests import run_tests

../../../miniconda/envs/test/lib/python3.6/site-packages/duecredit/injections/injector.py:285: in __import

    mod = self._orig_import(name, *args, **kwargs)

../../../miniconda/envs/test/lib/python3.6/site-packages/h5py/tests/__init__.py:15: in <module>

    from . import old, hl

../../../miniconda/envs/test/lib/python3.6/site-packages/duecredit/injections/injector.py:285: in __import

    mod = self._orig_import(name, *args, **kwargs)

../../../miniconda/envs/test/lib/python3.6/site-packages/h5py/tests/old/__init__.py:4: in <module>

    from . import ( test_attrs,

../../../miniconda/envs/test/lib/python3.6/site-packages/duecredit/injections/injector.py:285: in __import

    mod = self._orig_import(name, *args, **kwargs)

<frozen importlib._bootstrap>:971: in _find_and_load

    ???

<frozen importlib._bootstrap>:955: in _find_and_load_unlocked

    ???

<frozen importlib._bootstrap>:665: in _load_unlocked

    ???

../../../miniconda/envs/test/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:162: in exec_module

    source_stat, co = _rewrite_test(fn, self.config)

../../../miniconda/envs/test/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:357: in _rewrite_test

    tree = ast.parse(source, filename=fn_)

../../../miniconda/envs/test/lib/python3.6/ast.py:35: in parse

    return compile(source, filename, mode, PyCF_ONLY_AST)

E     File "/home/travis/miniconda/envs/test/lib/python3.6/site-packages/h5py/tests/old/test_attrs_data.py", line 251

E       s = b"Hello\x00\Hello"

E          ^

E   SyntaxError: invalid escape sequence \H

IAlibay

😅 I was initially confused as to how the original code worked without leaving files behind, but I guess it just always left the *.npy and I never actually fully ran the analysis tests since it got implemented....

Thanks @jbarnoud, lgtm!

IAlibay · 2020-10-13T13:34:50Z

package/CHANGELOG

@@ -58,6 +58,8 @@ Fixes
    Universe (PR #2893)
  * ensure that unistd.h is included on macOS when compiling ENCORE's spe.c
    (Issue #2934)
+  * Fix tests for analysis.bat that could fail when run in parallel and that


I guess technically it should be newest first, but I've seen plenty of cases where we've not followed this so it's a bit late to be pedantic now 🤷‍♂️

Oops... I have been doing it wrong for a while 😅

IAlibay · 2020-10-13T13:36:53Z

There is one build failing in an odd way that seems absolutelly unrelated to the PR:

That's a weird one, I don't have a clue what's happening here :( I've restarted the job just to see if it's a consistent failure.

IAlibay · 2020-10-13T14:09:12Z

Test still fails.

Likely related, what's particularly interesting here is that unlike the py3.7+ tests, the h5py version which ends up being installed is 2.7 rather than 2.10.

For some strange reason, the dry run does pick up 2.10, but then in the actual install it picks up 2.7:
https://travis-ci.com/github/MDAnalysis/mdanalysis/jobs/398748734#L1137

edit: typos

jbarnoud · 2020-10-13T14:25:37Z

Also, it looks like previous builds from 3 days ago were picking 2.10 as expected.

jbarnoud · 2020-10-14T09:01:06Z

Looks like the travis error was a fluke, probably with the conda-forge repository.

IAlibay · 2020-10-14T12:38:59Z

Looks like the travis error was a fluke, probably with the conda-forge repository.

How strange, I can't see anything upstream that would be the cause of this, at least it works now 🤷‍♂️

Fixes MDAnalysis#2979 ## Work done in this PR The file test_bat_IO.npy was created in one test and used in another one. This leads to the second test possibly failing when the tests are run in parallel. This commit moves creating the file into a fixture that the two tests depends. The new fixture also uses the tmpdir fixture to avoid the file created during the tests to remain in the test directory.

jbarnoud added 3 commits October 12, 2020 21:03

Remove unused import

2f5345c

Update Changelog

cecfe06

jbarnoud added testing Component-Analysis labels Oct 13, 2020

jbarnoud changed the title ~~Use a ficture for npz files in test_bat~~ Use a fixture for npz files in test_bat Oct 13, 2020

IAlibay approved these changes Oct 13, 2020

View reviewed changes

IAlibay merged commit 64a7c05 into develop Oct 14, 2020

IAlibay deleted the issue2979-bat_IO branch October 14, 2020 12:42

fiona-naughton added the maintainability label Sep 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a fixture for npz files in test_bat #2981

Use a fixture for npz files in test_bat #2981

jbarnoud commented Oct 13, 2020 •

edited

Loading

codecov bot commented Oct 13, 2020 •

edited

Loading

jbarnoud commented Oct 13, 2020

IAlibay left a comment •

edited

Loading

IAlibay Oct 13, 2020

jbarnoud Oct 13, 2020

IAlibay commented Oct 13, 2020

IAlibay commented Oct 13, 2020 •

edited

Loading

jbarnoud commented Oct 13, 2020

jbarnoud commented Oct 14, 2020

IAlibay commented Oct 14, 2020

Use a fixture for npz files in test_bat #2981

Use a fixture for npz files in test_bat #2981

Conversation

jbarnoud commented Oct 13, 2020 • edited Loading

PR Checklist

codecov bot commented Oct 13, 2020 • edited Loading

Codecov Report

jbarnoud commented Oct 13, 2020

IAlibay left a comment • edited Loading

Choose a reason for hiding this comment

IAlibay Oct 13, 2020

Choose a reason for hiding this comment

jbarnoud Oct 13, 2020

Choose a reason for hiding this comment

IAlibay commented Oct 13, 2020

IAlibay commented Oct 13, 2020 • edited Loading

jbarnoud commented Oct 13, 2020

jbarnoud commented Oct 14, 2020

IAlibay commented Oct 14, 2020

jbarnoud commented Oct 13, 2020 •

edited

Loading

codecov bot commented Oct 13, 2020 •

edited

Loading

IAlibay left a comment •

edited

Loading

IAlibay commented Oct 13, 2020 •

edited

Loading