Skip to content

RF: do not require testing harnesses for regular usage #2854

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

Merged
merged 3 commits into from
Jan 20, 2019

Conversation

yarikoptic
Copy link
Member

continuation to #2850 , I guess awaiting on verdict from #2853 as well

@yarikoptic
Copy link
Member Author

additional note/motivation is that pytest >= 3.6 cannot even be satisfied on stable Debian/Ubuntus (e.g. debian stretch/stable has 3.0.6).

@yarikoptic
Copy link
Member Author

@oesteban do you remember "why?" (ref: a4dfe7e):

commit a4dfe7e6a2ec78ba9cea20005799001f16ddcb99 (origin/oesteban-patch-2)
Author: Oscar Esteban <[email protected]>
Date:   Thu Dec 13 16:41:55 2018 -0800

    [FIX] Update pytest req'd version to 3.6
    
    Fixes #2826.

diff --git a/nipype/info.py b/nipype/info.py
index 1cf361c40..be6edb713 100644
--- a/nipype/info.py
+++ b/nipype/info.py
@@ -108,7 +108,7 @@ NUMPY_MIN_VERSION_37 = '1.15.3'
 SCIPY_MIN_VERSION = '0.14'
 TRAITS_MIN_VERSION = '4.6'
 DATEUTIL_MIN_VERSION = '2.2'
-PYTEST_MIN_VERSION = '3.0'
+PYTEST_MIN_VERSION = '3.6'
 FUTURE_MIN_VERSION = '0.16.0'
 SIMPLEJSON_MIN_VERSION = '3.8.0'
 PROV_VERSION = '1.5.2'

seems to run tests just fine on debian stretch with 3.0.6-1 .

@yarikoptic
Copy link
Member Author

ah, @oesteban - please disregard my above questsion -- did not spot #2826 which somewhat explains the reason. I think for the Debian package I would just eliminate any versioning of the dependency since it would install most recent available, and fail if fails to build/test

@effigies
Copy link
Member

Okay. So we may wish to drop that requirement, but that means either setting a max version on xdist, which itself set a minimum version of 3.6 (see #2826, #2827) or dropping the requirement for xdist, which will in turn require removing the -n auto from the pytest.ini. At which point, we'd want to instruct Travis and Circle to install pytest-xdist separately, and add -n auto for the sake of speed.

I'm fine with all of this. It's just a little rabbit hole.

@yarikoptic
Copy link
Member Author

Thank you @effigies ! whatever you feel works ok for you is ok with me -- at least I can workaround.

@effigies effigies added this to the 1.1.8 milestone Jan 14, 2019
@codecov-io
Copy link

codecov-io commented Jan 16, 2019

Codecov Report

Merging #2854 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2854   +/-   ##
=======================================
  Coverage   67.46%   67.46%           
=======================================
  Files         341      341           
  Lines       43392    43392           
  Branches     5383     5383           
=======================================
  Hits        29276    29276           
  Misses      13419    13419           
  Partials      697      697
Flag Coverage Δ
#smoketests 50.53% <ø> (ø) ⬆️
#unittests 64.88% <ø> (ø) ⬆️
Impacted Files Coverage Δ
nipype/info.py 94.2% <ø> (ø) ⬆️

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 ebbd062...7085e2b. Read the comment docs.

@effigies effigies merged commit 7085e2b into nipy:master Jan 20, 2019
@yarikoptic yarikoptic deleted the bf-nopytest branch February 17, 2019 03:10
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.

3 participants